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.

Sign up for GitHub

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

Jump to bottom

TUI Client can handle server crashes #21843

Open
theHamsta opened this issue Jan 16, 2023 · 2 comments
Open

TUI Client can handle server crashes #21843

theHamsta opened this issue Jan 16, 2023 · 2 comments
Labels
api libnvim, Nvim RPC API enhancement feature request has:plan remote remote UI, --remote commands, p2p / peer-to-peer rpc tui
Milestone
backlog

Comments

@theHamsta
Copy link
Member

theHamsta commented Jan 16, 2023 •
edited

Problem

One of the main advantages of having separate processes for backend server and TUI client is that a segfault in the server doesn't need to be fatal for the client. The client could display a message what happened, clear the terminal or even try to restart the server.

At the moment, users just observe a crash without information what happened.

Example of a crash nvim-treesitter/nvim-treesitter#4170 . In case of a SEGFAULT, users could get a link to a wiki page that explains how to obtain coredumps or debug the crashing process via gdb.

Expected behavior

I would expect to read an error message on a unexpected termination of the server and including the bad return code and if possible whether the server process was kill by a signal. Users could be informed whether there are any steps to investigate and report the issue or where to find possible logs or stderr of the child process.

OBS for instance uses a separate process for ffmpeg muxing. When it segfaults or crashes in another way you get a error message explaining what happened.

The terminal client might even have enough state information to offer to try to recover the crashed session.
@theHamsta theHamsta added the enhancement feature request label Jan 16, 2023
@justinmk justinmk added tui remote remote UI, --remote commands, p2p / peer-to-peer labels Jan 16, 2023
@justinmk justinmk added this to the backlog milestone Jan 16, 2023
@justinmk justinmk changed the title Neovim Terminal Client Could Try Handle Server Crashes TUI Client can handle server crashes Jan 16, 2023
@zeertzjq
Copy link
Member

zeertzjq commented Feb 2, 2023

A potential problem here is that it may be hard to tell a normal server exit using :cquit from a server crash. If the server exits using :cquit the client should just exit with the same exit code to keep backward-compatibility.

A possible solution is to add a "stop" UI event and make the server send it when exiting normally, so when the server exits without sending a "stop" event it has likely crashed.

zeertzjq added a commit to zeertzjq/neovim that referenced this issue Feb 2, 2023
@zeertzjq
refactor(exit): pass error message to preserve_exit()
2e28d9f 
Problem:
1. Some calls to preserve_exit() don't put a message in IObuff, so the
   IObuff print by preserve_exit() contains unrelated information.
2. If a TUI client runs out of memory or receives a deadly signal, the
   error message is shown on alternate screen and cannot be easily seen
   because the TUI exits alternate screen soon afterwards.

Solution:
Pass error message to preserve_exit() and exit alternate screen before
printing it.

Note that this doesn't fix the problem that server error messages cannot
be easily seen on exit. This is tracked in neovim#21608 and neovim#21843.
@zeertzjq zeertzjq mentioned this issue Feb 2, 2023
Merged
zeertzjq added a commit to zeertzjq/neovim that referenced this issue Feb 2, 2023
@zeertzjq
refactor(exit): pass error message to preserve_exit()
f118366 
Problem:
1. Some calls to preserve_exit() don't put a message in IObuff, so the
   IObuff printed by preserve_exit() contains unrelated information.
2. If a TUI client runs out of memory or receives a deadly signal, the
   error message is shown on alternate screen and cannot be easily seen
   because the TUI exits alternate screen soon afterwards.

Solution:
Pass error message to preserve_exit() and exit alternate screen before
printing it.

Note that this doesn't fix the problem that server error messages cannot
be easily seen on exit. This is tracked in neovim#21608 and neovim#21843.
zeertzjq added a commit that referenced this issue Feb 4, 2023
@zeertzjq
refactor(exit): pass error message to preserve_exit() (#22097)
69bb145 
Problem:
1. Some calls to preserve_exit() don't put a message in IObuff, so the
   IObuff printed by preserve_exit() contains unrelated information.
2. If a TUI client runs out of memory or receives a deadly signal, the
   error message is shown on alternate screen and cannot be easily seen
   because the TUI exits alternate screen soon afterwards.

Solution:
Pass error message to preserve_exit() and exit alternate screen before
printing it.

Note that this doesn't fix the problem that server error messages cannot
be easily seen on exit. This is tracked in #21608 and #21843.
@justinmk justinmk added has:plan api libnvim, Nvim RPC API rpc labels Feb 15, 2023
yesean pushed a commit to yesean/neovim that referenced this issue Mar 25, 2023
@zeertzjq @yesean
refactor(exit): pass error message to preserve_exit() (neovim#22097)
fe65be9 
Problem:
1. Some calls to preserve_exit() don't put a message in IObuff, so the
   IObuff printed by preserve_exit() contains unrelated information.
2. If a TUI client runs out of memory or receives a deadly signal, the
   error message is shown on alternate screen and cannot be easily seen
   because the TUI exits alternate screen soon afterwards.

Solution:
Pass error message to preserve_exit() and exit alternate screen before
printing it.

Note that this doesn't fix the problem that server error messages cannot
be easily seen on exit. This is tracked in neovim#21608 and neovim#21843.
@zeertzjq
Copy link
Member

#25288 makes it possible to tell :cquit from a crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
api libnvim, Nvim RPC API enhancement feature request has:plan remote remote UI, --remote commands, p2p / peer-to-peer rpc tui
Projects
None yet
Milestone
backlog
Development

No branches or pull requests
3 participants
@justinmk @theHamsta @zeertzjq