Skip to content

fix: improve EOFError exception when remote nvim crashes #589

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

Merged
merged 1 commit into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions pynvim/msgpack_rpc/event_loop/asyncio.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,14 @@ def __init__(self, on_data, on_error):
@override
def connection_made(self, transport):
"""Used to signal `asyncio.Protocol` of a successful connection."""
del transport # no-op
self._transport = transport

@override
def connection_lost(self, exc: Optional[Exception]) -> None:
"""Used to signal `asyncio.Protocol` of a lost connection."""
debug(f"connection_lost: exc = {exc}")
self._on_error(exc if exc else EOFError())
warn(f"connection_lost: exc = {exc}")

self._on_error(exc if exc else EOFError("connection_lost"))

@override
def data_received(self, data: bytes) -> None:
Expand All @@ -63,11 +64,19 @@ def data_received(self, data: bytes) -> None:
@override
def pipe_connection_lost(self, fd: int, exc: Optional[Exception]) -> None:
"""Used to signal `asyncio.SubprocessProtocol` of a lost connection."""
debug("pipe_connection_lost: fd = %s, exc = %s", fd, exc)

assert isinstance(self._transport, asyncio.SubprocessTransport)
debug_info = {'fd': fd, 'exc': exc, 'pid': self._transport.get_pid()}
warn(f"pipe_connection_lost {debug_info}")

if os.name == 'nt' and fd == 2: # stderr
# On windows, ignore piped stderr being closed immediately (#505)
return
self._on_error(exc if exc else EOFError())

# pipe_connection_lost() *may* be called before process_exited() is
# called, when a Nvim subprocess crashes (SIGABRT). Do not handle
# errors here, as errors will be handled somewhere else
# self._on_error(exc if exc else EOFError("pipe_connection_lost"))

@override
def pipe_data_received(self, fd, data):
Expand All @@ -81,8 +90,13 @@ def pipe_data_received(self, fd, data):
@override
def process_exited(self) -> None:
"""Used to signal `asyncio.SubprocessProtocol` when the child exits."""
debug("process_exited")
self._on_error(EOFError())
assert isinstance(self._transport, asyncio.SubprocessTransport)
pid = self._transport.get_pid()
return_code = self._transport.get_returncode()

warn("process_exited, pid = %s, return_code = %s", pid, return_code)
err = EOFError(f"process_exited: pid = {pid}, return_code = {return_code}")
self._on_error(err)


class AsyncioEventLoop(BaseEventLoop):
Expand Down
14 changes: 12 additions & 2 deletions pynvim/msgpack_rpc/event_loop/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,13 @@ def run(self, data_cb: Callable[[bytes], None]) -> None:
signal.signal(signal.SIGINT, default_int_handler)
self._on_data = None

# eventloop was stopped due to an error, re-raise it
# (e.g. connection lost when subprocess nvim dies)
if self._error:
# Note: traceback is not preserved and attached for some reason,
# should be somewhere from msgpack_rpc.event_loop.asyncio.Protocol
raise self._error

@abstractmethod
def _run(self) -> None:
raise NotImplementedError()
Expand Down Expand Up @@ -234,8 +241,11 @@ def _on_signal(self, signum: signal.Signals) -> None:
self.stop()

def _on_error(self, exc: Exception) -> None:
debug(str(exc))
self._error = exc
warn('on_error: %s', repr(exc))
if self._error is None:
# ignore subsequent exceptions, it's enough to raise only
# the first exception arrived
self._error = exc
self.stop()

def _on_interrupt(self) -> None:
Expand Down
Loading