fix(test): extend cloudflare workers retry to cover full request cycle#2079
Open
Zelys-DFKH wants to merge 2 commits into
Open
fix(test): extend cloudflare workers retry to cover full request cycle#2079Zelys-DFKH wants to merge 2 commits into
Zelys-DFKH wants to merge 2 commits into
Conversation
Wrangler on Node 20 accepts the initialize handshake before it can reliably handle subsequent requests. The previous retry loop only wrapped connect(), so a successful connect followed by a failed callTool() still caused the test to fail. Extend the loop to retry the full interaction (connect + callTool + assertion) and close the client on each failed attempt before retrying.
|
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
Author
|
All checks passing, including Node 20. Ready for review whenever you get a chance. |
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.
Found this while digging into the CI failures on #2026. The Cloudflare Workers test has been flaky on Node 20 with
Error: Network connection lost.Root cause
Wrangler/miniflare reports "Ready" before it can serve all request types reliably. The initialize handshake (
client.connect()) goes through, but the subsequentclient.callTool()can still fail.Commit 5a2c8a6 added a retry loop for this, but it only covered
connect(). A successful connect followed by a failingcallTool()had no retry path.The fix
The retry loop now wraps the full interaction: connect, callTool, assertion, and close. Any failure cleans up the client and retries with a 1-second backoff, up to 5 attempts. The 30s test timeout is unchanged.
Why it's separate from #2026
#2026 kept hitting this same failure even though its change (44 lines in
packages/middleware/node) has nothing to do with the Cloudflare test. Splitting them out makes both cleaner to review. Once this lands, #2026 can rebase onto main and the Node 20 run should pass.