neovim / pynvim

Python client and plugin host for Nvim
http://pynvim.readthedocs.io/en/latest/
Apache License 2.0
1.48k stars 118 forks source link

More problems with msgpack v1 #438

Closed somini closed 4 years ago

somini commented 4 years ago

This hanged nvim completely on hitting backspace in a denite buffer (I'm on the old Shougo/denite.nvim@67475c7, before the v3 rewrite).

Here's a traceback:

2020-04-06 22:09:23,683 [DEBUG @ session.py:_yielding_request:163] 6621 - yielding from greenlet <greenlet.greenlet object at 0x7fbc9aaf0400> to wait for response
2020-04-06 22:09:23,683 [DEBUG @ msgpack_stream.py:_on_data:58] 6621 - waiting for message...
2020-04-06 22:09:23,683 [DEBUG @ msgpack_stream.py:_on_data:63] 6621 - unpacker needs more data...
2020-04-06 22:09:23,684 [DEBUG @ msgpack_stream.py:_on_data:58] 6621 - waiting for message...
2020-04-06 22:09:23,684 [DEBUG @ msgpack_stream.py:_on_data:60] 6621 - received message: [1, 1555, None, None]
2020-04-06 22:09:23,685 [DEBUG @ async_session.py:_on_response:100] 6621 - received response: None, None
2020-04-06 22:09:23,685 [DEBUG @ session.py:response_cb:159] 6621 - response is available for greenlet <greenlet.greenlet object at 0x7fbc9aaf0400>, switching back
2020-04-06 22:09:23,685 [DEBUG @ msgpack_stream.py:send:33] 6621 - sent [0, 1556, 'nvim_call_function', ('denite#util#getchar', [False])]
2020-04-06 22:09:23,685 [DEBUG @ base.py:send:118] 6621 - Sending 'b'\x94\x00\xcd\x06\x14\xb2nvim_call_function\x92\xb3denite#util#getchar\x91\xc2''
2020-04-06 22:09:23,685 [DEBUG @ session.py:_yielding_request:163] 6621 - yielding from greenlet <greenlet.greenlet object at 0x7fbc9aaf0400> to wait for response
2020-04-06 22:09:23,685 [DEBUG @ msgpack_stream.py:_on_data:58] 6621 - waiting for message...
2020-04-06 22:09:23,686 [DEBUG @ msgpack_stream.py:_on_data:63] 6621 - unpacker needs more data...
2020-04-06 22:09:23,686 [DEBUG @ msgpack_stream.py:_on_data:58] 6621 - waiting for message...
2020-04-06 22:09:23,686 [ERROR @ base_events.py:default_exception_handler:1707] 6621 - Exception in callback _UnixReadPipeTransport._read_ready()
handle: <Handle _UnixReadPipeTransport._read_ready()>
Traceback (most recent call last):
  File "/usr/lib/python3.8/asyncio/events.py", line 81, in _run
    self._context.run(self._callback, *self._args)
  File "/usr/lib/python3.8/asyncio/unix_events.py", line 500, in _read_ready
    self._protocol.data_received(data)
  File "/usr/lib/python3.8/site-packages/pynvim/msgpack_rpc/event_loop/asyncio.py", line 60, in data_received
    self._on_data(data)
  File "/usr/lib/python3.8/site-packages/pynvim/msgpack_rpc/msgpack_stream.py", line 59, in _on_data
    msg = next(self._unpacker)
  File "msgpack/_unpacker.pyx", line 528, in msgpack._cmsgpack.Unpacker.__next__
  File "msgpack/_unpacker.pyx", line 459, in msgpack._cmsgpack.Unpacker._unpack
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x80 in position 0: invalid start byte

I have the latest v0.4.1 on the latest Arch Linux:

Downgrading to python-msgpack v0.6.2 resolves it.

See #436


From https://pypi.org/project/msgpack/#description and https://github.com/msgpack/msgpack-python/commit/5534d0c7af0114db3d27f7b96c82a7fe22ce1e40#diff-88b99bb28683bd5b7e3a204826ead112R40, I'm guessing the Unpacker needs the raw_as_bytes option to keep the same behaviour as before?

Shougo commented 4 years ago

From https://pypi.org/project/msgpack/#description and msgpack/msgpack-python@5534d0c#diff-88b99bb28683bd5b7e3a204826ead112R40, I'm guessing the Unpacker needs the raw_as_bytes option to keep the same behaviour as before?

You can change the pynvim's msgpack argument and test it to fix. pip supports install from git repository. So you should create the PR instead. Because you can reproduce the problem easily.

Shougo commented 4 years ago

https://github.com/neovim/pynvim/issues/299#issuecomment-362790688

It may be related issue.

Shougo commented 4 years ago

deoplete uses unicode_errors='surrogateescape' for it.

Shougo commented 4 years ago

https://github.com/neovim/pynvim/blob/95e6995ea92e97ed906cb22e422f3e591c72f9c5/pynvim/msgpack_rpc/msgpack_stream.py#L23

Shougo commented 4 years ago

https://github.com/neovim/pynvim/blob/3efa4878464021f2c23b311f43a599318595177d/pynvim/compat.py#L34

Python3 unicode_errors_default is surrogateescape. Why it is error??

Shougo commented 4 years ago

OH....

https://github.com/neovim/pynvim/blob/master/pynvim/msgpack_rpc/msgpack_stream.py#L24

It is broken.

It must be:

self._unpacker = Unpacker(unicode_errors=unicode_errors_default)
Shougo commented 4 years ago

@bfredl Ping.

tomtomjhj commented 4 years ago

I think my issue(https://github.com/neovim/pynvim/issues/435) is related to this. That bug is reproduced only on msgpack 1.0.0.

Shougo commented 4 years ago

@somini @tomtomjhj Please test #439.

tomtomjhj commented 4 years ago

@Shougo That works for me. Thanks!

somini commented 4 years ago

@somini @tomtomjhj Please test #439.

@Shougo Can confirm #439 fixes this, thanks for the quick turn around.

Shougo commented 4 years ago

OK.

@bfredl Can you merge the PR?

somini commented 4 years ago

What's the status of this?

Shougo commented 4 years ago

@bfredl seems no activity now... https://github.com/bfredl

somini commented 4 years ago

Ping @neovim

This has emailed 21 people. I'm sorry...

Shougo commented 4 years ago

Fixed now.

somini commented 4 years ago

I apologise, but this has been fixed and merged to master, but no new release has been done. Meaning this hasn't hit the repositories yet.

https://github.com/neovim/pynvim/compare/0.4.1...master

Since this is a breaking change, I believe a new release is in order.

Shougo commented 4 years ago

@justinmk Can you release the new version?

tjdevries commented 4 years ago

If i recall, @bfredl is the only one w/ pip permissions. I think that was the hold up (but I don't remember for sure)

Shougo commented 4 years ago

Oh..

justinmk commented 4 years ago

release steps are here: https://github.com/neovim/pynvim#release

Let's see if @bfredl replies, else I will think about what to do in a few days

somini commented 4 years ago

Bump...

tjdevries commented 4 years ago

Just in case we're stuck for a bit you can pip install from a git revision. That might solve the problem for you.

somini commented 4 years ago

Friendly bump. Personally I'm not stuck, I'm sticking with the old version.

@bfredl is still AWOL, I hope everything is all right.

somini commented 3 years ago

Friendly bump. There's still no release on pip that includes this fix, which means distros don't pickup the new release.

Shougo commented 3 years ago

Hm...

somini commented 3 years ago

Another friendly bump. This is still broken.

jamessan commented 3 years ago

Thanks for the ping. I've released a new version of pynvim.

somini commented 3 years ago

Thanks, can confirm this is now hit the Arch Linux repos:

$ pacman -Q python-{msgpack,pynvim}
python-msgpack 1.0.0-1
python-pynvim 0.4.2-1