API: more reliable, descriptive VimL errors #8371

justinmk · 2018-05-07T01:26:05Z

Capture abort-causing non-exception errors.
- nvim_command did this correctly already, because do_cmdline() sets msg_list. That same pattern is needed in nvim_eval and nvim_call_function, else such errors are not reported as errors to API clients.
Return VimL errors instead of generic errors for:
- nvim_call_function
- nvim_call_dict_function
- nvim_eval
Fix tests which were silently broken before this change.

This violates #6150 (because VimL errors are localized) where we agreed not to localize API errors. But that can be fixed later.

This should make nvim_call_{dict_}function() and nvim_eval() behave more correctly, as well as giving better error messages for VimL errors. "More correctly" because previously they did not fail for some types of errors.

@garyburd This should fix the weirdness you reported.

justinmk · 2018-05-07T01:29:33Z

src/nvim/message.c

@@ -504,6 +505,9 @@ int emsg(const char_u *s_)
    if (cause_errthrow((char_u *)s, severe, &ignore) == true) {
      if (!ignore) {
        did_emsg = true;
+        if (msg_first_ignored_err == NULL) {
+          msg_first_ignored_err = xstrdup(s);
+        }


This is ugly, ~~but there's no way around it that I could find.~~ call_func() does not treat non-exception, abort-triggering emsg() errors the same way as do_cmdline_cmd().

Update: we can get the desired behavior by using the msg_list = &private_msg_list pattern from do_cmdline(). Then cause_errthrow() will capture the error in the msg_list, and try_end() will notice it.

justinmk · 2018-05-07T01:36:16Z

runtime/autoload/msgpack.vim

              \. "    return ('{\"_TYPE\": v:msgpack_types.integer,'\n"
              \. "            + '\"_VAL\": [{sign},{v1},{v2},{v3}]}').format(\n"
-              \. "              sign=1 if timestamp >= 0 else -1,\n"
+              \. "              sign=1 if timestamp >= 0 else sign=-1,\n"


@ZyX-I I took a guess at how to fix this, can you help? This is the failing test:

[ ERROR ] /home/vagrant/neovim/test/functional/plugin/shada_spec.lua @ 2077: autoload/shada.vim function shada#get_binstrings works stack traceback: test/functional/helpers.lua:99: in function 'shada#get_binstrings' /home/vagrant/neovim/test/functional/plugin/shada_spec.lua:2069: in function 'getbstrings_eq' /home/vagrant/neovim/test/functional/plugin/shada_spec.lua:2083: in function </home/vagrant/neovim/test/functional/plugin/shada_spec.lua:2077>

This is kwarg("sign", ternary(cond=ge(var("timestamp"), num("0")), if_true=num("1"), if_false=num("-1"))), it is Python ternary operator used to determine value of a keyword argument. What you have here now is not going to work, not only assignment is not an expression, there is no assignment here. And I do not think this is full error message.

@ZyX-I Yes this doesn't work, I did not want to continue debugging it until getting your opinion. Do we even need s:msgpack_init_python() anymore?

Neovim does not have strptime().

@ZyX-I what did you intend with timestamp = int(timestamp.timestamp()) ? timestamp() function does not exist on that object in python2.

This fixes the python errors (but now the tests fail):

timestamp = (timestamp - datetime.datetime(1970,1,1)).total_seconds()

@justinmk There should be a method to get seconds since epoch from a timestamp (specifically .timestamp() in Python 3). Using method you tried here is not valid, datetime.datetime yields local time. E.g. datetime.datetime(1970, 1, 1).timestamp() returns -10800.0 in my system (as my timezone is UTC+3). I do not know what is appropriate in Python 2, but at least int(timestamp.strftime('%s')) seems to work (though I do not like that).

justinmk · 2018-05-07T01:49:25Z

test/functional/lua/luaeval_spec.lua

@@ -51,12 +51,12 @@ describe('luaeval()', function()
  end)
  describe('recursive lua values', function()
    it('are successfully transformed', function()
-      funcs.luaeval('rawset(_G, "d", {})')
-      funcs.luaeval('rawset(d, "d", d)')
+      command('lua rawset(_G, "d", {})')


@ZyX-I The funcs.luaeval variant gives this error:

[ RUN ] luaeval() recursive lua values are successfully transformed: ERR test/functional/helpers.lua:99: E5101: Cannot convert given lua type

Apparently it used to give the same error before, just ignored that.

justinmk · 2018-05-09T21:07:21Z

@ZyX-I changes to msgpack.vim seem to work on unix, but appveyor still fails with the similar errors I had been seeing:

  [  ERROR   ] C:/projects/neovim/test/functional\plugin\msgpack_spec.lua @ 284: autoload/msgpack.vim function msgpack#strptime works
  test\functional\helpers.lua:99: Vim(return):error caught in request handler 'python_eval ('shada_dict_strptime()',)':
  
  stack traceback:
  	test\functional\helpers.lua:99: in function 'nvim_eval'
  	C:/projects/neovim/test/functional\plugin\msgpack_spec.lua:287: in function <C:/projects/neovim/test/functional\plugin\msgpack_spec.lua:284>
  
  [  ERROR   ] C:/projects/neovim/test/functional\plugin\shada_spec.lua @ 1361: autoload/shada.vim function shada#strings_to_sd works with multiple items
  test\functional\helpers.lua:99: Vim(return):error caught in request handler 'python_eval ('shada_dict_strptime()',)':
  
  stack traceback:
  	test\functional\helpers.lua:99: in function 'request'
  	test\functional\helpers.lua:213: in function 'nvim_command'
  	C:/projects/neovim/test/functional\plugin\shada_spec.lua:1340: in function 'strings2sd_eq'
  	C:/projects/neovim/test/functional\plugin\shada_spec.lua:1362: in function <C:/projects/neovim/test/functional\plugin\shada_spec.lua:1361>

Going to require python3 on Windows for this feature.

- Return VimL errors instead of generic errors for: - nvim_call_function - nvim_call_dict_function - Fix tests which were silently broken before this change. This violates neovim#6150 where we agreed not to translate API errors. But that can be fixed later.

This condition is not perfectly reliable: (did_emsg && force_abort && !current_exception) The more proper way to check for abort-causing non-exception errors is to set up `msg_list` using the "pattern" given by do_cmdline().

Use the same pattern as nvim_call_function (_call_function).

timestamp.strftime('%s') workaround only works on unix. ref: neovim#8371 (comment)

FEATURES: 3cc7ebf #7234 built-in VimL expression parser 6a7c904 #4419 implement <Cmd> key to invoke command in any mode b836328 #7679 'startup: treat stdin as text instead of commands' 58b210e :digraphs : highlight with hl-SpecialKey #2690 7a13611 #8276 'startup: Let `-s -` read from stdin' 1e71978 events: VimSuspend, VimResume #8280 1e7d5e8 #6272 'stdpath()' f96d99a #8247 server: introduce --listen e8c39f7 #8226 insert-mode: interpret unmapped META as ESC 98e7112 msg: do not scroll entire screen (#8088) f72630b #8055 let negative 'writedelay' show all redraws 5d2dd2e win: has("wsl") on Windows Subsystem for Linux #7330 a4f6cec cmdline: CmdlineEnter and CmdlineLeave autocommands (#7422) 207b7ca #6844 channels: support buffered output and bytes sockets/stdio API: f85cbea #7917 API: buffer updates 418abfc #6743 API: list information about all channels/jobs. 36b2e3f #8375 API: nvim_get_commands 273d2cd #8329 API: Make nvim_set_option() update `:verbose set …` 8d40b36 #8371 API: more reliable/descriptive VimL errors ebb1acb #8353 API: nvim_call_dict_function 9f994bb #8004 API: nvim_list_uis 3405704 #7520 API/UI: forward option updates to UIs 911b1e4 #7821 API: improve nvim_command_output WINDOWS OS: 9cefd83 #8084, #8516 build/win: support MSVC ee4e1fd win: Fix reading content from stdin (#8267) TUI: ffb8904 #8309 TUI: add support for mouse release events in urxvt 8d5a46e #8081 TUI: implement "standout" attribute 6071637 TUI: support TERM=konsole-256color 67848c0 #7653 TUI: report TUI info with -V3 ('verbose' >= 3) 3d0ee17 TUI/rxvt: enable focus-reporting d109f56 #7640 TUI: 'term' option: reflect effective terminal behavior FIXES: ed6a113 #8273 'job-control: avoid kill-timer race' 4e02f1a #8107 'jobs: separate process-group' 451c48a terminal: flush vterm output buffer on pty output #8486 5d6732f :checkhealth fixes #8335 53f11dc #8218 'Fix errors reported by PVS' d05712f inccommand: pause :terminal redraws (#8307) 51af911 inccommand: do not execute trailing commands #8256 84359a4 terminal: resize to the max dimensions (#8249) d49c1dd #8228 Make vim_fgets() return the same values as in Vim 60e96a4 screen: winhl=Normal:Background should not override syntax (#8093) 0c59ac1 #5908 'shada: Also save numbered marks' ba87a2c cscope: ignore EINTR while reading the prompt (#8079) b1412dc #7971 ':terminal Enter/Leave should not increment jumplist' 3a5721e TUI: libtermkey: force CSI driver for mouse input #7948 6ff13d7 #7720 TUI: faster startup 1c6e956 #7862 TUI: fix resize-related segfaults a58c909 #7676 TUI: always hide cursor when flushing, never flush buffers during unibilium output 303e1df #7624 TUI: disable BCE almost always 249bdb0 #7761 mark: Make sure that jumplist item will not have zero lnum 6f41ce0 #7704 macOS: Set $LANG based on the system locale a043899 #7633 'Retry fgets on EINTR' CHANGES: ad60927 #8304 default to 'nofsync' f3f1970 #8035 defaults: 'fillchars' a6052c7 #7984 defaults: sidescroll=1 b69fa86 #7888 defaults: enable cscopeverbose 7c4bb23 defaults: do :filetype stuff unless explicitly "off" 2aa308c #5658 'Apply :lmap in macros' 8ce6393 terminal: Leave 'relativenumber' alone (#8360) e46534b #4486 refactor: Remove maxmem, maxmemtot options 131aad9 win: defaults: 'shellcmdflag', 'shellxquote' #7343 c57d315 #8031 jobwait(): return -2 on interrupt also with timeout 6452831 clipboard: macOS: fallback to tmux if pbcopy is broken #7940 300d365 #7919 Make 'langnoremap' apply directly after a map ada1956 #7880 'lua/executor: Remove lightuserdata' INTERNAL: de0a954 #7806 internal statistics for list impl dee78a4 #7708 rewrite internal list impl

justinmk commented May 7, 2018

View reviewed changes

justinmk force-pushed the api-viml-errors branch from 95b53da to a48aa7d Compare May 9, 2018 02:02

justinmk changed the title ~~API: return non-generic VimL errors~~ API: more reliable, descriptive VimL errors May 9, 2018

justinmk added 5 commits May 9, 2018 23:18

doc

34b6a3d

msgpack.vim: fix syntax errors, python2 errors

33bfea3

API: better way to capture abort-causing non-exception errors

32b0470

This condition is not perfectly reliable: (did_emsg && force_abort && !current_exception) The more proper way to check for abort-causing non-exception errors is to set up `msg_list` using the "pattern" given by do_cmdline().

API: nvim_eval(): return non-generic VimL errors

2326a4a

Use the same pattern as nvim_call_function (_call_function).

justinmk force-pushed the api-viml-errors branch 2 times, most recently from 2704cbd to 7dfdfe4 Compare May 10, 2018 00:29

justinmk added 2 commits May 10, 2018 04:01

test: API: fix tests after improved error capture

79a0d82

nvim_eval: fix memory leak

cb8ea55

justinmk force-pushed the api-viml-errors branch from 7dfdfe4 to cb8ea55 Compare May 10, 2018 02:01

msgpack.vim: require python3 on Windows

966e7ab

timestamp.strftime('%s') workaround only works on unix. ref: neovim#8371 (comment)

justinmk force-pushed the api-viml-errors branch from de59223 to 966e7ab Compare May 10, 2018 13:56

justinmk merged commit 8d40b36 into neovim:master May 10, 2018

justinmk deleted the api-viml-errors branch May 10, 2018 17:19

justinmk mentioned this pull request Oct 4, 2018

[RFC] debug: spawn child process with --inspect-brk neovim/node-client#95

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API: more reliable, descriptive VimL errors #8371

API: more reliable, descriptive VimL errors #8371

justinmk commented May 7, 2018 •

edited

Loading

justinmk May 7, 2018 •

edited

Loading

justinmk May 7, 2018

ZyX-I May 7, 2018

justinmk May 7, 2018

ZyX-I May 7, 2018 •

edited

Loading

justinmk May 7, 2018 •

edited

Loading

ZyX-I May 7, 2018

justinmk May 7, 2018

ZyX-I May 7, 2018

justinmk commented May 9, 2018 •

edited

Loading

API: more reliable, descriptive VimL errors #8371

API: more reliable, descriptive VimL errors #8371

Conversation

justinmk commented May 7, 2018 • edited Loading

justinmk May 7, 2018 • edited Loading

Choose a reason for hiding this comment

justinmk May 7, 2018

Choose a reason for hiding this comment

ZyX-I May 7, 2018

Choose a reason for hiding this comment

justinmk May 7, 2018

Choose a reason for hiding this comment

ZyX-I May 7, 2018 • edited Loading

Choose a reason for hiding this comment

justinmk May 7, 2018 • edited Loading

Choose a reason for hiding this comment

ZyX-I May 7, 2018

Choose a reason for hiding this comment

justinmk May 7, 2018

Choose a reason for hiding this comment

ZyX-I May 7, 2018

Choose a reason for hiding this comment

justinmk commented May 9, 2018 • edited Loading

justinmk commented May 7, 2018 •

edited

Loading

justinmk May 7, 2018 •

edited

Loading

ZyX-I May 7, 2018 •

edited

Loading

justinmk May 7, 2018 •

edited

Loading

justinmk commented May 9, 2018 •

edited

Loading