Merge pull request #9896 from justinmk/api-async-error

API: emit nvim_error_event on failed async request

src/nvim/func_attr.h

@@ -205,10 +205,15 @@
 #endif
 
 #ifdef DEFINE_FUNC_ATTRIBUTES
+/// Non-deferred API function.
 # define FUNC_API_ASYNC
+/// Internal C function not exposed in the RPC API.
 # define FUNC_API_NOEXPORT
+/// API function not exposed in VimL/eval.
 # define FUNC_API_REMOTE_ONLY
+/// API function introduced at the given API level.
 # define FUNC_API_SINCE(X)
+/// API function deprecated since the given API level.
 # define FUNC_API_DEPRECATED_SINCE(X)
 # define FUNC_ATTR_MALLOC REAL_FATTR_MALLOC
 # define FUNC_ATTR_ALLOC_SIZE(x) REAL_FATTR_ALLOC_SIZE(x)

src/nvim/msgpack_rpc/channel.c

@@ -348,28 +348,28 @@ static void handle_request(Channel *channel, msgpack_object *request)
 
     if (is_get_mode && !input_blocking()) {
       // Defer the event to a special queue used by os/input.c. #6247
-      multiqueue_put(ch_before_blocking_events, response_event, 1, evdata);
+      multiqueue_put(ch_before_blocking_events, request_event, 1, evdata);
     } else {
       // Invoke immediately.
-      response_event((void **)&evdata);
+      request_event((void **)&evdata);
     }
   } else {
-    multiqueue_put(channel->events, response_event, 1, evdata);
+    multiqueue_put(channel->events, request_event, 1, evdata);
     DLOG("RPC: scheduled %.*s", method->via.bin.size, method->via.bin.ptr);
   }
 }
 
