net.box: fix on_disconnect inconsistency with documentation

According to the documentation [1]:
> If the trigger function causes an error, the error is logged but
otherwise is ignored.

However, currently, the `on_disconnect` trigger behaves the same way as the
`on_connect` trigger, i.e., the connection is terminated and its state
changes to 'error'. Let's fix this inconsistency and log errors from the
`on_disconnect` trigger, but otherwise ignore them.

Closes tarantool#9677
Closes tarantool#9797

NO_DOC=<bugfix>

1. https://www.tarantool.io/en/doc/latest/reference/reference_lua/net_box/#lua-function.conn.on_disconnect

changelogs/unreleased/gh-9677-netbox-on_disconnect-error-behaviour.md

@@ -0,0 +1,4 @@
+## bugfix/lua
+
+* Fixed inconsistency between documented `on_disconnect` trigger behaviour of
+  `net.box` connections when an error is thrown and actual behaviour (gh-9717).

changelogs/unreleased/gh-9797-netbox-on_disconnect-error-hangs-server.md

@@ -0,0 +1,5 @@
+## bugfix/lua
+
+* Fixed a bug in `on_disconnect` trigger of `net.box` connections which caused
+  a Tarantool server to hang indefinitely when an error was thrown from the
+  trigger (gh-9797).

src/box/lua/net_box.lua

@@ -287,7 +287,11 @@ local function new_sm(uri_or_fd, opts)
                    state == 'closed' then
                 if was_connected then
                     remote._is_connected = false
-                    remote._on_disconnect:run(remote)
+                    local ok, trigger_err = pcall(remote._on_disconnect.run,
+                                                  remote._on_disconnect, remote)
+                    if not ok then
+                        log.error(trigger_err)
+                    end
                 end
                 -- A server may exit after initiating a graceful shutdown but
                 -- before all clients close their connections (for example, on

test/box-luatest/gh_9309_errors_in_triggers_test.lua

@@ -175,8 +175,6 @@ g.after_test('test_net_box_triggers_error', function()
     g.client = nil
 end)
 
--- Cannot test on_disconnect triggers because of
--- https://github.com/tarantool/luatest/issues/356
 g.test_net_box_triggers_error = function()
     g.client:exec(function(uri)
         local net = require('net.box')
@@ -188,6 +186,12 @@ g.test_net_box_triggers_error = function()
         -- Error is raised when the connection is used
         t.assert_error_msg_content_equals(errmsg, conn.call, conn, 'abc')
 
+        conn = net.connect(uri, {wait_connected = true})
+        errmsg = 'net.box on_disconnect error'
+        -- Error is logged - it will be checked later
+        conn:on_disconnect(function() error(errmsg) end)
+        conn:close()
+
         conn = net.connect(uri, {wait_connected = false})
         t.assert_equals(conn.state, 'initial')
         errmsg = 'net.box on_schema_reload error'
@@ -203,6 +207,7 @@ g.test_net_box_triggers_error = function()
     end, {g.server.net_box_uri})
     g.server:drop()
 
+    t.assert(g.client:grep_log("net.box on_disconnect error"))
     t.assert(g.client:grep_log("net.box on_shutdown error"))
     g.client:drop()
 end

test/box-luatest/gh_9677_netbox_on_disconnect_error_behaviour_test.lua

@@ -0,0 +1,31 @@
+local server = require('luatest.server')
+local t = require('luatest')
+
+local g = t.group()
+
+g.before_all(function(cg)
+    cg.server = server:new()
+    cg.server:start()
+end)
+
+g.after_all(function(cg)
+    cg.server:drop()
+end)
+
+-- Tests that the actual behaviour of the `on_disconnect` trigger when an error
+-- is thrown is consistent with its documented behaviour.
+g.test_on_disconnect_error_behaviour = function(cg)
+    local trigger_err = '777'
+    cg.server:exec(function(trigger_err)
+        local net_box = require('net.box')
+
+        local c = net_box.connect(box.cfg.listen)
+        c:on_disconnect(function()
+            error(trigger_err)
+        end)
+        local ok = pcall(function() c:close() end)
+        t.assert(ok)
+        t.assert(c:wait_state('closed', 10))
+    end, {trigger_err})
+    t.assert(cg.server:grep_log(trigger_err))
+end

test/box-luatest/gh_9797_netbox_on_disconnect_error_hangs_server_test.lua

@@ -0,0 +1,32 @@
+local net_box = require('net.box')
+local server = require('luatest.server')
+local t = require('luatest')
+
+local g = t.group()
+
+g.before_all(function(cg)
+    cg.server = server:new()
+    cg.server:start()
+end)
+
+g.after_all(function(cg)
+    cg.server:drop()
+end)
+
+-- Tests that throwing an error from the `on_disconnect` trigger does not hang
+-- the server indefinitely.
+g.test_on_disconnect_error_hangs_server = function(cg)
+    local c = net_box.connect(cg.server.net_box_uri)
+    c:on_disconnect(function()
+        error(777)
+    end)
+    pcall(function() c:close() end)
+    local log_file = cg.server:exec(function()
+        box.ctl.set_on_shutdown_timeout(1)
+        -- `grep_log` will not be able to retrieve it after we drop the server.
+        return box.cfg.log
+    end)
+    cg.server:drop()
+    t.assert_not(cg.server:grep_log('on_shutdown triggers failed', 1024,
+                                    {filename = log_file}))
+end