[Tarantool-patches] [PATCH] netbox: fix empty error message
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Tue Jun 23 00:23:48 MSK 2020
Hi! Thanks for the patch!
See 5 comments below, a patch on top the branch and in the end of
the email.
On 22/06/2020 20:34, Olga Arkhangelskaia wrote:
> When the connection was not established yet netbox reported empty error
> while executing a remote request.
> Closes #4787
> ---
> Branch OKriw/gh-4787-netbox-reports-empty-error
>
> src/box/lua/net_box.lua | 6 ++--
> test/app/gh-4787-netbox-empty-errmsg.result | 34 +++++++++++++++++++
> test/app/gh-4787-netbox-empty-errmsg.test.lua | 15 ++++++++
> 3 files changed, 53 insertions(+), 2 deletions(-)
> create mode 100644 test/app/gh-4787-netbox-empty-errmsg.result
> create mode 100755 test/app/gh-4787-netbox-empty-errmsg.test.lua
>
> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> index 9560bfdd4..6774729b4 100644
> --- a/src/box/lua/net_box.lua
> +++ b/src/box/lua/net_box.lua
> @@ -541,8 +541,10 @@ local function create_transport(host, port, user, password, callback,
> local function perform_async_request(buffer, skip_header, method, on_push,
> on_push_ctx, request_ctx, ...)
> if state ~= 'active' and state ~= 'fetch_schema' then
> - return nil, box.error.new({code = last_errno or E_NO_CONNECTION,
> - reason = last_error})
> + return nil, box.error.new({code = last_error or E_NO_CONNECTION,
> + reason = last_error or
> + string.format("connection is not eshatblished, state: %s",
> + state)})
1. The code block is really bad formatted. Also there are typos in the word
'eshatblished'.
> end
> -- alert worker to notify it of the queued outgoing data;
> -- if the buffer wasn't empty, assume the worker was already alerted
> diff --git a/test/app/gh-4787-netbox-empty-errmsg.result b/test/app/gh-4787-netbox-empty-errmsg.result
> new file mode 100644
> index 000000000..9e14cfb19
> --- /dev/null
> +++ b/test/app/gh-4787-netbox-empty-errmsg.result
2. The test passes even without the fix.
> @@ -0,0 +1,34 @@
> +-- test-run result file version 2
> +netbox = require('net.box')
> + | ---
> + | ...
> +--
> +--gh-4787:netbox reported empty error message while executing remote call
> +--
> +box.schema.user.grant('guest', 'execute', 'universe')
> + | ---
> + | ...
> +ok, err = nil
> + | ---
> + | ...
> +-- Due to race when wait_connected = false, run whole block to get an error
> +do \
> + c = netbox.connect(box.cfg.listen, {wait_connected = false}) \
> + ok, err = pcall(c.call, c, 'any', {}, {is_async = true}) \
> +end
> + | ---
> + | ...
> +err ~= nil
> + | ---
> + | - true
> + | ...
> +err:unpack().message ~= nil
> + | ---
> + | - true
> + | ...
> +c:close()
> + | ---
> + | ...
> +box.schema.user.revoke('guest', 'read,write,execute,create', 'universe')
> + | ---
> + | ...
> diff --git a/test/app/gh-4787-netbox-empty-errmsg.test.lua b/test/app/gh-4787-netbox-empty-errmsg.test.lua
> new file mode 100755
> index 000000000..2949f16ea
> --- /dev/null
> +++ b/test/app/gh-4787-netbox-empty-errmsg.test.lua
> @@ -0,0 +1,15 @@
> +netbox = require('net.box')
> +--
> +--gh-4787:netbox reported empty error message while executing remote call
3. Please, use whitespaces after symbols like ',', ':', '--', etc. Also
end sentences using the dot, and keep the comments in 66 line width.
> +--
> +box.schema.user.grant('guest', 'execute', 'universe')
> +ok, err = nil
> +-- Due to race when wait_connected = false, run whole block to get an error
> +do \
> + c = netbox.connect(box.cfg.listen, {wait_connected = false}) \
4. Indentation step in Lua code is 4 spaces, not 3.
> + ok, err = pcall(c.call, c, 'any', {}, {is_async = true}) \
5. You are calling a not existing function. So the error message can actually
contain something about this type of error instead of the not established
connection. I know I used the test in the issue, but it is not perfect.
> +end
> +err ~= nil
> +err:unpack().message ~= nil
> +c:close()
> +box.schema.user.revoke('guest', 'read,write,execute,create', 'universe')
Consider the diff below, which I also pushed on top of the branch.
====================
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 6774729b4..70bba0d6f 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -541,10 +541,11 @@ local function create_transport(host, port, user, password, callback,
local function perform_async_request(buffer, skip_header, method, on_push,
on_push_ctx, request_ctx, ...)
if state ~= 'active' and state ~= 'fetch_schema' then
- return nil, box.error.new({code = last_error or E_NO_CONNECTION,
- reason = last_error or
- string.format("connection is not eshatblished, state: %s",
- state)})
+ local code = last_errno or E_NO_CONNECTION
+ local msg = last_error or
+ string.format('Connection is not established, state is "%s"',
+ state)
+ return nil, box.error.new({code = code, reason = msg})
end
-- alert worker to notify it of the queued outgoing data;
-- if the buffer wasn't empty, assume the worker was already alerted
diff --git a/test/app/gh-4787-netbox-empty-errmsg.result b/test/app/gh-4787-netbox-empty-errmsg.result
index 72d2d08e0..d30337a05 100644
--- a/test/app/gh-4787-netbox-empty-errmsg.result
+++ b/test/app/gh-4787-netbox-empty-errmsg.result
@@ -2,32 +2,60 @@
netbox = require('net.box')
| ---
| ...
+fiber = require('fiber')
+ | ---
+ | ...
--
---gh-4787:netbox reported empty error message while executing remote call
+-- gh-4787: netbox reported empty error message while executing
+-- remote call.
--
-box.schema.user.grant('guest', 'execute', 'universe')
+box.schema.user.create('test', { password = 'test' })
| ---
| ...
-ok, err = nil
+box.schema.user.grant('test', 'super')
| ---
| ...
-do \
- c = netbox.connect(box.cfg.listen, {wait_connected = false}) \
- ok, err = pcall(c.call, c, 'any', {}, {is_async = true}) \
+function echo(...) return ... end
+ | ---
+ | ...
+
+-- Check that a request in 'auth' state returns a correct error.
+function req_during_auth() \
+ local c = netbox.connect(box.cfg.listen, { \
+ user = 'test', password = 'test', wait_connected = false \
+ }) \
+ while c.state ~= 'auth' do fiber.yield() end \
+ local ok, err = pcall(c.call, c, 'echo', {}, {is_async = true}) \
+ c:close() \
+ return ok, err \
end
| ---
| ...
-err ~= nil
+
+req_during_auth()
+ | ---
+ | - false
+ | - Connection is not established, state is "auth"
+ | ...
+
+-- Check the same for 'initial' state.
+ok, err = nil
+ | ---
+ | ...
+do \
+ c = netbox.connect(box.cfg.listen, {wait_connected = false}) \
+ ok, err = pcall(c.call, c, 'echo', {}, {is_async = true}) \
+end
| ---
- | - true
| ...
-err:unpack().message ~= nil
+ok, err
| ---
- | - true
+ | - false
+ | - Connection is not established, state is "initial"
| ...
c:close()
| ---
| ...
-box.schema.user.revoke('guest', 'read,write,execute,create', 'universe')
+box.schema.user.drop('test')
| ---
| ...
diff --git a/test/app/gh-4787-netbox-empty-errmsg.test.lua b/test/app/gh-4787-netbox-empty-errmsg.test.lua
index 41ab3fec0..0eecaa1bf 100755
--- a/test/app/gh-4787-netbox-empty-errmsg.test.lua
+++ b/test/app/gh-4787-netbox-empty-errmsg.test.lua
@@ -1,14 +1,32 @@
netbox = require('net.box')
+fiber = require('fiber')
--
---gh-4787:netbox reported empty error message while executing remote call
+-- gh-4787: netbox reported empty error message while executing
+-- remote call.
--
-box.schema.user.grant('guest', 'execute', 'universe')
+box.schema.user.create('test', { password = 'test' })
+box.schema.user.grant('test', 'super')
+function echo(...) return ... end
+
+-- Check that a request in 'auth' state returns a correct error.
+function req_during_auth() \
+ local c = netbox.connect(box.cfg.listen, { \
+ user = 'test', password = 'test', wait_connected = false \
+ }) \
+ while c.state ~= 'auth' do fiber.yield() end \
+ local ok, err = pcall(c.call, c, 'echo', {}, {is_async = true}) \
+ c:close() \
+ return ok, err \
+end
+
+req_during_auth()
+
+-- Check the same for 'initial' state.
ok, err = nil
-do \
- c = netbox.connect(box.cfg.listen, {wait_connected = false}) \
- ok, err = pcall(c.call, c, 'any', {}, {is_async = true}) \
+do \
+ c = netbox.connect(box.cfg.listen, {wait_connected = false}) \
+ ok, err = pcall(c.call, c, 'echo', {}, {is_async = true}) \
end
-err ~= nil
-err:unpack().message ~= nil
+ok, err
c:close()
-box.schema.user.revoke('guest', 'read,write,execute,create', 'universe')
+box.schema.user.drop('test')
More information about the Tarantool-patches
mailing list