fix(api): generic error messages, not using TRY_WRAP #31596

Problem:
- API functions using `try_start` directly, do not surface the
  underlying error message, and instead show generic messages.
- Error-handling code is duplicated in the API impl.
- Failure modes are not tested.

Solution:
- Use `TRY_WRAP`.
- Add tests.

runtime/doc/api.txt

@@ -1647,11 +1647,9 @@ nvim_command({command})                                       *nvim_command()*
 
     On execution error: fails with Vimscript error, updates v:errmsg.
 
-    Prefer using |nvim_cmd()| or |nvim_exec2()| over this. To evaluate
+    Prefer |nvim_cmd()| or |nvim_exec2()| instead. To modify an Ex command in
-    multiple lines of Vim script or an Ex command directly, use
+    a structured way before executing it, modify the result of
-    |nvim_exec2()|. To construct an Ex command using a structured format and
+    |nvim_parse_cmd()| then pass it to |nvim_cmd()|.
-    then execute it, use |nvim_cmd()|. To modify an Ex command before
-    evaluating it, use |nvim_parse_cmd()| in conjunction with |nvim_cmd()|.
 
     Parameters: ~
       • {command}  Ex command string

runtime/lua/vim/_meta/api.lua

@@ -885,10 +885,8 @@ function vim.api.nvim_cmd(cmd, opts) end
 ---
 --- On execution error: fails with Vimscript error, updates v:errmsg.
 ---
---- Prefer using `nvim_cmd()` or `nvim_exec2()` over this. To evaluate multiple lines of Vim script
+--- Prefer `nvim_cmd()` or `nvim_exec2()` instead. To modify an Ex command in a structured way
---- or an Ex command directly, use `nvim_exec2()`. To construct an Ex command using a structured
+--- before executing it, modify the result of `nvim_parse_cmd()` then pass it to `nvim_cmd()`.
---- format and then execute it, use `nvim_cmd()`. To modify an Ex command before evaluating it, use
---- `nvim_parse_cmd()` in conjunction with `nvim_cmd()`.
 ---
 --- @param command string Ex command string
 function vim.api.nvim_command(command) end

src/nvim/api/vim.c

@@ -669,16 +669,9 @@ void nvim_set_current_dir(String dir, Error *err)
   memcpy(string, dir.data, dir.size);
   string[dir.size] = NUL;
 
-  try_start();
+  TRY_WRAP(err, {
-
+    changedir_func(string, kCdScopeGlobal);
-  if (!changedir_func(string, kCdScopeGlobal)) {
+  });
-    if (!try_end(err)) {
-      api_set_error(err, kErrorTypeException, "Failed to change directory");
-    }
-    return;
-  }
-
-  try_end(err);
 }
 
 /// Gets the current line.
@@ -942,14 +935,9 @@ void nvim_set_current_win(Window window, Error *err)
     return;
   }
 
-  try_start();
+  TRY_WRAP(err, {
-  goto_tabpage_win(win_find_tabpage(win), win);
+    goto_tabpage_win(win_find_tabpage(win), win);
-  if (!try_end(err) && win != curwin) {
+  });
-    api_set_error(err,
-                  kErrorTypeException,
-                  "Failed to switch to window %d",
-                  window);
-  }
 }
 
 /// Creates a new, empty, unnamed buffer.
@@ -1208,14 +1196,9 @@ void nvim_set_current_tabpage(Tabpage tabpage, Error *err)
     return;
   }
 
-  try_start();
+  TRY_WRAP(err, {
-  goto_tabpage_tp(tp, true, true);
+    goto_tabpage_tp(tp, true, true);
-  if (!try_end(err) && tp != curtab) {
+  });
-    api_set_error(err,
-                  kErrorTypeException,
-                  "Failed to switch to tabpage %d",
-                  tabpage);
-  }
 }
 
 /// Pastes at cursor (in any mode), and sets "redo" so dot (|.|) will repeat the input. UIs call

src/nvim/api/vimscript.c

@@ -125,19 +125,17 @@ theend:
 ///
 /// On execution error: fails with Vimscript error, updates v:errmsg.
 ///
