Skip to content

fix(streamable-http): count bare-priming-then-EOF reconnects against … - #3323

Draft
gingeekrishna wants to merge 1 commit into
modelcontextprotocol:mainfrom
gingeekrishna:fix/3307-eof-reconnect-budget
Draft

fix(streamable-http): count bare-priming-then-EOF reconnects against …#3323
gingeekrishna wants to merge 1 commit into
modelcontextprotocol:mainfrom
gingeekrishna:fix/3307-eof-reconnect-budget

Conversation

@gingeekrishna

Copy link
Copy Markdown

…the request budget

A reconnect that reaches EOF without delivering any real data (only a bare id-bearing priming event) was resetting the attempt counter to 0 instead of incrementing it. This let a server that repeatedly opened the resumable stream, emitted only a priming event, and closed again reconnect forever rather than giving up after MAX_RECONNECTION_ATTEMPTS and resolving the waiter with CONNECTION_CLOSED.

Track whether any event with non-empty data was received during the reconnect. A reconnect that made real progress (delivered a notification) still earns a fresh budget for the next reconnect; a reconnect that saw only bare priming events counts against the budget the same way a transport exception does.

Adds a regression test that drives _handle_reconnection with a mock transport returning priming-then-EOF on every reconnect and asserts the waiter resolves with CONNECTION_CLOSED after exactly MAX_RECONNECTION_ATTEMPTS attempts.

Fixes #3307

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

…the request budget

A reconnect that reaches EOF without delivering any real data (only a bare
id-bearing priming event) was resetting the attempt counter to 0 instead of
incrementing it. This let a server that repeatedly opened the resumable stream,
emitted only a priming event, and closed again reconnect forever rather than
giving up after MAX_RECONNECTION_ATTEMPTS and resolving the waiter with
CONNECTION_CLOSED.

Track whether any event with non-empty data was received during the reconnect.
A reconnect that made real progress (delivered a notification) still earns a
fresh budget for the next reconnect; a reconnect that saw only bare priming
events counts against the budget the same way a transport exception does.

Adds a regression test that drives _handle_reconnection with a mock transport
returning priming-then-EOF on every reconnect and asserts the waiter resolves
with CONNECTION_CLOSED after exactly MAX_RECONNECTION_ATTEMPTS attempts.

Fixes modelcontextprotocol#3307
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.

Streamable HTTP clean EOF reconnects can exceed the request retry budget

1 participant