fix: allow h2 post request multiplexing by mcollina · Pull Request #5391 · nodejs/undici (original) (raw)

@mcollina

Summary

Fixes: #5390

Tests

@mcollina

Signed-off-by: Matteo Collina hello@matteocollina.com

@codecov-commenter

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:

@suzunn

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.

@thedebugger

@mcollina thanks for quick PR. I'll also test this in on my side and report back how is this working

@thedebugger

@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?

metcoder95

@metcoder95

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 }})