Skip to content

fix(cli): stop waiting for the cloud's file check when the caller aborts - #7361

Merged
miguel-heygen merged 1 commit into
mainfrom
fix/cloud-complete-abort
Nov 7, 2026
Merged

miguel-heygen merged 1 commit into
mainfrom
fix/cloud-complete-abort

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What

The wait for the cloud's file check (added in the previous change, up to 10 minutes) ignored the abort signal, so Ctrl-C during it did nothing until the budget ran out.

  • completeWithRetry takes the upload's AbortSignal; an abort during the wait ends it at once with "Stopped waiting for the cloud to finish checking the uploaded file." (the abort reason is kept as the cause).
  • The test named "two minutes" polled for about 88 s; it is now named "more than a minute".

Why

A review note on the wait change: a wait of up to 10 minutes that does not listen to opts.signal makes the CLI look hung on Ctrl-C.

Size

One function and its test file.

Test plan

  • On this head: bun run format:check exit 0; fallow audit --base origin/main --fail-on-issues exit 0 ("No issues in 55 changed files"); oxlint exit 0; tsc no error in src/cloud; vitest run src/cloud/upload: 25 passed, 3 runs in a row.
  • fails without the fix: with the abort handling removed, stops waiting when the caller aborts fails (1 failed, 4 passed in uploadComplete.test.ts).
  • fake timers: the abort test aborts one second into the wait, advances 1 ms, and expects the rejection and fewer than 6 polls.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at 2ad3a7ed. No blockers.

  • The fix is correct. sleep(ms, signal) resolves early on abort, and the new signal?.aborted check right after it throws "Stopped waiting for the cloud to finish checking the uploaded file." with cause: signal.reason. uploadAssetViaDirectUpload now passes opts.signal through. The wait is cut short at the next sleep, not after the full 10-minute budget.
  • The "fails without the fix" claim holds. I ran it locally. With the abort check removed and sleep back to no signal, stops waiting when the caller aborts times out (1 failed, 4 passed in uploadComplete.test.ts). With the fix restored, vitest run src/cloud/upload* passes 25 of 25.
  • The test rename is accurate. The 20×409 test polls for about 88 s, so "more than a minute" is right.
  • CI on this head: CI, CodeQL, regression, preview-regression, PR captures, Player perf and Comments are all green. Windows render verification was still running and doesn't touch this code.

Non-blocking nits:

  1. sleep adds its abort listener only when the signal fires later. If Ctrl-C lands while completeAssetUpload is in flight, the next sleep still waits its full poll (at most 5 s) before the check throws. A if (signal?.aborted) throw … before the sleep, or an early resolve in sleep when the signal is already aborted, would make it instant.
  2. Each sleep leaves a { once: true } listener on the signal when the timer wins. Over a 10-minute wait that's about 120 dead closures. They're harmless, but sleep could remove its listener in the timeout path.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 7, 2026
Merged via the queue into main with commit 336abaa Nov 7, 2026
70 checks passed
@miguel-heygen
miguel-heygen deleted the fix/cloud-complete-abort branch November 7, 2026 11:45
SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 7, 2026
…, heygen-com#7360, heygen-com#7347); review to PR heygen-com#7362

Adopt heygen-com#7357 and heygen-com#7361 (upload completion waits and retries), heygen-com#7360 (Chrome
start failure names its cause in check) and heygen-com#7347 (Safari perf tooling);
release v0.9.20 not applicable. Pin open heygen-com#7362 (needs its golden LFS pointer
when adopted); record issue heygen-com#7363. Monitored 126 -> 127.

Co-authored-by: Miguel Ángel <[email protected]>
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant