fix(streamable-http): count bare-priming-then-EOF reconnects against … - #3323
fix(streamable-http): count bare-priming-then-EOF reconnects against …#3323gingeekrishna wants to merge 2 commits into
Conversation
…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
14a5906 to
b68785a
Compare
|
Thanks for the contribution. This repository only keeps pull requests open when they're linked to an issue that a maintainer has assigned to the author — CONTRIBUTING.md explains why and how we work. This PR has been closed for now because you aren't currently assigned to #3307. If a maintainer would like this change as a PR from you, they'll assign you to #3307 and this PR will reopen automatically — there's nothing more you need to do. (If you opened the issue, this PR already shows up on its timeline.) There's no need to open a new PR — this one will be reopened. While it's closed, please push any updates as new commits rather than force-pushing, since GitHub can't reopen a PR whose branch has been rewritten. Maintainers: reopening this PR, removing the |
…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
Checklist
Additional context