Retry Handler Fails When Request Has a Body

[sdelano]

Bug Description

When a body is provided to a request, such as in a PUT request, the retry handler fails with UND_ERR_REQ_CONTENT_LENGTH_MISMATCH.

Reproducible By

repro.ts:

import http from 'http';
import { Agent, RetryAgent } from 'undici';

const myDispatcher = new RetryAgent(new Agent());

async function failListener(req, res) {
  res.setHeader('content-type', 'application/json');
  res.writeHead(500);
  res.end(`{"message": "failure"}`);
}

async function makeRetryRequest() {
  const result = await fetch('http://localhost:3333/fail', {
    method: 'PUT',
    body: JSON.stringify({ foo: 'bar', baz: 'bat' }),
    headers: {
      'content-type': 'application/json',
    },
    dispatcher: myDispatcher,
  });
  console.log(result);
}

async function main() {
  http.createServer(failListener).listen(3333, makeRetryRequest);
}

main();

npx tsx repro.ts

Expected Behavior

The request should be retried and should not fail because undici fails to handle re-use of the body correctly.

Logs & Screenshots

> npx tsx repro.ts
node:internal/deps/undici/undici:11372
    Error.captureStackTrace(err, this);
          ^

TypeError: fetch failed
    at Object.fetch (node:internal/deps/undici/undici:11372:11)
    at Server.makeRetryRequest REDACTED/repro.ts:13:18) {
  cause: RequestContentLengthMismatchError: Request body length does not match content-length header
      at AsyncWriter.end REDACTED/node_modules/undici/lib/dispatcher/client-h1.js:1319:15)
      at writeIterable (REDACTED/node_modules/undici/lib/dispatcher/client-h1.js:1201:12) {
    code: 'UND_ERR_REQ_CONTENT_LENGTH_MISMATCH'
  }
}

Node.js v20.8.1

Environment

• macOS 14.4.1
• nodejs v20.8.1
• undici 6.18.0

Additional context

None

[mcollina]

This is indeed tricky. On one hand, state altering verbs should not be automatically retried, on the other hand, it's a pretty common case.

[metcoder95]

It might be better to just scope this to the safe methods only GET, HEAD, and OPTIONS instead of the idempotent ones.

Tho the issue seems to root the AsyncIterable that is made out of fetch.
As the AsyncIterable has been already consumed at its first request, when the RetryHandler attempts a new request, the content-length mismatch surfaces as undici fails to iterate over the body because it was already consumed.

Not sure what will be the course of action here, I'd say that maybe the handler can make a best attempt to detect specific bodies (Stream, AsyncIterable) and clone them and attempt to free them after successful request. Just will add the overhead of duplicated bodies

[mcollina]

I think the fix here is to avoid the use of an AsyncIterable in fetch if the full body was specified. We can save quite a bit of overhead.

[metcoder95]

FWIW I tested directly with Streams and are handled properly by the RetryHandler, but facing similar issue to as per fetch with AsyncIterator (once consumed it does not loop). So this is definitely something that the handler might need to be aware of

[ronag]

Streams should not be possible to handle correctly unless they were 0 length.