messages: use proper multiline error message for rpcrequest and API wrappers

15 files changed, 136 insertions, 30 deletions

diff --git a/runtime/doc/eval.txt b/runtime/doc/eval.txt
index 7682a1a584..3bf75491f4 100644
--- a/runtime/doc/eval.txt
+++ b/runtime/doc/eval.txt
@@ -6078,7 +6078,7 @@ nr2char({expr} [, {utf8}])				*nr2char()*
 		characters.  nr2char(0) is a real NUL and terminates the
 		string, thus results in an empty string.
 
-nvim_...({...})						*nvim_...()* *eval-api*
+nvim_...({...})					*E5555* *nvim_...()* *eval-api*
 		Call nvim |api| functions. The type checking of arguments will
 		be stricter than for most other builtins. For instance,
 		if Integer is expected, a |Number| must be passed in, a
diff --git a/runtime/doc/ui.txt b/runtime/doc/ui.txt
index 60d55bda61..ca10edccba 100644
--- a/runtime/doc/ui.txt
+++ b/runtime/doc/ui.txt
@@ -702,6 +702,8 @@ events, which the UI must handle.
 		"echo"		|:echo| message
 		"echomsg"	|:echomsg| message
 		"echoerr"	|:echoerr| message
+		"lua_error"	Error in |:lua| code
+		"rpc_error"	Error response from |rpcrequest()|
 		"return_prompt"	|press-enter| prompt after a multiple messages
 		"quickfix"	Quickfix navigation message
 		"wmsg"		Warning ("search hit BOTTOM", |W10|, …)
diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index 12e309bfdc..58285a7716 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -6529,7 +6529,7 @@ static void api_wrapper(typval_T *argvars, typval_T *rettv, FunPtr fptr)
   Object result = fn(VIML_INTERNAL_CALL, args, &err);
 
   if (ERROR_SET(&err)) {
-    nvim_err_writeln(cstr_as_string(err.msg));
+    emsgf_multiline((const char *)e_api_error, err.msg);
     goto end;
   }
 
