Skip to content

Conversation

@ochaplashkin
Copy link

@ochaplashkin ochaplashkin commented Jan 30, 2023

Due to the specifics of the unpack function in the server:exec() method an array structure check has been added. An error will be raised if the arguments are not an array.

Normal cases

-- some_test.lua
server:exec(function(a, b)
    -- `a` is 'a'
    -- `b` is 'b'
end, {'a', 'b'})

server:exec(function()
    -- nothing
end)

Number of items and structure length

Due to the specific and non-deterministic # method, we cannot use this table attribute.
These cases will be skipped and will not be checked.

-- some_test.lua
server:exec(function(a)
    -- nothing
end)

server:exec(function(a,b)
    -- `a` is 'a'
    -- `b` is nil
end, {'a'})

server:exec(function(a)
    -- `a` is 'a'
    --  the rest of the arguments are cut off
end, {'a', 'b'})

Corner case with shifting arguments

-- some_test.lua
server:exec(function(a,b,c)
    -- `a` is 'a'
    -- `b` is 'c' (!) shifted
    -- `c` is nil
end, {'a', b='b', 'c'})

Luatest will give an error:

bad argument #3 for exec at malformed_args_test.lua:66: an array is required"
  • Tests
  • Changelog
  • Documentation

Resolves #230

@ochaplashkin ochaplashkin marked this pull request as draft January 30, 2023 16:09
@ochaplashkin ochaplashkin changed the title Add logging when malformed args passed Add logging when malformed arguments passed Jan 30, 2023
@ochaplashkin ochaplashkin force-pushed the add-log-when-malformed-arguments branch from 71fd99e to d7642b9 Compare January 30, 2023 16:33
@ochaplashkin ochaplashkin marked this pull request as ready for review January 31, 2023 11:50
@ochaplashkin ochaplashkin force-pushed the add-log-when-malformed-arguments branch from d7642b9 to 12d23ec Compare February 1, 2023 10:26
@ochaplashkin ochaplashkin marked this pull request as draft February 3, 2023 14:12
@ochaplashkin ochaplashkin force-pushed the add-log-when-malformed-arguments branch 6 times, most recently from a62ee8d to d9ed0d8 Compare February 8, 2023 12:29
@ochaplashkin
Copy link
Author

@sergepetrenko , hi!
I updated the logic of the work and added an obvious error if there are problems with the types in the arguments.

Everything else (logging) remained unchanged.

@sergepetrenko sergepetrenko self-requested a review February 9, 2023 09:46
Copy link
Contributor

@sergepetrenko sergepetrenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this!

Looks good. Only one note from me.

@ochaplashkin ochaplashkin force-pushed the add-log-when-malformed-arguments branch 2 times, most recently from 35741a5 to 175db2d Compare February 13, 2023 08:11
@ochaplashkin ochaplashkin marked this pull request as ready for review February 13, 2023 08:15
@ochaplashkin
Copy link
Author

I don't really like the current version, but I don't know yet how to work more comfortably with the table structure
Moved from draft to review.

@ochaplashkin ochaplashkin changed the title Add logging when malformed arguments passed Add error when malformed arguments passed Feb 17, 2023
@ochaplashkin ochaplashkin force-pushed the add-log-when-malformed-arguments branch 2 times, most recently from 1425756 to 003b449 Compare February 17, 2023 09:40
@ochaplashkin ochaplashkin force-pushed the add-log-when-malformed-arguments branch from 003b449 to d8dc120 Compare February 27, 2023 10:33
@ochaplashkin ochaplashkin changed the title Add error when malformed arguments passed Raise an error when non-array arguments passed to the server:exec Feb 27, 2023
@ochaplashkin ochaplashkin force-pushed the add-log-when-malformed-arguments branch from d8dc120 to c044c9e Compare February 27, 2023 11:05
An error will be raised if the non-array arguments have been passed to
the `server:exec()`.

Resolves tarantool#230
@ochaplashkin ochaplashkin force-pushed the add-log-when-malformed-arguments branch from c044c9e to 4fda84c Compare February 27, 2023 12:45
Copy link
Contributor

@ylobankov ylobankov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ylobankov ylobankov merged commit 0a8b39f into tarantool:master Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

server:exec() silently ignores malformed arguments

4 participants