fix(server): treat peer resets as disconnects, not server errors (#316) - #317
Open
DukeLog wants to merge 1 commit into
Open
fix(server): treat peer resets as disconnects, not server errors (#316)#317DukeLog wants to merge 1 commit into
DukeLog wants to merge 1 commit into
Conversation
A client that goes away without sending a Close frame (killed CLI, closed terminal, suspended machine) makes libuv report ECONNRESET on the next read. tcp.lua escalated every read error to server.on_error, which logs at ERROR level and therefore raises a vim.notify(ERROR). Selection changes are broadcast to every connected client, so each selection that probed a dead socket interrupted the user with a hit-enter prompt, once per stale client. The disconnect itself was already handled correctly on the same branch, so the notification was pure noise. Handle ECONNRESET and ECONNABORTED like EOF: disconnect the client with 1006 and skip on_error. Every other read error still surfaces as before. Stream callbacks pass the bare error name, but luv also formats errors as "NAME: message", so match on the leading name and keep the whole string as the disconnect reason. Fixes coder#316
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #316. Making a visual selection pops up
and a
Press ENTER or type command to continueprompt, once per stale client.Cause
Selection changes are broadcast to every connected client (
selection.lua->server/init.lua:398->tcp.lua:296). When a client has already gone away without sending a Close frame (killed CLI, closed terminal, suspended machine), that write resets the connection and libuv reportsECONNRESETto the read callback.tcp.luatreats every read error as a server fault and routes it toserver.on_error, whichserver/init.lua:91logs at ERROR level, i.e.vim.notify(..., ERROR). The disconnect itself is already handled correctly on the same branch, so the notification is just noise. It shows up on selections because that is what generates traffic to a dead socket.Easy to hit if you keep several Claude sessions attached to one Neovim instance and close some of them.
Fix
Handle
ECONNRESETandECONNABORTEDlike EOF: disconnect the client with 1006 and skipon_error. Every other read error,EPIPEandENOTCONNincluded, still surfaces as before.The lookup keys off the leading error name rather than the whole string. Stream callbacks currently pass the bare name (
ECONNRESET), but luv also formats errors asNAME: message, and matching only one of the two shapes would leave the branch dead if that ever changes.Testing
tests/unit/server/tcp_spec.luacover both error shapes for both codes, plusEPIPE,ENOTCONN,ETIMEDOUTandEHOSTUNREACH: no route to hostto pin down that genuine errors are still reported. The four peer-reset cases fail onmainand pass here.SO_LINGER 0to force an RST. Before: 1 disconnect, 1 error notification. After: 1 disconnect, 0 notifications.