Skip to content

Do not mask stream POST retry ordering in client-errors #5335

@mcollina

Description

@mcollina

Problem

While working on the Node 26 / global dispatcher compatibility backport, we found that test/node-test/client-errors.js only passed on Node 26 after the test was relaxed on main to tolerate extra/reordered POST attempts.

That relaxation appears to mask a real client bug.

The strict invariant from the original test is:

  1. a streaming POST body writes "a string" and then errors with kaboom
  2. that POST fails
  3. the following queued GET reconnects and succeeds
  4. the failed POST must not be retried before the queued GET

On Node 26, with the original strict test, the server can observe a POST where it expected the queued GET. The current main test handles this by tearing down extra POST attempts, but that accepts behavior the test was meant to reject.

Root cause

In the HTTP/1 write path, undici probes Node streams before installing the writer/error handling:

if (body && typeof body.read === 'function') {
  // Try to read EOF in order to get length.
  body.read(0)
}

For user-provided Readable bodies, body.read(0) can synchronously invoke the stream's read() implementation. If that implementation pushes data and errors, the body error happens before undici has attached the request body writer/error handlers. This changes the order of events on Node 26 and lets the queued GET proceed as if it were the first request, while the original POST may still be observed/retried unexpectedly.

Suggested fix

Forward-port the v7.x fix from PR #5319:

  • remove the eager body.read(0) probe from lib/dispatcher/client-h1.js
  • restore test/node-test/client-errors.js to assert the strict order instead of tolerating additional POST attempts
  • remove the helper behavior that destroys extra POST requests in that test

The test should fail fast if the request following the failed POST is not the queued GET.

Validation from the v7.x backport

With the eager body.read(0) removed and strict client-errors expectations restored, the following passed locally:

  • Node 26: node --test --test-reporter=spec --test-timeout=180000 test/node-test/client-errors.js
  • Node 25: same test file
  • Node 20: same test file
  • npm run test:node-test

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions