Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Out-of-order Msgpack responses from remote plugin cause channel to close #19932

Closed
ddickstein opened this issue Aug 24, 2022 · 4 comments
Closed

Comments

@ddickstein
Copy link
Contributor

ddickstein commented Aug 24, 2022

Neovim version (nvim -v)

0.7.0

Vim (not Nvim) behaves the same?

N/A

Operating system/version

CentOS 7.9

Terminal name/version

xfce4-terminal 0.8.7.4

$TERM environment variable

tmux-256color

Installation

system package manager

How to reproduce the issue

In a remote plugin in a language of your choosing, create a new RPC connection with Neovim. Then send the following:

  1. [ 0, 1, "nvim_call_function", [ "rpcrequest", [ {channel id}, "rpc" ] ] ] (we receive: [ 0, 1, "rpc", [ ] ])
  2. [ 0, 2, "nvim_call_function", [ "rpcrequest", [ {channel id}, "rpc" ] ] ] (we receive: [ 0, 2, "rpc", [ ] ])
  3. [ 1, 1, nil, nil ]

Expected behavior

Neovim should continue being blocked until the remote plugin returns a response for request 2.

Actual behavior

Neovim closes the connection and the following error is logged in $NVIM_LOG_FILE:

call_set_error:615: RPC: ch {channel id} returned a response with an unknown request id. Ensure the client is properly synchronized

The Msgpack RPC spec says that responses can be given out of order, but it seems that Neovim requires responses to be given in reverse order of requests. The idea that responses should be given in this order is intuitive - operating like a call stack - but shouldn't be required per the spec.

@ddickstein ddickstein added the bug issues reporting wrong behavior label Aug 24, 2022
@bfredl
Copy link
Member

bfredl commented Aug 24, 2022

Rather, we should document that we deviate from the msgpack-rpc spec in this respect. In addition, properly supporting an async mixture of requests and notifications is going to require to deviate from the spec even more, not less.

@bfredl bfredl added documentation and removed bug issues reporting wrong behavior labels Aug 24, 2022
@zeertzjq zeertzjq added the rpc label Aug 24, 2022
@ddickstein
Copy link
Contributor Author

ddickstein commented Aug 24, 2022

In departing in this way from the Msgpack RPC spec, does Neovim guarantee that all messages, whether requests or notifications, will be processed in the order in which they are received?

@bfredl
Copy link
Member

bfredl commented Aug 24, 2022

yes.

@justinmk
Copy link
Member

#24061 mentions this in :help msgpack-rpc

justinmk added a commit that referenced this issue Jun 19, 2023
- nvim requires rpc responses in reverse order. #19932
- NVIM_APPNAME: UIs normally should NOT set this.

ref #23520
fix #24050
fix #23660
fix #23353
fix #23337
fix #22213
fix #19161
fix #18088
fix #20693
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants