Skip to content

fix(utils/stream): re-acquire writer lock when pipe() throws#4988

Open
Sriharsha-dev369 wants to merge 1 commit into
honojs:mainfrom
Sriharsha-dev369:fix/streaming-pipe-writer-lock
Open

fix(utils/stream): re-acquire writer lock when pipe() throws#4988
Sriharsha-dev369 wants to merge 1 commit into
honojs:mainfrom
Sriharsha-dev369:fix/streaming-pipe-writer-lock

Conversation

@Sriharsha-dev369

@Sriharsha-dev369 Sriharsha-dev369 commented Jun 1, 2026

Copy link
Copy Markdown

Fix: Writer lock not re-acquired after pipeTo() failure

Problem

pipe() calls releaseLock() before await pipeTo() but only re-acquires
the writer on the success path. If pipeTo() rejects (source error, client
disconnect), the writer is left released and subsequent write() / close()
calls are silently lost.

Fix

  • Wraps pipeTo() in try/finally so the writer is always re-acquired
  • Adds preventAbort: true so the destination stays writable after a source
    error — without it, the re-acquired writer targets an aborted stream and
    writes are still dropped

Tests

The added tests assert that a write() after a failed pipe() is delivered;
they fail without either part of the fix.


Fixes #4981

Checklist

  • Add tests
  • Run tests
  • bun run format:fix && bun run lint:fix
  • Add TSDoc/JSDoc to document the code

@codecov

codecov Bot commented Jun 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.88%. Comparing base (8e2cccc) to head (57e7bf7).

Files with missing lines Patch % Lines
src/utils/stream.ts 60.00% 2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (60.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4988      +/-   ##
==========================================
- Coverage   78.89%   78.88%   -0.02%     
==========================================
  Files         154      154              
  Lines       10566    10569       +3     
  Branches     2213     2213              
==========================================
+ Hits         8336     8337       +1     
- Misses       2230     2232       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Sriharsha-dev369 Sriharsha-dev369 force-pushed the fix/streaming-pipe-writer-lock branch from 57e7bf7 to ef01781 Compare June 1, 2026 18:51
@Sriharsha-dev369

Sriharsha-dev369 commented Jun 1, 2026

Copy link
Copy Markdown
Author

Pushed an updated commit to fix the Codecov patch coverage — added a test covering the success path of finally. Could you approve the CI run? Thanks!

@Sriharsha-dev369 Sriharsha-dev369 force-pushed the fix/streaming-pipe-writer-lock branch from ef01781 to 4ec478b Compare June 1, 2026 20:44
@Sriharsha-dev369 Sriharsha-dev369 force-pushed the fix/streaming-pipe-writer-lock branch from 4ec478b to 8f766f1 Compare June 1, 2026 21:00
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.

StreamingApi.pipe(): WritableStream writer lock permanently lost when pipeTo() throws

1 participant