Conversation
`reportError` was `???`, so any notification whose params failed to decode raised a NotImplementedError that took down the whole channel instead of being reported to the peer. Notifications carry no call id to respond to, so the protocol error goes out with a null id - the same thing the dispatcher already does for the other notification-level protocol errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
FS2Channel.reportErroris???, so any notification whose params fail to decode raises aNotImplementedErrorthat takes down the whole channel, rather than reporting the error to the peer.The dispatcher routes notification decode failures there:
Notifications carry no call id to respond to, so this sends the protocol error with a null id — the same thing the dispatcher already does for the other notification-level protocol errors (
sendProtocolError(pError)).How I hit this
Driving a langoustine-based LSP server:
exit(())serializes itsUnitparams asnull, which comes back as an undecodable payload for the endpoint, and the server died withNotImplementedErrorinstead of shutting down cleanly.Test
The added test sends a notification with params that don't match the endpoint's input type, then sends a second, valid notification to a different endpoint. Without the fix it fails with
NotImplementedError: an implementation is missing; with it, the channel survives and serves the second notification.Draft because
FutureBaseChannel.reportErroris also???— I left it alone since that file has several other???s and looks unfinished, but happy to fill it in too if you'd like it in the same PR.🤖 Generated with Claude Code