Skip to content

JSCO-75: Revert TLS retry behavior #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thejcfactor merged 1 commit into from
Jun 25, 2026
Merged

JSCO-75: Revert TLS retry behavior #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 31 additions & 25 deletions lib/errorhandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,37 +211,43 @@ export class ErrorHandler {
return false
}

// Per the RFC, only DNS and TCP-dial failures retry; anything after the
// socket is dialed (TLS handshake, cert verification) must fail fast.
return connectionRetryAllowList.has(nodeError.code)
return !connectionDenyList.has(nodeError.code)
}
}

/**
* Node `err.code`s for the DNS and TCP-dial failures the RFC marks retriable.
*
* Taken from https://github.com/sindresorhus/is-retry-allowed
* @internal
*/
const connectionRetryAllowList = new Set([
// DNS resolution failures
'EAI_AGAIN',
const connectionDenyList = new Set([
'ENOTFOUND',
// TCP-dial / connection failures
'ECONNREFUSED',
'ECONNRESET',
'ECONNABORTED',
'ETIMEDOUT',
'EHOSTUNREACH',
'EHOSTDOWN',
'ENETUNREACH',
'ENETDOWN',
'ENETRESET',
'EADDRNOTAVAIL',
// EPIPE looks like a post-dial write failure, but it only reaches here as a
// request-side error (isRequestError), meaning the peer tore down the
// connection before accepting our request (e.g. a closed keep-alive socket,
// an LB drop, or a restarting node). Nothing was processed, so retrying is
// safe. A broken pipe after the server began responding surfaces as a
// response-side error and fails fast instead.
'EPIPE',
'UNABLE_TO_GET_ISSUER_CERT',
'UNABLE_TO_GET_CRL',
'UNABLE_TO_DECRYPT_CERT_SIGNATURE',
'UNABLE_TO_DECRYPT_CRL_SIGNATURE',
'UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY',
'CERT_SIGNATURE_FAILURE',
'CRL_SIGNATURE_FAILURE',
'CERT_NOT_YET_VALID',
'CERT_HAS_EXPIRED',
'CRL_NOT_YET_VALID',
'CRL_HAS_EXPIRED',
'ERROR_IN_CERT_NOT_BEFORE_FIELD',
'ERROR_IN_CERT_NOT_AFTER_FIELD',
'ERROR_IN_CRL_LAST_UPDATE_FIELD',
'ERROR_IN_CRL_NEXT_UPDATE_FIELD',
'OUT_OF_MEM',
'DEPTH_ZERO_SELF_SIGNED_CERT',
'SELF_SIGNED_CERT_IN_CHAIN',
'UNABLE_TO_GET_ISSUER_CERT_LOCALLY',
'UNABLE_TO_VERIFY_LEAF_SIGNATURE',
'CERT_CHAIN_TOO_LONG',
'CERT_REVOKED',
'INVALID_CA',
'PATH_LENGTH_EXCEEDED',
'INVALID_PURPOSE',
'CERT_UNTRUSTED',
'CERT_REJECTED',
'HOSTNAME_MISMATCH',
])
89 changes: 0 additions & 89 deletions test/errorhandler.test.ts
View file
Open in desktop

This file was deleted.

11 changes: 6 additions & 5 deletions test/hostselection.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,10 @@ describe('#Host selection', function () {
}
})

it('rejects with a retriable ConnectionError when no records resolve', async function () {
// An empty result is a DNS failure; per the RFC it should rejoin the retry
// path, so it is wrapped as a request-side ConnectionError carrying ENOTFOUND.
it('wraps a no-records DNS result as a request-side ConnectionError', async function () {
// An empty result is a DNS failure, so it is wrapped as a request-side
// ConnectionError carrying ENOTFOUND, rejoining the request error path.
// Whether that code retries is decided separately by ErrorHandler.
stubLookup([])
const cluster = createInstance(
`https://${HOSTNAME}:${PORT}`,
Expand All @@ -136,7 +137,7 @@ describe('#Host selection', function () {
}
})

it('wraps a DNS-resolution failure as a retriable ConnectionError', async function () {
it('wraps a DNS-resolution failure as a request-side ConnectionError', async function () {
// e.g. a transient resolver failure during a rebalance.
stubLookup(() => {
const e = new Error('getaddrinfo EAI_AGAIN') as NodeJS.ErrnoException
Expand All @@ -157,7 +158,7 @@ describe('#Host selection', function () {
}
assert.instanceOf(caught, ConnectionError)
const err = caught as ConnectionError
// isRequestError + a DNS code is what ErrorHandler classifies as retriable.
// isRequestError + the original code is what ErrorHandler uses to classify the failure.
assert.isTrue(err.isRequestError)
assert.strictEqual((err.cause as NodeJS.ErrnoException).code, 'EAI_AGAIN')
} finally {
Expand Down
Loading