Skip to content

Implement FS2Channel.reportError - #108

Draft
kubukoz wants to merge 1 commit into
neandertech:mainfrom
kubukoz:fix-report-error
Draft

kubukoz wants to merge 1 commit into
neandertech:mainfrom
kubukoz:fix-report-error

Conversation

@kubukoz

@kubukoz kubukoz commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

FS2Channel.reportError is ???, so any notification whose params fail to decode raises a NotImplementedError that takes down the whole channel, rather than reporting the error to the peer.

The dispatcher routes notification decode failures there:

case (InputMessage.NotificationMessage(_, params), ep: NotificationEndpoint[F, in]) =>
  ep.inCodec(HCursor.fromJson(params.data)) match {
    case Right(value) => ep.run(input, value)
    case Left(value)  => reportError(Some(params), ProtocolError.ParseError(value.getMessage), ep.method)
  }

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 its Unit params as null, which comes back as an undecodable payload for the endpoint, and the server died with NotImplementedError instead 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.reportError is 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

`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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant