fix: allow h2 post request multiplexing by mcollina · Pull Request #5391 · nodejs/undici (original) (raw)
Summary
- allow HTTP/2 to multiplex non-idempotent requests on independent streams
- keep HTTP/2 serialization for upgrade/CONNECT setup and non-replayable streaming bodies
- add a regression test for concurrent POST requests over an established H2 session
Fixes: #5390
Tests
node --test test/issue-5390.jsnode --test test/http2-dispatcher.js test/http2-agent.jsgit diff --check
Signed-off-by: Matteo Collina hello@matteocollina.com
Codecov Report
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.26%. Comparing base (01e89e9) to head (0c84e04).
⚠️ Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@ ## main #5391 +/- ##
Coverage 93.26% 93.26%
Files 110 110
Lines 36757 36757
Hits 34283 34283
Misses 2474 2474
☔ 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.
I think the behavior change is right for H2, but I would add one more regression test with a non-buffer body, for example a Readable or async iterable POST body. The new test covers concurrent POST requests with replayable string bodies, while the removed guard also used to serialize the cases people usually classify as non-replayable. Even if retry eligibility is handled elsewhere, having a streaming-body fixture here would lock down the intended separation: H2 stream multiplexing should be allowed, but a later stream/session failure must not accidentally make the request replayable.
@mcollina thanks for quick PR. I'll also test this in on my side and report back how is this working
@mcollina I tested the PR - PR removes the non-idempotent POST busy guard, which fixes low-level undici.request() with fixed Buffer bodies. However, undici.fetch() POST with a JSON string body still serializes on H2 because the request body reaches client-h2 busy() as an async iterable with unknown body length. The remaining stream-body guard still returns busy while another H2 stream is running.
This affects ordinary fetch POST JSON requests, including POST requests whose response is text/event-stream. Both of these request types we are using in Tensorlake ts sdk. Can we relax that constraints for H2?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})