Visitar URL original
fix(mock): drain unread stream request bodies after replying by TonyWu2333 · Pull Request #5962 · nodejs/undici · GitHub
Skip to content

fix(mock): drain unread stream request bodies after replying - #5962

Open
TonyWu2333 wants to merge 2 commits into
nodejs:mainfrom
TonyWu2333:fix/mock-drain-stream-request-body
Open

TonyWu2333 wants to merge 2 commits into
nodejs:mainfrom
TonyWu2333:fix/mock-drain-stream-request-body

Conversation

@TonyWu2333

@TonyWu2333 TonyWu2333 commented Oct 5, 2026 •

Copy link
Copy Markdown

This relates to...

Fixes #2791

Rationale

With MockAgent, pipeline() hangs if the request body is bigger than the highWaterMark. The mock only reads a stream body when the handler has onBodySent/onRequestSent, and the pipeline handler has neither, so the writable side waits on backpressure forever. Against a real server the body gets consumed, so it works there.

Changes

Features

N/A

Bug Fixes

  • After a mocked reply is sent, resume a stream request body that hasn't been read yet (readableDidRead === false). Bodies already read by a reply callback or onBodySent are left alone.
  • Errors from that body are ignored, since the response is already done. Without this, a body stream that fails would throw an uncaught exception.
  • Added two tests: a 1 MiB body through mockAgent.pipeline(), and an unread body that errors after the reply.

One thing to note: if a reply callback keeps a reference to opts.body and only reads it after the response has ended, it'll now find it drained. I don't think that's a real use case, but happy to narrow this down if you'd prefer.

Breaking Changes and Deprecations

N/A

Status

The mock never reads a stream request body unless the handler has
onBodySent/onRequestSent, so with pipeline() a body bigger than the
highWaterMark blocks on backpressure and the pipeline never finishes.
Resume the body once the reply is sent, and ignore its errors since
the response is already done.

Fixes: nodejs#2791
@codecov-commenter

codecov-commenter commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.13%. Comparing base (eedbe2d) to head (c889610).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5962      +/-   ##
==========================================
+ Coverage   86.11%   86.13%   +0.01%     
==========================================
  Files         115      115              
  Lines       45723    45734      +11     
==========================================
+ Hits        39376    39392      +16     
+ Misses       6347     6342       -5     

☔ View full report in Codecov by Harness.
📢 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.

This branch has not been deployed

No deployments
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.

Using the MockAgent and the pipeline method

3 participants