-/// Prefer using |nvim_cmd()| or |nvim_exec2()| over this. To evaluate multiple lines of Vim script
+/// Prefer |nvim_cmd()| or |nvim_exec2()| instead. To modify an Ex command in a structured way
-/// or an Ex command directly, use |nvim_exec2()|. To construct an Ex command using a structured
+/// before executing it, modify the result of |nvim_parse_cmd()| then pass it to |nvim_cmd()|.
-/// format and then execute it, use |nvim_cmd()|. To modify an Ex command before evaluating it, use
-/// |nvim_parse_cmd()| in conjunction with |nvim_cmd()|.
 ///
 /// @param command  Ex command string
 /// @param[out] err Error details (Vim error), if any
 void nvim_command(String command, Error *err)
   FUNC_API_SINCE(1)
 {
-  try_start();
+  TRY_WRAP(err, {
-  do_cmdline_cmd(command.data);
+    do_cmdline_cmd(command.data);
-  try_end(err);
+  });
 }
 
 /// Evaluates a Vimscript |expression|. Dicts and Lists are recursively expanded.
@@ -283,13 +281,11 @@ Object nvim_call_dict_function(Object dict, String fn, Array args, Arena *arena,
   bool mustfree = false;
   switch (dict.type) {
   case kObjectTypeString:
-    try_start();
+    TRY_WRAP(err, {
-    if (eval0(dict.data.string.data, &rettv, NULL, &EVALARG_EVALUATE) == FAIL) {
+      eval0(dict.data.string.data, &rettv, NULL, &EVALARG_EVALUATE);
-      api_set_error(err, kErrorTypeException,
+      clear_evalarg(&EVALARG_EVALUATE, NULL);
-                    "Failed to evaluate dict expression");
+    });
-    }
+    if (ERROR_SET(err)) {
-    clear_evalarg(&EVALARG_EVALUATE, NULL);
-    if (try_end(err)) {
       return rv;
     }
     // Evaluation of the string arg created a new dict or increased the

src/nvim/api/window.c

@@ -182,14 +182,9 @@ void nvim_win_set_height(Window window, Integer height, Error *err)
     return;
   }
 
-  if (height > INT_MAX || height < INT_MIN) {
+  TRY_WRAP(err, {
-    api_set_error(err, kErrorTypeValidation, "Height value outside range");
+    win_setheight_win((int)height, win);
-    return;
+  });
-  }
-
-  try_start();
-  win_setheight_win((int)height, win);
-  try_end(err);
 }
 
 /// Gets the window width
@@ -224,14 +219,9 @@ void nvim_win_set_width(Window window, Integer width, Error *err)
     return;
   }
 
-  if (width > INT_MAX || width < INT_MIN) {
+  TRY_WRAP(err, {
-    api_set_error(err, kErrorTypeValidation, "Width value outside range");
+    win_setwidth_win((int)width, win);
-    return;
+  });
-  }
-
-  try_start();
-  win_setwidth_win((int)width, win);
-  try_end(err);
 }
 
 /// Gets a window-scoped (w:) variable
@@ -436,15 +426,15 @@ Object nvim_win_call(Window window, LuaRef fun, Error *err)
   }
   tabpage_T *tabpage = win_find_tabpage(win);
 
-  try_start();
   Object res = OBJECT_INIT;
-  win_execute_T win_execute_args;
+  TRY_WRAP(err, {
-  if (win_execute_before(&win_execute_args, win, tabpage)) {
+    win_execute_T win_execute_args;
-    Array args = ARRAY_DICT_INIT;
+    if (win_execute_before(&win_execute_args, win, tabpage)) {
-    res = nlua_call_ref(fun, NULL, args, kRetLuaref, NULL, err);
+      Array args = ARRAY_DICT_INIT;
-  }
+      res = nlua_call_ref(fun, NULL, args, kRetLuaref, NULL, err);
-  win_execute_after(&win_execute_args);
+    }
-  try_end(err);
+    win_execute_after(&win_execute_args);
+  });
   return res;
 }
 

test/functional/api/vim_spec.lua

@@ -695,7 +695,7 @@ describe('API', function()
         pcall_err(request, 'nvim_call_dict_function', 42, 'f', { 1, 2 })
       )
       eq(
-        'Failed to evaluate dict expression',
+        'Vim:E121: Undefined variable: foo',
         pcall_err(request, 'nvim_call_dict_function', 'foo', 'f', { 1, 2 })
       )
       eq('dict not found', pcall_err(request, 'nvim_call_dict_function', '42', 'f', { 1, 2 }))
