fix(webfetch): keep body read inside the timeout scope to stop "context canceled"#59
Merged
Merged
Conversation
…xt canceled" fetchURL created a per-request timeout context and deferred its cancel, then returned the *http.Response. The cancel fired the moment fetchURL returned — before WebFetch read the body via readResponseBody. net/http ties the body read to the request context, so any response still streaming when Do() returned aborted with "failed to read response: context canceled". Large/slow pages (e.g. Aliyun help docs) hit this every time because their body is still in flight when Do() returns; small pages got lucky only because their body was already buffered. The error is "context canceled", not "deadline exceeded" — it was never a real timeout, just a premature cancel. Merge the request and the body read into one timeout-scoped function (fetchBody), mirroring Safari's correct localWebFetch, so the cancel cannot fire before the read. Add a regression test that streams a slow body and asserts it is read in full (reproduces the exact error against the old structure).
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.
Problem
webfetchagainst large/slow pages (reported on Aliyun help docs, e.g.https://help.aliyun.com/zh/openapi/user-guide/openapi-mcp-server-guide) fails with:The user set
timeout: 60, yet the error iscontext canceled, notcontext deadline exceeded— so it was never a real timeout. It's a premature context cancel.Root cause
fetchURLcreated a per-request timeout context anddefer cancel()'d it, then returned the*http.Response:WebFetchthen calledreadResponseBody(resp)afterfetchURLreturned.net/httpties the lifetime ofresp.Bodyto the request context, so theio.ReadAllaborts withcontext canceledfor any response still streaming whenDo()returned. Large pages trip this every time; small pages got lucky only because their body was already buffered whenDo()returned.Safari's own
localWebFetch(the support-agent path) does it correctly —Do()andReadAllshare a singledefer cancel()scope. The runner had split them across the cancel boundary.Fix
Merge the request and the body read into one timeout-scoped function (
fetchBody), so the cancel cannot fire before the read. This makes the bug class structurally impossible.processor.Processstill runs on the outer ctx (body is already in memory, must not be bound to the per-fetch timeout).Test
TestFetchBody_ReadsSlowStreamedBodystreams a body slowly (so it cannot be pre-buffered) and asserts the full body is read. Verified it reproduces the exact failure against the old split structure (failed to read response: context canceled) and passes against the fix.🤖 Generated with Claude Code