Skip to content

fix(cli): close asset streams when clients disconnect - #5321

Merged
jrusso1020 merged 4 commits into
heygen-com:mainfrom
user-github-me:fix/static-asset-disconnect
Oct 24, 2026
Merged

jrusso1020 merged 4 commits into
heygen-com:mainfrom
user-github-me:fix/static-asset-disconnect

Conversation

@user-github-me

@user-github-me user-github-me commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Cancelling a large download from the CLI's local project server unpipes the response but leaves its paused ReadStream and file handle open. Destroy the stream when the response closes, covering cancelled and completed requests through the same file-serving helper. Native and cached proxy assets both use this path.

The current-main integration preserves its new in-process server behavior and tests.

Validation:

  • All 51 static-server and play tests pass, including full, open-ended Range, bounded Range and proxy download cancellation.
  • Prior source and built CLI checks with real 32 MiB files confirm cancelled streams close and release their descriptors; completed 200 and 206 responses return all expected bytes.
  • Full workspace build, CLI typecheck, lint, formatting, comment gates and normal commit hooks pass.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

serveFileWithRange pipes a ReadStream into the response, and when the client disconnects pipe unpipes but leaves the paused stream and its fd open; destroying the stream on the response's close fixes that for full, ranged and cached-proxy responses, which all funnel through this helper. Main still lacks the listener. On a normal completion close fires after finish, when the stream has already ended, so the destroy is a no-op and cannot truncate output, including on the in-process socket-pair path. The test spies createReadStream, disconnects after the first chunk, and polls until the stream is closed and destroyed for all four request shapes, then checks the server still answers. Need: yes · Size: ok · Reuse: n/a · Simpler: one line, already minimal.

— Rames

@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
@jrusso1020
jrusso1020 removed this pull request from the merge queue due to a manual request Oct 24, 2026
@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
Merged via the queue into heygen-com:main with commit 410030a Oct 24, 2026
106 checks passed
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.

2 participants