@@ -1957,6 +1957,16 @@ describe('API', function()
       api.nvim_set_current_win(api.nvim_list_wins()[2])
       eq(api.nvim_list_wins()[2], api.nvim_get_current_win())
     end)
+
+    it('failure modes', function()
+      n.command('split')
+
+      eq('Invalid window id: 9999', pcall_err(api.nvim_set_current_win, 9999))
+
+      -- XXX: force nvim_set_current_win to fail somehow.
+      n.command("au WinLeave * throw 'foo'")
+      eq('WinLeave Autocommands for "*": foo', pcall_err(api.nvim_set_current_win, 1000))
+    end)
   end)
 
   describe('nvim_{get,set}_current_tabpage, nvim_list_tabpages', function()
@@ -1976,6 +1986,16 @@ describe('API', function()
       eq(api.nvim_list_tabpages()[2], api.nvim_get_current_tabpage())
       eq(api.nvim_list_wins()[2], api.nvim_get_current_win())
     end)
+
+    it('failure modes', function()
+      n.command('tabnew')
+
+      eq('Invalid tabpage id: 999', pcall_err(api.nvim_set_current_tabpage, 999))
+
+      -- XXX: force nvim_set_current_tabpage to fail somehow.
+      n.command("au TabLeave * throw 'foo'")
+      eq('TabLeave Autocommands for "*": foo', pcall_err(api.nvim_set_current_tabpage, 1))
+    end)
   end)
 
   describe('nvim_get_mode', function()

test/functional/api/window_spec.lua

@@ -359,6 +359,15 @@ describe('API/win', function()
       eq(2, api.nvim_win_get_height(api.nvim_list_wins()[2]))
     end)
 
+    it('failure modes', function()
+      command('split')
+      eq('Invalid window id: 999999', pcall_err(api.nvim_win_set_height, 999999, 10))
+      eq(
+        'Wrong type for argument 2 when calling nvim_win_set_height, expecting Integer',
+        pcall_err(api.nvim_win_set_height, 0, 0.9)
+      )
+    end)
+
     it('correctly handles height=1', function()
       command('split')
       api.nvim_set_current_win(api.nvim_list_wins()[1])
@@ -409,6 +418,15 @@ describe('API/win', function()
       eq(2, api.nvim_win_get_width(api.nvim_list_wins()[2]))
     end)
 
+    it('failure modes', function()
+      command('vsplit')
+      eq('Invalid window id: 999999', pcall_err(api.nvim_win_set_width, 999999, 10))
+      eq(
+        'Wrong type for argument 2 when calling nvim_win_set_width, expecting Integer',
+        pcall_err(api.nvim_win_set_width, 0, 0.9)
+      )
+    end)
+
     it('do not cause ml_get errors with foldmethod=expr #19989', function()
       insert([[
         aaaaa

test/functional/autocmd/dirchanged_spec.lua

@@ -351,11 +351,10 @@ describe('autocmd DirChanged and DirChangedPre', function()
     eq(2, eval('g:cdprecount'))
     eq(2, eval('g:cdcount'))
 
-    local status, err = pcall(function()
+    eq(
-      request('nvim_set_current_dir', '/doesnotexist')
+      'Vim:E344: Can\'t find directory "/doesnotexist" in cdpath',
-    end)
+      t.pcall_err(request, 'nvim_set_current_dir', '/doesnotexist')
-    eq(false, status)
+    )
-    eq('Failed to change directory', string.match(err, ': (.*)'))
     eq({ directory = '/doesnotexist', scope = 'global', changed_window = false }, eval('g:evpre'))
     eq(3, eval('g:cdprecount'))
     eq(2, eval('g:cdcount'))

test/functional/lua/vim_spec.lua

@@ -3955,6 +3955,17 @@ stack traceback:
       eq(win2, val)
     end)
 
+    it('failure modes', function()
+      matches(
+        'nvim_exec2%(%): Vim:E492: Not an editor command: fooooo',
+        pcall_err(exec_lua, [[vim.api.nvim_win_call(0, function() vim.cmd 'fooooo' end)]])
+      )
+      eq(
+        'Error executing lua: [string "<nvim>"]:0: fooooo',
+        pcall_err(exec_lua, [[vim.api.nvim_win_call(0, function() error('fooooo') end)]])
+      )
+    end)
+
     it('does not cause ml_get errors with invalid visual selection', function()
       -- Add lines to the current buffer and make another window looking into an empty buffer.
       exec_lua [[