Skip to content

Commit f8af4aa

Browse files
amcaplanclaude
andcommitted
Add comprehensive tests for network error detection and retry behavior
Tests verify the key behaviors from the PR: 1. Certificate/TLS/SSL errors are detected as network errors (not bugs) 2. Certificate errors are NOT retried (they're permanent) 3. Transient errors (timeouts, DNS failures) ARE retried 4. All production error patterns from the PR are covered New test coverage: - isTransientNetworkError: 14 transient error patterns, blank reasons - isNetworkError: All transient errors + permanent cert/TLS/SSL errors - Retry behavior: Verifies cert errors fail immediately without retries - Edge cases: Non-Error objects, non-network errors This ensures the fix for 6,941+ production certificate errors works correctly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 14f6ab5 commit f8af4aa

File tree

1 file changed

+154
-1
lines changed

1 file changed

+154
-1
lines changed

packages/cli-kit/src/private/node/api.test.ts

Lines changed: 154 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {retryAwareRequest} from './api.js'
1+
import {retryAwareRequest, isNetworkError, isTransientNetworkError} from './api.js'
22
import {ClientError} from 'graphql-request'
33
import {describe, test, vi, expect, beforeEach, afterEach} from 'vitest'
44

@@ -342,4 +342,157 @@ describe('retryAwareRequest', () => {
342342
await expect(result).rejects.toThrowError(nonBlankUnknownReason)
343343
expect(mockRequestFn).toHaveBeenCalledTimes(1)
344344
})
345+
346+
test('does not retry certificate/TLS/SSL errors (permanent network errors)', async () => {
347+
vi.useRealTimers()
348+
const certificateErrors = [
349+
'certificate has expired',
350+
"Hostname/IP does not match certificate's altnames",
351+
'TLS handshake failed',
352+
'SSL certificate problem: unable to get local issuer certificate',
353+
]
354+
355+
for (const certError of certificateErrors) {
356+
const mockRequestFn = vi.fn().mockImplementation(() => {
357+
throw new Error(certError)
358+
})
359+
360+
const result = retryAwareRequest(
361+
{
362+
request: mockRequestFn,
363+
url: 'https://example.com/graphql.json',
364+
useNetworkLevelRetry: true,
365+
maxRetryTimeMs: 2000,
366+
},
367+
undefined,
368+
{defaultDelayMs: 10, scheduleDelay: (fn) => fn()},
369+
)
370+
371+
await expect(result).rejects.toThrowError(certError)
372+
expect(mockRequestFn).toHaveBeenCalledTimes(1)
373+
}
374+
})
375+
})
376+
377+
describe('isTransientNetworkError', () => {
378+
test('identifies transient network errors that should be retried', () => {
379+
const transientErrors = [
380+
'socket hang up',
381+
'ECONNRESET',
382+
'ECONNABORTED',
383+
'ENOTFOUND',
384+
'ENETUNREACH',
385+
'network socket disconnected',
386+
'ETIMEDOUT',
387+
'ECONNREFUSED',
388+
'EAI_AGAIN',
389+
'EPIPE',
390+
'the operation was aborted',
391+
'timeout occurred',
392+
'premature close',
393+
'getaddrinfo ENOTFOUND',
394+
]
395+
396+
for (const errorMsg of transientErrors) {
397+
expect(isTransientNetworkError(new Error(errorMsg))).toBe(true)
398+
}
399+
})
400+
401+
test('identifies blank reason network errors', () => {
402+
const blankReasonErrors = [
403+
'request to https://example.com failed, reason:',
404+
'request to https://example.com failed, reason: ',
405+
'request to https://example.com failed, reason:\n\t',
406+
]
407+
408+
for (const errorMsg of blankReasonErrors) {
409+
expect(isTransientNetworkError(new Error(errorMsg))).toBe(true)
410+
}
411+
})
412+
413+
test('does not identify certificate errors as transient (should not be retried)', () => {
414+
const permanentErrors = [
415+
'certificate has expired',
416+
'cert verification failed',
417+
'TLS handshake failed',
418+
'SSL certificate problem',
419+
"Hostname/IP does not match certificate's altnames",
420+
]
421+
422+
for (const errorMsg of permanentErrors) {
423+
expect(isTransientNetworkError(new Error(errorMsg))).toBe(false)
424+
}
425+
})
426+
427+
test('does not identify non-network errors as transient', () => {
428+
const nonNetworkErrors = [
429+
'Invalid JSON',
430+
'Syntax error',
431+
'undefined is not a function',
432+
'request failed with status 500',
433+
]
434+
435+
for (const errorMsg of nonNetworkErrors) {
436+
expect(isTransientNetworkError(new Error(errorMsg))).toBe(false)
437+
}
438+
})
439+
440+
test('returns false for non-Error objects', () => {
441+
expect(isTransientNetworkError('string error')).toBe(false)
442+
expect(isTransientNetworkError(null)).toBe(false)
443+
expect(isTransientNetworkError(undefined)).toBe(false)
444+
expect(isTransientNetworkError({message: 'ENOTFOUND'})).toBe(false)
445+
})
446+
})
447+
448+
describe('isNetworkError', () => {
449+
test('identifies all transient network errors', () => {
450+
const transientErrors = [
451+
'ECONNRESET',
452+
'ETIMEDOUT',
453+
'ENOTFOUND',
454+
'socket hang up',
455+
'premature close',
456+
]
457+
458+
for (const errorMsg of transientErrors) {
459+
expect(isNetworkError(new Error(errorMsg))).toBe(true)
460+
}
461+
})
462+
463+
test('identifies permanent network errors (certificate/TLS/SSL)', () => {
464+
const permanentErrors = [
465+
'certificate has expired',
466+
'cert verification failed',
467+
'TLS handshake failed',
468+
'SSL certificate problem',
469+
"Hostname/IP does not match certificate's altnames",
470+
'unable to verify the first certificate',
471+
'self signed certificate in certificate chain',
472+
]
473+
474+
for (const errorMsg of permanentErrors) {
475+
expect(isNetworkError(new Error(errorMsg))).toBe(true)
476+
}
477+
})
478+
479+
test('does not identify non-network errors', () => {
480+
const nonNetworkErrors = [
481+
'Invalid JSON',
482+
'Syntax error',
483+
'undefined is not a function',
484+
'request failed with status 500',
485+
]
486+
487+
for (const errorMsg of nonNetworkErrors) {
488+
expect(isNetworkError(new Error(errorMsg))).toBe(false)
489+
}
490+
})
491+
492+
test('returns false for non-Error objects', () => {
493+
expect(isNetworkError('string error')).toBe(false)
494+
expect(isNetworkError(null)).toBe(false)
495+
expect(isNetworkError(undefined)).toBe(false)
496+
expect(isNetworkError({message: 'certificate error'})).toBe(false)
497+
})
345498
})

0 commit comments

Comments
 (0)