Repository navigation
fix(sandbox): drop in-flight fetches once the pool gives up or the time budget passes - #9793
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
[Medium impact] The PR appears safe to merge; no blocking issues were found. SummaryBounds sandbox fetches when an execution times out, is cancelled, or loses its worker. Requests left running after a normal return get a time limit instead of being stopped immediately.
Reviews (4) · Last reviewed commit: "test(sandbox): pin that a fire-and-forge..." · Reviewed by Greptile |
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
Summary
fetch()on the app side. When it gave up on an execution (host timeout, caller abort, worker loss), the outbound request kept running. The only limit was the script-settable idle timeout, which a slow-drip server keeps resetting. The owner's execution slot was already free, so per-owner limits didn't cover these requestscancelledflag on each pending execution with anabortcontroller that fires on every give-up path. Pass it intosecureFetchWithValidation, which already honorssignalthrough DNS validation and the pinned requesttimeoutMs+ 1s grace), viaAbortSignal.any([abort, AbortSignal.timeout(...)]). The pool gives up on the execution no later than that, so the cap never fires while the execution is alive. It only bounds a fetch the script left running after returning, whose response nothing can readPromise.allof many fetches) can't tripMaxListenersExceededWarningor pile up stale redirect-hop listeners on itType of Change
Testing
test-isolated-vm-remote-e2e.ts: 4 new checks against a server that never answers:AbortSignal.any+AbortSignal.timeoutbehave identically under Node 22 and Bunisolated-vm.test.ts+isolated-vm-worker-hardening.test.ts28/28; biome,check:test-patterns,check:comment-hygiene,check:utils,check:boundaries,check:source-conventions; full gate left to CIChecklist
test-auditauthoring gate)🤖 Generated with Claude Code