@@ -14118,8 +14118,11 @@ static void f_rpcrequest(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 
 
   Error err = ERROR_INIT;
-  Object result = rpc_send_call((uint64_t)argvars[0].vval.v_number,
-                                tv_get_string(&argvars[1]), args, &err);
+
+  uint64_t chan_id = (uint64_t)argvars[0].vval.v_number;
+  const char *method = tv_get_string(&argvars[1]);
+
+  Object result = rpc_send_call(chan_id, method, args, &err);
 
   if (l_provider_call_nesting) {
     current_SID = save_current_SID;
@@ -14132,7 +14135,20 @@ static void f_rpcrequest(typval_T *argvars, typval_T *rettv, FunPtr fptr)
   }
 
   if (ERROR_SET(&err)) {
-    nvim_err_writeln(cstr_as_string(err.msg));
+    const char *name = NULL;
+    Channel *chan = find_channel(chan_id);
+    if (chan) {
+      name = rpc_client_name(chan);
+    }
+    msg_ext_set_kind("rpc_error");
+    if (name) {
+      emsgf_multiline("Error invoking '%s' on channel %"PRIu64" (%s):\n%s",
+                      method, chan_id, name, err.msg);
+    } else {
+      emsgf_multiline("Error invoking '%s' on channel %"PRIu64":\n%s",
+                      method, chan_id, err.msg);
+    }
+
     goto end;
   }
 
diff --git a/src/nvim/globals.h b/src/nvim/globals.h
index ec14ada3d2..9fa294ba87 100644
--- a/src/nvim/globals.h
+++ b/src/nvim/globals.h
@@ -1039,6 +1039,7 @@ EXTERN char_u e_au_recursive[] INIT(= N_(
 EXTERN char_u e_unsupportedoption[] INIT(= N_("E519: Option not supported"));
 EXTERN char_u e_fnametoolong[] INIT(= N_("E856: Filename too long"));
 EXTERN char_u e_float_as_string[] INIT(= N_("E806: using Float as a String"));
+
 EXTERN char_u e_autocmd_err[] INIT(=N_(
     "E5500: autocmd has thrown an exception: %s"));
 EXTERN char_u e_cmdmap_err[] INIT(=N_(
@@ -1047,6 +1048,10 @@ EXTERN char_u e_cmdmap_repeated[] INIT(=N_(
     "E5521: <Cmd> mapping must end with <CR> before second <Cmd>"));
 EXTERN char_u e_cmdmap_key[] INIT(=N_(
     "E5522: <Cmd> mapping must not include %s key"));
+
+EXTERN char_u e_api_error[] INIT(=N_(
+    "E5555: API call: %s"));
+
 EXTERN char_u e_floatonly[] INIT(=N_(
     "E5601: Cannot close window, only floating window would remain"));
 EXTERN char_u e_floatexchange[] INIT(=N_(
diff --git a/src/nvim/lua/executor.c b/src/nvim/lua/executor.c
index 72b97736fc..4e94c10283 100644
--- a/src/nvim/lua/executor.c
+++ b/src/nvim/lua/executor.c
@@ -54,6 +54,7 @@ static void nlua_error(lua_State *const lstate, const char *const msg)
   size_t len;
   const char *const str = lua_tolstring(lstate, -1, &len);
 
+  msg_ext_set_kind("lua_error");
   emsgf_multiline(msg, (int)len, str);
 
   lua_pop(lstate, 1);
diff --git a/src/nvim/msgpack_rpc/channel.c b/src/nvim/msgpack_rpc/channel.c
index 3438949e2d..2f3af22b65 100644
--- a/src/nvim/msgpack_rpc/channel.c
+++ b/src/nvim/msgpack_rpc/channel.c
@@ -687,6 +687,22 @@ Dictionary rpc_client_info(Channel *chan)
   return copy_dictionary(chan->rpc.info);
 }
 
+const char *rpc_client_name(Channel *chan)
+{
+  if (!chan->is_rpc) {
+    return NULL;
+  }
+  Dictionary info = chan->rpc.info;
+  for (size_t i = 0; i < info.size; i++) {
+    if (strequal("name", info.items[i].key.data)
+        && info.items[i].value.type == kObjectTypeString) {
+      return info.items[i].value.data.string.data;
+    }
+  }
+
+  return NULL;
+}
+
 #if MIN_LOG_LEVEL <= DEBUG_LOG_LEVEL
 #define REQ "[request]  "
 #define RES "[response] "
diff --git a/test/functional/api/server_requests_spec.lua b/test/functional/api/server_requests_spec.lua
index 7d9a8269d9..07218f11dd 100644
--- a/test/functional/api/server_requests_spec.lua
+++ b/test/functional/api/server_requests_spec.lua
@@ -10,7 +10,7 @@ local ok = helpers.ok
 local meths = helpers.meths
 local spawn, merge_args = helpers.spawn, helpers.merge_args
 local set_session = helpers.set_session
-local expect_err = helpers.expect_err
+local meth_pcall = helpers.meth_pcall
 
 describe('server -> client', function()
   local cid
@@ -221,8 +221,8 @@ describe('server -> client', function()
     end)
 
     it('returns an error if the request failed', function()
-      expect_err('Vim:Invalid method: does%-not%-exist',
-                 eval, "rpcrequest(vim, 'does-not-exist')")
+      eq({false, "Vim:Error invoking 'does-not-exist' on channel 3:\nInvalid method: does-not-exist" },
+         meth_pcall(eval, "rpcrequest(vim, 'does-not-exist')"))
     end)
   end)
 
diff --git a/test/functional/api/vim_spec.lua b/test/functional/api/vim_spec.lua
index a7d8dc59ec..69bd584ab4 100644
--- a/test/functional/api/vim_spec.lua
+++ b/test/functional/api/vim_spec.lua
@@ -892,7 +892,7 @@ describe('API', function()
       eq({info=info}, meths.get_var("opened_event"))
       eq({[1]=testinfo,[2]=stderr,[3]=info}, meths.list_chans())
       eq(info, meths.get_chan_info(3))
-      eval('rpcrequest(3, "nvim_set_client_info", "cat", {}, "remote",'..
+      eval('rpcrequest(3, "nvim_set_client_info", "amazing-cat", {}, "remote",'..
                        '{"nvim_command":{"n_args":1}},'.. -- and so on
                        '{"description":"The Amazing Cat"})')
       info = {
@@ -900,7 +900,7 @@ describe('API', function()
         id=3,
         mode='rpc',
         client = {
-          name='cat',
+          name='amazing-cat',
           version={major=0},
           type='remote',
           methods={nvim_command={n_args=1}},
@@ -909,6 +909,9 @@ describe('API', function()
       }
       eq({info=info}, meths.get_var("info_event"))
       eq({[1]=testinfo,[2]=stderr,[3]=info}, meths.list_chans())
+
+      eq({false, "Vim:Error invoking 'nvim_set_current_buf' on channel 3 (amazing-cat):\nWrong type for argument 1, expecting Buffer"},
+         meth_pcall(eval, 'rpcrequest(3, "nvim_set_current_buf", -1)'))
     end)
 
     it('works for :terminal channel', function()
diff --git a/test/functional/autocmd/autocmd_spec.lua b/test/functional/autocmd/autocmd_spec.lua
index 1eeaa62864..20538d7141 100644
--- a/test/functional/autocmd/autocmd_spec.lua
+++ b/test/functional/autocmd/autocmd_spec.lua
@@ -219,7 +219,7 @@ describe('autocmd', function()
     eq(7, eval('g:test'))
 
     -- API calls are blocked when aucmd_win is not in scope
-    eq({false, 'Vim(call):Invalid window id'},
+    eq({false, 'Vim(call):E5555: API call: Invalid window id'},
        meth_pcall(command, "call nvim_set_current_win(g:winid)"))
 
     -- second time aucmd_win is needed, a different code path is invoked
@@ -257,7 +257,7 @@ describe('autocmd', function()
     eq(0, eval('g:had_value'))
     eq(7, eval('g:test'))
 
-    eq({false, 'Vim(call):Invalid window id'},
+    eq({false, 'Vim(call):E5555: API call: Invalid window id'},
        meth_pcall(command, "call nvim_set_current_win(g:winid)"))
   end)
 end)
diff --git a/test/functional/eval/api_functions_spec.lua b/test/functional/eval/api_functions_spec.lua
index 40d06b599f..0e3a88802d 100644
--- a/test/functional/eval/api_functions_spec.lua
+++ b/test/functional/eval/api_functions_spec.lua
@@ -34,16 +34,16 @@ describe('eval-API', function()
     eq('Vim(call):E119: Not enough arguments for function: nvim_set_option', err)
 
     err = exc_exec('call nvim_buf_set_lines(1, 0, -1, [], ["list"])')
-    eq('Vim(call):Wrong type for argument 4, expecting Boolean', err)
+    eq('Vim(call):E5555: API call: Wrong type for argument 4, expecting Boolean', err)
 
     err = exc_exec('call nvim_buf_set_lines(0, 0, -1, v:true, "string")')
-    eq('Vim(call):Wrong type for argument 5, expecting ArrayOf(String)', err)
+    eq('Vim(call):E5555: API call: Wrong type for argument 5, expecting ArrayOf(String)', err)
 
     err = exc_exec('call nvim_buf_get_number("0")')
-    eq('Vim(call):Wrong type for argument 1, expecting Buffer', err)
+    eq('Vim(call):E5555: API call: Wrong type for argument 1, expecting Buffer', err)
 
     err = exc_exec('call nvim_buf_line_count(17)')
-    eq('Vim(call):Invalid buffer id', err)
+    eq('Vim(call):E5555: API call: Invalid buffer id', err)
   end)
 
 
diff --git a/test/functional/helpers.lua b/test/functional/helpers.lua
index 35084f6cff..e2639f41e7 100644
--- a/test/functional/helpers.lua
+++ b/test/functional/helpers.lua
@@ -73,7 +73,7 @@ if prepend_argv then
   nvim_argv = new_nvim_argv
 end
 
-local session, loop_running, last_error
+local session, loop_running, last_error, method_error
 
 local function get_session()
   return session
@@ -190,12 +190,21 @@ local function call_and_stop_on_error(lsession, ...)
   return result
 end
 
+local function set_method_error(err)
+  method_error = err
+end
+
 local function run_session(lsession, request_cb, notification_cb, setup_cb, timeout)
   local on_request, on_notification, on_setup
 
   if request_cb then
     function on_request(method, args)
-      return call_and_stop_on_error(lsession, request_cb, method, args)
+      method_error = nil
+      local result = call_and_stop_on_error(lsession, request_cb, method, args)
+      if method_error ~= nil then
+        return method_error, true
+      end
+      return result
     end
   end
 
@@ -806,6 +815,7 @@ local module = {
   run = run,
   run_session = run_session,
   set_session = set_session,
+  set_method_error = set_method_error,
   set_shell_powershell = set_shell_powershell,
   skip_fragile = skip_fragile,
   source = source,
diff --git a/test/functional/provider/python3_spec.lua b/test/functional/provider/python3_spec.lua
index 3a33109079..68d4d1e2a1 100644
--- a/test/functional/provider/python3_spec.lua
+++ b/test/functional/provider/python3_spec.lua
@@ -40,8 +40,8 @@ describe('python3 provider', function()
     -- mostly bogus.
     local very_long_symbol = string.rep('a', 1200)
     feed_command(':silent! py3 print('..very_long_symbol..' b)')
-    -- Truncated error message would not contain this (last) line.
-    eq('SyntaxError: invalid syntax', eval('v:errmsg'))
+    -- Error message will contain this (last) line.
+    eq('Error invoking \'python_execute\' on channel 3 (python3-script-host):\n  File "<string>", line 1\n    print('..very_long_symbol..' b)\n          '..string.rep(' ',1200)..' ^\nSyntaxError: invalid syntax', eval('v:errmsg'))
   end)
 
   it('python3_execute with nested commands', function()
diff --git a/test/functional/ui/messages_spec.lua b/test/functional/ui/messages_spec.lua
index d49d2f0316..976dbe5893 100644
--- a/test/functional/ui/messages_spec.lua
+++ b/test/functional/ui/messages_spec.lua
@@ -4,6 +4,7 @@ local clear, feed = helpers.clear, helpers.feed
 local eval = helpers.eval
 local eq = helpers.eq
 local command = helpers.command
+local set_method_error = helpers.set_method_error
 
 
 describe('ui/ext_messages', function()
@@ -631,7 +632,7 @@ describe('ui/ext_messages', function()
     eq(0, eval('&cmdheight'))
   end)
 
-  it('supports multiline messages', function()
+  it('supports multiline messages from lua', function()
     feed(':lua error("such\\nmultiline\\nerror")<cr>')
     screen:expect{grid=[[
       ^                         |
@@ -641,9 +642,61 @@ describe('ui/ext_messages', function()
       {1:~                        }|
     ]], messages={{
         content = {{'E5105: Error while calling lua chunk: [string "<VimL compiled string>"]:1: such\nmultiline\nerror', 2}},
-        kind = "emsg"
+        kind = "lua_error"
      }}}
   end)
+
+  it('supports multiline messages from rpc', function()
+    feed(':call rpcrequest(1, "test_method")<cr>')
+
+    screen:expect{grid=[[
+      ^                         |
+      {1:~                        }|
+      {1:~                        }|
+      {1:~                        }|
+      {1:~                        }|
+    ]], messages={{
+      content = {{"Error invoking 'test_method' on channel 1:\ncomplete\nerror\n\nmessage", 2}},
+      kind = "rpc_error"
+    }}, request_cb=function (name)
+      if name == "test_method" then
+        set_method_error("complete\nerror\n\nmessage")
+      end
+    end}
+  end)
+end)
+
+describe('ui/builtin messages', function()
+  local screen
+  before_each(function()
+    clear()
+    screen = Screen.new(60, 7)
+    screen:attach({rgb=true, ext_popupmenu=true})
+    screen:set_default_attr_ids({
+      [1] = {bold = true, foreground = Screen.colors.Blue1},
+      [2] = {foreground = Screen.colors.Grey100, background = Screen.colors.Red},
+      [3] = {bold = true, reverse = true},
+      [4] = {bold = true, foreground = Screen.colors.SeaGreen4},
+    })
+  end)
+
+  it('supports multiline messages from rpc', function()
+    feed(':call rpcrequest(1, "test_method")<cr>')
+
+    screen:expect{grid=[[
+      {3:                                                            }|
+      {2:Error invoking 'test_method' on channel 1:}                  |
+      {2:complete}                                                    |
+      {2:error}                                                       |
+                                                                  |
+      {2:message}                                                     |
+      {4:Press ENTER or type command to continue}^                     |
+    ]], request_cb=function (name)
+      if name == "test_method" then
+        set_method_error("complete\nerror\n\nmessage")
+      end
+    end}
+  end)
 end)
 
 describe('ui/ext_messages', function()
diff --git a/test/functional/ui/screen.lua b/test/functional/ui/screen.lua
index a81851cbba..044e4cc39c 100644
--- a/test/functional/ui/screen.lua
+++ b/test/functional/ui/screen.lua
@@ -322,7 +322,7 @@ function Screen:expect(expected, attr_ids, attr_ignore)
     assert(not (attr_ids ~= nil or attr_ignore ~= nil))
     local is_key = {grid=true, attr_ids=true, attr_ignore=true, condition=true,
                     any=true, mode=true, unchanged=true, intermediate=true,
-                    reset=true, timeout=true}
+                    reset=true, timeout=true, request_cb=true}
     for _, v in ipairs(ext_keys) do
       is_key[v] = true
     end
@@ -497,7 +497,7 @@ function Screen:_wait(check, flags)
 
     return true
   end
-  run_session(self._session, nil, notification_cb, nil, minimal_timeout)
+  run_session(self._session, flags.request_cb, notification_cb, nil, minimal_timeout)
   if not did_flush then
     err = "no flush received"
   elseif not checked then
@@ -510,7 +510,7 @@ function Screen:_wait(check, flags)
 
   if not success_seen then
     did_miminal_timeout = true
-    run_session(self._session, nil, notification_cb, nil, timeout-minimal_timeout)
+    run_session(self._session, flags.request_cb, notification_cb, nil, timeout-minimal_timeout)
   end
 
   local did_warn = false
@@ -565,12 +565,12 @@ asynchronous (feed(), nvim_input()) and synchronous API calls.
   end
 end
 
-function Screen:sleep(ms)
+function Screen:sleep(ms, request_cb)
   local function notification_cb(method, args)
     assert(method == 'redraw')
     self:_redraw(args)
   end
-  run_session(self._session, nil, notification_cb, nil, ms)
+  run_session(self._session, request_cb, notification_cb, nil, ms)
 end
 
 function Screen:_redraw(updates)
@@ -1145,8 +1145,8 @@ end
 -- Use snapshot_util({},true) to generate a text-only (no attributes) test.
 --
 -- @see Screen:redraw_debug()
-function Screen:snapshot_util(attrs, ignore)
-  self:sleep(250)
+function Screen:snapshot_util(attrs, ignore, request_cb)
+  self:sleep(250, request_cb)
   self:print_snapshot(attrs, ignore)
 end
 
diff --git a/third-party/cmake/BuildLuarocks.cmake b/third-party/cmake/BuildLuarocks.cmake
index 3fbd47bda2..de4db35bfd 100644
--- a/third-party/cmake/BuildLuarocks.cmake
+++ b/third-party/cmake/BuildLuarocks.cmake
@@ -202,7 +202,7 @@ if(USE_BUNDLED_BUSTED)
   # DEPENDS on the previous module, because Luarocks breaks if parallel.
   add_custom_command(OUTPUT ${HOSTDEPS_LIB_DIR}/luarocks/rocks/nvim-client
     COMMAND ${LUAROCKS_BINARY}
-    ARGS build nvim-client 0.1.0-1 ${LUAROCKS_BUILDARGS}
+    ARGS build nvim-client 0.2.0-1 ${LUAROCKS_BUILDARGS}
     DEPENDS luv)
   add_custom_target(nvim-client
     DEPENDS ${HOSTDEPS_LIB_DIR}/luarocks/rocks/nvim-client)