Skip to content

fix(retry): skip the content-length checkpoint for HEAD and for a 206 without content-range - #5610

Open
marko1olo wants to merge 2 commits into
nodejs:mainfrom
marko1olo:fix/retry-range-checkpoints
Open

fix(retry): skip the content-length checkpoint for HEAD and for a 206 without content-range#5610
marko1olo wants to merge 2 commits into
nodejs:mainfrom
marko1olo:fix/retry-range-checkpoints

Conversation

@marko1olo

Copy link
Copy Markdown
Contributor

This relates to...

Two defects in RetryHandler's byte-range checkpoint logic. No issue filed for either.

They are in one PR because they are the same three lines of logic and the same test file, and splitting them would mean two PRs touching the same block for the same reason. Each is a separate commit, so they can be taken apart if you would rather.

Rationale

1. A HEAD response with Content-Length breaks the retry interceptor.

onResponseStart establishes a resume checkpoint from the response Content-Length without looking at the request method:

if (this.end == null) {
  const contentLength = headers['content-length']
  this.end = contentLength != null ? Number(contentLength) - 1 : null
}

A HEAD response legitimately carries Content-Length describing the body a GET would return, while delivering no body at all. So the handler expects contentLength bytes, receives none, and the request fails with RequestRetryError: Content-Range mismatch. Any HEAD through interceptors.retry() or RetryAgent against a server that sets Content-Length on HEAD — which is normal — cannot succeed.

2. A spec-compliant multipart/byteranges 206 trips an assertion.

For a multi-range request, RFC 9110 §15.3.7.2 says the 206 carries Content-Type: multipart/byteranges and no top-level Content-Range; the ranges live in the part headers. The first-206 path handles that case deliberately — it forwards the response unchanged when parseRangeHeader returns null — but parseRangeHeader never returns null for an absent header, so the guard is unreachable and the request dies on AssertionError [ERR_ASSERTION] instead.

// Range: bytes=0-3,10-14 -> 206 multipart/byteranges, no top-level Content-Range
await request(url, { headers: { range: 'bytes=0-3,10-14' }, dispatcher })
// AssertionError [ERR_ASSERTION]

An assertion failure is the wrong shape for "the server answered in a way this handler does not resume": it reads as an undici bug to the caller, and it is not recoverable.

Changes

lib/handler/retry-handler.js, two conditions:

  • skip the Content-Length checkpoint when the request method is HEAD;
  • treat a range header that yields no usable end offset as "no checkpoint", so the existing forward-unchanged path is reachable.

test/interceptors/retry.js gains a case for each. Both fail on current main.

npm run test:interceptors is 294 passing / 0 failing here, and npx eslint is clean on both files.

Features

  • Implemented a new feature

Bug Fixes

  • Fixed a bug

Status

onResponseStart derives the resume checkpoint from content-length without
looking at the method, so a HEAD gets this.end = content-length - 1 while
its body is legitimately empty. onResponseEnd then requires
this.start === this.end + 1 and throws RequestRetryError('Content-Range
mismatch'); 200 is not a retryable status, so the request fails outright.
A HEAD has no body to checkpoint, so leave this.end null.

Signed-off-by: marko1olo <marko1olo@users.noreply.github.com>
parseRangeHeader returns { start: 0, end: null, size: null } for an absent
or empty value and only returns null when a value is present and fails the
regex, so the `range == null` guard in the first-206 branch never fires for
a response that has no content-range at all. Execution falls through to
`assert(end != null && Number.isFinite(end), 'invalid content-length')`,
which throws AssertionError [ERR_ASSERTION] out of onResponseStart before
any data reaches the handler.

RFC 9110 15.3.7.2 requires a server to omit the top-level content-range on
a multipart/byteranges response, since each part carries its own, so a
compliant server answering a multi-range request trips this.

A content-range that matches the regex always yields a numeric end, so
widening the guard to range.end == null only adds the absent and empty
cases. Neither can produce a resume checkpoint, so both take the existing
pass-through and the response is forwarded untouched.

Signed-off-by: marko1olo <marko1olo@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.49%. Comparing base (1089808) to head (2658f52).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5610      +/-   ##
==========================================
- Coverage   93.50%   93.49%   -0.02%     
==========================================
  Files         110      110              
  Lines       38303    38303              
==========================================
- Hits        35816    35812       -4     
- Misses       2487     2491       +4     

☔ 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.

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.

2 participants