Repository navigation
Conversation
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.
Summary
Fixes #742.
UserWorker.fetch()waits for the request body pipe and the response together withPromise.allSettled. When the body is a branch of a tee'd stream, which is whatreq.clone()produces (the issue's reproduction, and the CLI relay until supabase/cli#6580), the pipe never settles:op_user_worker_fetch_sendreturns, whether withWorkerAlreadyRetiredbefore dispatch or with an early response from a worker that never reads the body, the Rust side cancels the request body resource.pipeTocancels its source. Cancelling one branch of a tee does not resolve until the other branch is cancelled too (ReadableStreamTeein the Streams spec), and the other branch belongs to the caller's originalRequest, which nobody reads.allSettlednever resolves, so neither doesfetch(), and the main service can neither answer nor retry.A request without a body, or one whose body is passed through as-is, rejects promptly; only the cloned body hangs. The early-response variant is the runtime side of #737.
This makes
fetch()await only the response. The body pipe keeps running in the background with its rejection swallowed as before (#374), andrequestBodyRidis closed once the response has settled so the resource does not linger in the table.fetch()has no handle to the other tee branch, so the pipe cannot be unblocked from here; not waiting for it matches what the Rust side already assumes.Validation
test_user_worker_fetch_settles_when_worker_already_retired(new fixturemain_retired_worker_fetch, which keeps a worker handle across requests) dispatches to a retired worker with no body, with the inbound body passed through, and with a cloned body; all three now reject withWorkerAlreadyRetiredwithin the deadline. Onmain, the cloned case times out.test_user_worker_fetch_settles_when_response_arrives_before_body_is_sentforwards a cloned two-chunk upload to a worker that returns 413 without reading it; the 413 now comes back as soon as it arrives. Onmain, the call times out.