Skip to content

Route handler-level parse errors back to peer instead of completing client futures#17

Merged
jkschneider merged 1 commit into
mainfrom
route-handler-errors-to-peer
May 6, 2026
Merged

Route handler-level parse errors back to peer instead of completing client futures#17
jkschneider merged 1 commit into
mainfrom
route-handler-errors-to-peer

Conversation

@jkschneider
Copy link
Copy Markdown
Member

@jkschneider jkschneider commented May 6, 2026

Summary

  • HeaderDelimitedMessageHandler.receive() returned JsonRpcError.invalidRequest(...) for frame and parse failures on inbound messages, and JsonRpc.bind() treated every JsonRpcError as a peer response. Combined with PR Fix silent hang when deserialization fails with no request id #14's extractId() recovering an id from malformed bytes where possible, this gave two silent failure modes:

  • if the extracted id collided with an open client request, that unrelated client future was completed exceptionally with the peer's malformed message treated as a wire error response;

  • if the extracted id was null (the more common case), the "Error with no id" branch in JsonRpc.bind failed every open client future at once.

In neither case was the actual error sent back to the peer that produced the malformed message.

This PR:

  • Introduces JsonRpcReceiveException (extends IOException) carrying the recovered id + JsonRpcError.Detail. Both delimited handlers now throw it for frame/parse failures rather than returning a JsonRpcError.
  • Adds a catch in JsonRpc.bind() that forks the error response back to the peer; openRequests is left untouched.
  • Brings NewLineDelimitedMessageHandler to symmetric error semantics: distinguishes mid-message EOF (peer died with partial bytes — throws EOFException) from parse failure on a complete frame (throws JsonRpcReceiveException with id extraction where possible).
  • Extracts the previously private extractId recovery logic from HeaderDelimitedMessageHandler into a package-private IdExtractor so both handlers share it.

Test plan

  • ./gradlew test — all 22 tests pass (1 pre-existing @Disabled)
  • JsonRpcTest.malformedInboundDoesNotCompleteOpenClientRequests — new regression test demonstrating the fix. Verified by neutralizing the new catch in JsonRpc.bind and observing the test fail (no error response sent to peer), then restoring.
  • NewLineDelimitedMessageHandlerTest — new test class (the handler had no tests previously); covers happy path, EOF between messages, mid-message EOF, malformed JSON in a complete frame, and id extraction from a partial frame.
  • HeaderDelimitedMessageHandlerTest.receiveStillReturnsInvalidRequestForNonEmptyMalformedHeader renamed to receiveThrowsForNonEmptyMalformedHeader and updated to assert the new throw contract.

…lient futures

HeaderDelimitedMessageHandler.receive() returned JsonRpcError.invalidRequest(...)
for frame and parse failures on inbound messages. JsonRpc.bind() then treated
every JsonRpcError as a peer response and dispatched it through openRequests
based on id. With PR #14's extractId() recovering an id from malformed bytes
where possible, this caused two distinct silent failure modes:

- if the extracted id collided with an open client request, that unrelated
  client future was completed exceptionally with the peer's malformed message
  treated as a wire error response;
- if the extracted id was null (the more common case), the "Error with no id"
  branch failed every open client future at once.

In neither case was the actual error sent back to the peer.

Introduce JsonRpcReceiveException carrying the recovered id and an
JsonRpcError.Detail. Both HeaderDelimitedMessageHandler and
NewLineDelimitedMessageHandler now throw it for frame and parse failures.
JsonRpc.bind() catches it and forks the error response back to the peer;
openRequests is left untouched.

NewLineDelimitedMessageHandler also now distinguishes mid-message EOF (peer
died with partial bytes — throws EOFException, matches Header) from a
parse failure on a complete frame (throws JsonRpcReceiveException with id
extraction where possible). Extracts the previously private extractId logic
into IdExtractor so both handlers share it.
@jkschneider jkschneider merged commit 51269af into main May 6, 2026
1 check passed
@jkschneider jkschneider deleted the route-handler-errors-to-peer branch May 6, 2026 23:53
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