Skip to content

Fix retries on connection errors reported by request.end() - #1

Merged
Kikobeats merged 2 commits into
v11from
v11-fix-end-callback-retry
Sep 14, 2026
Merged

Kikobeats merged 2 commits into
v11from
v11-fix-end-callback-retry

Conversation

@Kikobeats

@Kikobeats Kikobeats commented Sep 14, 2026 •

Copy link
Copy Markdown
Owner

Backport of sindresorhus#2470 to v11. Fixes sindresorhus#2469 for got v11. Upstream won't release this fix for v11.

Problem

Node.js 24.20.0 includes nodejs/node#64847: ClientRequest#end(callback) now calls callback with the error that stopped the flush. Before, callback was a 'finish' listener and never ran on failure.

_final passed that error to callback(error), so the got Request emitted 'error' while a retry was already scheduled. The result on Node.js 24.20.0, for any retryable connection error (ENOTFOUND, ECONNREFUSED, ...):

  1. The promise rejects early with ERR_SOCKET_CLOSED_BEFORE_CONNECTION instead of the real error.
  2. The pending retry throws The onCancel handler was attached after the promise settled as an uncaught exception.
  3. The retried request's error is also uncaught, which crashes processes that don't handle uncaughtException.

Fix

In _final, an error reported by end() goes through this._beforeError(error) and _final returns without calling callback, same as sindresorhus#2470. _beforeError ignores a second error, so the failure is handled once: whichever of the request's 'error' event and the end() callback comes first goes to the retry logic.

Tests

Added to test/retry.ts:

  • The Fix retries on connection errors sindresorhus/got#2470 cases, ported to v11's API (calculateDelay, beforeRetry(options, error), streams retrying through 'retry', process.nextTick since v11 supports Node.js >=10.19):
    • the request emits the error before the end() callback receives it: 3 attempts, ECONNREFUSED, beforeError runs once
    • only the end() callback receives the error: it still retries
    • the retry after an end() error succeeds
    • an end() error doesn't emit 'finish' on the stream before the retry
  • A refused connection to a real closed port: 2 retries, then ECONNREFUSED.

Results:

Check Result
New tests without the fix 4 fail on Node.js 24.16; all 5 fail on 24.20.0, plus the uncaught crash
test/retry.ts with the fix, Node.js 16 / 24.19.0 / 24.20.0 28 of 28 pass on each
Full suite on Node.js 16 526 pass; 4 failures, the same 4 as the unfixed baseline
Full suite on Node.js 24.20.0 63 failures, versus 108 unfixed; nothing fails only with the fix
xo on the changed files no errors, same 18 warnings as baseline
html-get test/index.js on Node.js 24.20.0 using this build passes 3 of 3; crashes with stock got@11.8.6

The failures on Node.js 24.20.0 are the 2020-era tests on modern Node; progress.ts and stream.ts crash there with and without the fix, which is why those files were also compared on Node.js 16 (221 pass, 0 fail across stream, progress, cache, cancel, timeout, retry, post, hooks and promise).

CI

Adds .github/workflows/test.yml (v11 only had Travis). On v11 pushes and pull requests it builds and runs test/retry.ts on Node.js LTS and latest. First run: 28 of 28 pass on both, Node.js 24.20.0 (LTS) and 26.8.2 (latest).

The full suite isn't run in CI because the 2020-era tests fail on modern Node.js without the fix too (63 failures on Node.js 24.20.0); the Node.js 10/12/14 jobs failed too, on old environment issues in agent/https tests. The four mocked regression tests fail without the fix on any Node.js version, so this job catches a regression.

Build setup

There's no lockfile, and current dev dependencies don't compile with TypeScript 4.0.3. To reproduce:

npm install --before=2021-06-15
npm install --no-save @types/node@14.18.34
npm run build

🤖 Generated with Claude Code

https://claude.ai/code/session_01VZiY7VMgg1nEMkCezwMXEC

Kikobeats and others added 2 commits September 14, 2026 13:21
Backport of sindresorhus#2470 to v11. Fixes sindresorhus#2469.

Node.js 24.20.0 (nodejs/node#64847) calls `ClientRequest#end()` callbacks
with the error that prevented the flush. `_final` forwarded it as a
stream error while a retry was scheduled: the promise rejected with
`ERR_SOCKET_CLOSED_BEFORE_CONNECTION`, retries stopped, and the pending
retry threw `The onCancel handler was attached after the promise settled`
plus the retried request's error as uncaught exceptions.

Route the error through `_beforeError` so it is deduplicated with the
request `error` event and handled by the retry logic.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VZiY7VMgg1nEMkCezwMXEC
Builds and runs test/retry.ts on Node.js LTS and latest, where the
request.end() retry regression from nodejs/node#64847 shows up.

Dependencies are resolved as of 2021-06-15 because there is no lockfile
and newer dev dependencies do not compile with TypeScript 4.0.3.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VZiY7VMgg1nEMkCezwMXEC
@Kikobeats
Kikobeats force-pushed the v11-fix-end-callback-retry branch from e1a387b to 4e340e7 Compare September 14, 2026 20:50
@Kikobeats
Kikobeats merged commit 578a9f9 into v11 Sep 14, 2026
3 checks passed
@Kikobeats
Kikobeats deleted the v11-fix-end-callback-retry branch September 14, 2026 21:04
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.

1 participant