-/// Responds to a message, depending on the type:
+/// Handles a message, depending on the type:
-///   - Request: writes the response.
+///   - Request: invokes method and writes the response (or error).
-///   - Notification: does nothing.
+///   - Notification: invokes method (emits `nvim_error_event` on error).
-static void response_event(void **argv)
+static void request_event(void **argv)
 {
   RequestEvent *e = argv[0];
   Channel *channel = e->channel;
   MsgpackRpcRequestHandler handler = e->handler;
   Error error = ERROR_INIT;
   Object result = handler.fn(channel->id, e->args, &error);
-  if (e->type == kMessageTypeRequest) {
+  if (e->type == kMessageTypeRequest || ERROR_SET(&error)) {
     // Send the response.
     msgpack_packer response;
     msgpack_packer_init(&response, &out_buffer, msgpack_sbuffer_write);

test/functional/api/vim_spec.lua

@@ -49,13 +49,23 @@ describe('API', function()
 
   it('handles errors in async requests', function()
     local error_types = meths.get_api_info()[2].error_types
-    nvim_async("bogus")
+    nvim_async('bogus')
     eq({'notification', 'nvim_error_event',
         {error_types.Exception.id, 'Invalid method: nvim_bogus'}}, next_msg())
     -- error didn't close channel.
     eq(2, eval('1+1'))
   end)
 
+  it('failed async request emits nvim_error_event', function()
+    local error_types = meths.get_api_info()[2].error_types
+    nvim_async('command', 'bogus')
+    eq({'notification', 'nvim_error_event',
+        {error_types.Exception.id, 'Vim:E492: Not an editor command: bogus'}},
+        next_msg())
+    -- error didn't close channel.
+    eq(2, eval('1+1'))
+  end)
+
   it('does not set CA_COMMAND_BUSY #7254', function()
     nvim('command', 'split')
     nvim('command', 'autocmd WinEnter * startinsert')

test/functional/ex_cmds/swapfile_preserve_recover_spec.lua

@@ -13,6 +13,7 @@ local rmdir = helpers.rmdir
 local set_session = helpers.set_session
 local spawn = helpers.spawn
 local nvim_async = helpers.nvim_async
+local expect_msg_seq = helpers.expect_msg_seq
 
 describe(':recover', function()
   before_each(clear)
@@ -163,6 +164,13 @@ describe('swapfile detection', function()
     screen2:expect{any=[[Found a swap file by the name ".*]]
                        ..[[Xtest_swapdialog_dir[/\].*]]..testfile..[[%.swp"]]}
     feed('e')  -- Chose "Edit" at the swap dialog.
-    feed('<c-c>')
+    expect_msg_seq({
+      ignore={'redraw'},
+      seqs={
+        { {'notification', 'nvim_error_event', {0, 'Vim(edit):E325: ATTENTION'}},
+        }
+      }
+    })
+    feed('<cr>')
   end)
 end)

test/functional/helpers.lua

@@ -23,6 +23,7 @@ local neq = global_helpers.neq
 local ok = global_helpers.ok
 local read_file = global_helpers.read_file
 local sleep = global_helpers.sleep
+local table_contains = global_helpers.table_contains
 local table_flatten = global_helpers.table_flatten
 local write_file = global_helpers.write_file
 
@@ -129,16 +130,33 @@ end
 
 -- Expects a sequence of next_msg() results. If multiple sequences are
 -- passed they are tried until one succeeds, in order of shortest to longest.
+--
+-- Can be called with positional args (list of sequences only):
+--    expect_msg_seq(seq1, seq2, ...)
+-- or keyword args:
+--    expect_msg_seq{ignore={...}, seqs={seq1, seq2, ...}}
+--
+-- ignore:      List of ignored event names.
+-- seqs:        List of one or more potential event sequences.
 local function expect_msg_seq(...)
   if select('#', ...) < 1 then
     error('need at least 1 argument')
   end
-  local seqs = {...}
+  local arg1 = select(1, ...)
+  if (arg1['seqs'] and select('#', ...) > 1) or type(arg1) ~= 'table'  then
+    error('invalid args')
+  end
+  local ignore = arg1['ignore'] and arg1['ignore'] or {}
+  local seqs = arg1['seqs'] and arg1['seqs'] or {...}
+  if type(ignore) ~= 'table' then
+    error("'ignore' arg must be a list of strings")
+  end
   table.sort(seqs, function(a, b)  -- Sort ascending, by (shallow) length.
     return #a < #b
   end)
 
   local actual_seq = {}
+  local nr_ignored = 0
   local final_error = ''
   local function cat_err(err1, err2)
     if err1 == nil then
@@ -151,13 +169,17 @@ local function expect_msg_seq(...)
     -- Collect enough messages to compare the next expected sequence.
     while #actual_seq < #expected_seq do
       local msg = next_msg(10000)  -- Big timeout for ASAN/valgrind.
+      local msg_type = msg and msg[2] or nil
       if msg == nil then
         error(cat_err(final_error,
-                      string.format('got %d messages, expected %d',
+                      string.format('got %d messages (ignored %d), expected %d',
-                                    #actual_seq, #expected_seq)))
+                                    #actual_seq, nr_ignored, #expected_seq)))
-      end
+      elseif table_contains(ignore, msg_type) then
+        nr_ignored = nr_ignored + 1
+      else
         table.insert(actual_seq, msg)
       end
+    end
     local status, result = pcall(eq, expected_seq, actual_seq)
     if status then
       return result

test/helpers.lua

@@ -627,6 +627,19 @@ local function table_flatten(arr)
   return result
 end
 
+-- Checks if a list-like (vector) table contains `value`.
+local function table_contains(t, value)
+  if type(t) ~= 'table' then
+    error('t must be a table')
+  end
+  for _,v in ipairs(t) do
+    if v == value then
+      return true
+    end
+  end
+  return false
+end
+
 local function hexdump(str)
   local len = string.len(str)
   local dump = ""
@@ -771,6 +784,7 @@ local module = {
   repeated_read_cmd = repeated_read_cmd,
   shallowcopy = shallowcopy,
   sleep = sleep,
+  table_contains = table_contains,
   table_flatten = table_flatten,
   tmpname = tmpname,
   uname = uname,