From 2301a9472e1748637fc884e060ed277c6d06c5bf Mon Sep 17 00:00:00 2001 From: Okey Amy Date: Wed, 21 Oct 2026 22:09:29 +0100 Subject: [PATCH] fix(http): name corporate-network transport failures (#433) Node fetch collapses TLS interception, DNS, closed ports, and proxy authentication into "fetch failed", and the CLI retried all of them before blaming the API. Walk the cause chain and name the real failure. TLS certificate failures and a proxy 407 now fail on the first attempt with a NODE_EXTRA_CA_CERTS / HTTPS_PROXY remedy. Non-JSON 4xx pages that do not echo our x-request-id are reported as something between the user and the API instead of being mapped to AUTH_FORBIDDEN, CLIENT_TOO_OLD, or another API verdict. doctor prints the named cause and skips the tunnel probe when /me never reached the API. JSON API answers, 408/429, and 5xx keep their existing exit codes and retry behavior. --- DOCUMENTATION.md | 19 ++ src/commands/doctor.test.ts | 69 ++++++++ src/commands/doctor.ts | 58 +++++- src/lib/errors.ts | 36 +++- src/lib/http.test.ts | 216 ++++++++++++++++++++++- src/lib/http.ts | 226 ++++++++++++++++++++++-- src/lib/transport-failure.ts | 329 +++++++++++++++++++++++++++++++++++ 7 files changed, 926 insertions(+), 27 deletions(-) create mode 100644 src/lib/transport-failure.ts diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index da696075..8e76978b 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -1535,6 +1535,25 @@ The full list is the [exit-code table](#exit-codes). On every path the same info | `14` | Client too old — the backend requires a newer CLI (HTTP 426 `CLIENT_TOO_OLD`); upgrade to proceed | | `129` / `130` / `143` | Interrupted by a signal (SIGHUP / SIGINT / SIGTERM) — `128 + signal number` | +### Corporate TLS inspection and proxy block pages + +If every command exits `10` with a message naming `NODE_EXTRA_CA_CERTS`, the +machine does not trust the certificate chain presented by the network. Node's +default trust store is its bundled Mozilla roots plus any PEM bundle named by +`NODE_EXTRA_CA_CERTS`; it does not read the operating-system store. Point that +variable at the corporate root CA (for example +`NODE_EXTRA_CA_CERTS=/path/to/corporate-root-ca.pem`). Never set +`NODE_TLS_REJECT_UNAUTHORIZED=0` — it disables certificate checks for requests +that carry the API key. + +If a proxy, firewall, or captive portal answers with an HTML `4xx` page, the CLI +reports `UNAVAILABLE` with `details.reason: "not_testsprite"`, plus the status, +content type, and page title, instead of mapping the status to an API verdict +such as `AUTH_FORBIDDEN`. Ask IT to allow the API host, and set `HTTPS_PROXY` / +`HTTP_PROXY` when the network requires an explicit proxy. A JSON API error +envelope still maps by status; `408`, `429`, and `5xx` keep their existing retry +behavior. + ### Exit 7 on `test plan generate` — timeout or unsupported? Exit `7` is a shared bucket, and on this command it has three producers. The diff --git a/src/commands/doctor.test.ts b/src/commands/doctor.test.ts index 55f3e2e7..bb9ae3f2 100644 --- a/src/commands/doctor.test.ts +++ b/src/commands/doctor.test.ts @@ -397,6 +397,75 @@ describe('runDoctor — failing checks exit non-zero', () => { expect(capture.stdout.join('\n')).toContain('GET /me failed (NOT_FOUND)'); }); + it('a TLS trust failure names NODE_EXTRA_CA_CERTS and does not blame tunnels', async () => { + writeProfile('default', { apiKey: 'sk-user-abc' }, { path: credentialsPath }); + const { capture, deps } = makeCapture(); + const fetchImpl = vi.fn(async () => { + const cause = Object.assign(new Error('self-signed certificate in certificate chain'), { + code: 'SELF_SIGNED_CERT_IN_CHAIN', + }); + throw new TypeError('fetch failed', { cause }); + }) as unknown as DoctorDeps['fetchImpl']; + const rejection = await runDoctor( + { profile: 'default', output: 'text', debug: false }, + { ...healthyDeps(credentialsPath, { fetchImpl }), ...deps }, + ).catch((error: unknown) => error); + + expect(rejection).toBeInstanceOf(CLIError); + const out = capture.stdout.join('\n'); + expect(out).toContain('GET /me failed:'); + expect(out).toContain('NODE_EXTRA_CA_CERTS'); + expect(out).toContain('skipped; the connectivity check did not reach TestSprite'); + expect(out).not.toContain('not available on this endpoint'); + // Connectivity failed: the tunnel probe must not fire a second doomed request. + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + it('an unclassified fetch rejection does not blame the tunnel endpoint', async () => { + writeProfile('default', { apiKey: 'sk-user-abc' }, { path: credentialsPath }); + const { capture, deps } = makeCapture(); + const fetchImpl = vi.fn(async () => { + const cause = Object.assign(new Error('connect EUNKNOWN'), { code: 'EUNKNOWN' }); + throw new TypeError('fetch failed', { cause }); + }) as unknown as DoctorDeps['fetchImpl']; + const rejection = await runDoctor( + { profile: 'default', output: 'text', debug: false }, + { ...healthyDeps(credentialsPath, { fetchImpl }), ...deps }, + ).catch((error: unknown) => error); + + expect(rejection).toBeInstanceOf(CLIError); + const out = capture.stdout.join('\n'); + expect(out).toContain('GET /me failed (UNAVAILABLE)'); + expect(out).toContain('skipped; the connectivity check did not reach TestSprite'); + expect(out).not.toContain('not available on this endpoint'); + // Four connectivity attempts and no fifth tunnel probe. + expect(fetchImpl).toHaveBeenCalledTimes(4); + }); + + it('an HTML 403 block page is not reported as a rejected API key', async () => { + writeProfile('default', { apiKey: 'sk-user-abc' }, { path: credentialsPath }); + const { capture, deps } = makeCapture(); + const fetchImpl = vi.fn( + async () => + new Response( + 'Website blocked by your organization', + { status: 403, headers: { 'content-type': 'text/html; charset=utf-8' } }, + ), + ) as unknown as DoctorDeps['fetchImpl']; + const rejection = await runDoctor( + { profile: 'default', output: 'text', debug: false }, + { ...healthyDeps(credentialsPath, { fetchImpl }), ...deps }, + ).catch((error: unknown) => error); + + expect(rejection).toBeInstanceOf(CLIError); + const out = capture.stdout.join('\n'); + expect(out).not.toContain('API key rejected'); + expect(out).toContain('Website blocked by your organization'); + expect(out).toContain('skipped; the connectivity check did not reach TestSprite'); + expect(out).not.toContain('not available on this endpoint'); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + it('an excluded in-range Node runtime fails the Node.js check', async () => { writeProfile('default', { apiKey: 'sk-user-abc' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index bf653043..5d167c6a 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -33,6 +33,7 @@ import { CLIError, InterruptError, RequestTimeoutError, + TransportError, localValidationError, } from '../lib/errors.js'; import type { FetchImpl } from '../lib/http.js'; @@ -43,6 +44,7 @@ import { GLOBAL_OPTS_HINT, Output, resolveOutputMode, type OutputMode } from '.. import type { MeIdentityWire } from '../lib/response-schemas.js'; import { ME_IDENTITY_SCHEMA } from '../lib/response-schemas.js'; import { isVerifySkillInstalled } from '../lib/skill-nudge.js'; +import { isTransportFailure } from '../lib/transport-failure.js'; import { emitV3RoutingAdvisory, routingLabel } from '../lib/v3-advisory.js'; import { VERSION } from '../version.js'; import { SUPPORTED_NODE_RANGE, shouldRejectNodeVersion } from '../version-guard.js'; @@ -175,6 +177,7 @@ export async function runDoctor(opts: CommonOptions, deps: DoctorDeps = {}): Pro hasKey, endpointOk: endpointCheck.status === 'ok', config, + reachedApi: connectivity.reachedApi, }), ); @@ -292,13 +295,25 @@ async function checkConnectivity( v3Enabled?: boolean; organizations?: CliOrgSummary[]; org?: CliOrgBinding; + /** False when the request never produced an answer from the API. */ + reachedApi: boolean; }> { const name = 'Connectivity'; - if (opts.dryRun) return { check: { name, status: 'warn', detail: 'skipped under --dry-run' } }; + if (opts.dryRun) + return { + check: { name, status: 'warn', detail: 'skipped under --dry-run' }, + reachedApi: false, + }; if (!ctx.hasKey) - return { check: { name, status: 'warn', detail: 'skipped; no API key to test with' } }; + return { + check: { name, status: 'warn', detail: 'skipped; no API key to test with' }, + reachedApi: false, + }; if (!ctx.endpointOk) - return { check: { name, status: 'warn', detail: 'skipped; endpoint URL is invalid' } }; + return { + check: { name, status: 'warn', detail: 'skipped; endpoint URL is invalid' }, + reachedApi: false, + }; try { const client = makeHttpClient(opts, { @@ -316,6 +331,7 @@ async function checkConnectivity( v3Enabled: me.v3Enabled, organizations: me.organizations, org: me.org, + reachedApi: true, }; } catch (error) { if (error instanceof InterruptError) throw error; @@ -325,9 +341,29 @@ async function checkConnectivity( error.code === 'AUTH_INVALID' || error.code === 'AUTH_FORBIDDEN' ) { - return { check: { name, status: 'fail', detail: `API key rejected (${error.code})` } }; + return { + check: { name, status: 'fail', detail: `API key rejected (${error.code})` }, + reachedApi: true, + }; + } + // A named transport failure never reached the API (TLS interception, DNS, + // refused port, proxy block page). Print its actual cause instead of a + // bare UNAVAILABLE code, and let the tunnel check know not to blame the + // endpoint for a request that never arrived. + if (isTransportFailure(error)) { + return { + check: { name, status: 'fail', detail: `GET /me failed: ${error.message}` }, + reachedApi: false, + }; } - return { check: { name, status: 'fail', detail: `GET /me failed (${error.code})` } }; + return { + check: { name, status: 'fail', detail: `GET /me failed (${error.code})` }, + // A fetch rejection is a TransportError with no HTTP status. Local + // validation errors (e.g. a malformed key) are not transport failures: + // rerunning the tunnel check surfaces the same local error instead of + // hiding it behind a skip. An edge 502/504 carries its status. + reachedApi: !(error instanceof TransportError) || error.httpStatus !== undefined, + }; } return { check: { @@ -335,6 +371,7 @@ async function checkConnectivity( status: 'fail', detail: `GET /me failed (${error instanceof Error ? error.message : String(error)})`, }, + reachedApi: false, }; } } @@ -357,12 +394,21 @@ async function checkConnectivity( async function checkLocalTunnel( opts: CommonOptions, deps: DoctorDeps, - ctx: { hasKey: boolean; endpointOk: boolean; config: Config }, + ctx: { hasKey: boolean; endpointOk: boolean; config: Config; reachedApi: boolean }, ): Promise { const name = 'Local tunnel'; if (opts.dryRun) return { name, status: 'warn', detail: 'skipped under --dry-run' }; if (!ctx.hasKey) return { name, status: 'warn', detail: 'skipped; no API key to test with' }; if (!ctx.endpointOk) return { name, status: 'warn', detail: 'skipped; endpoint URL is invalid' }; + // If GET /me never reached the API, this probe cannot reach it either. The + // old fall-through blamed the endpoint for not supporting tunnels — a + // misleading diagnosis of a TLS/DNS/proxy failure. Say what happened instead. + if (!ctx.reachedApi) + return { + name, + status: 'warn', + detail: 'skipped; the connectivity check did not reach TestSprite', + }; const controller = new AbortController(); const shutdownSignal = deps.shutdownSignal ?? globalShutdown.signal; diff --git a/src/lib/errors.ts b/src/lib/errors.ts index b87452ac..389701e2 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -418,16 +418,34 @@ export function localValidationError( * Network/transport failure not associated with a server envelope. Treated * as `UNAVAILABLE` for exit-code purposes per design §8.2. */ +export interface TransportErrorOptions { + /** Cause-specific remediation; falls back to the generic availability text. */ + nextAction?: string; + /** + * Machine-readable cause, e.g. `{ reason: 'tls_untrusted', cause: + * 'UNABLE_TO_VERIFY_LEAF_SIGNATURE' }`. Every transport failure names what + * went wrong so `--output json` consumers and `doctor` can tell a TLS trust + * problem from a DNS typo or a proxy block page. + */ + details?: Record; + /** HTTP status when a response did arrive (e.g. an edge 502/504). */ + httpStatus?: number; +} + export class TransportError extends ApiError { - constructor(message: string, requestId = 'local') { - super({ - code: 'UNAVAILABLE', - message, - nextAction: - 'Service is temporarily unavailable. The CLI retries with exponential backoff; if this persists, report it.', - requestId, - details: {}, - }); + constructor(message: string, requestId = 'local', options: TransportErrorOptions = {}) { + super( + { + code: 'UNAVAILABLE', + message, + nextAction: + options.nextAction ?? + 'Service is temporarily unavailable. The CLI retries with exponential backoff; if this persists, report it.', + requestId, + details: options.details ?? {}, + }, + options.httpStatus, + ); this.name = 'TransportError'; } } diff --git a/src/lib/http.test.ts b/src/lib/http.test.ts index 93cdf596..ffa3e965 100644 --- a/src/lib/http.test.ts +++ b/src/lib/http.test.ts @@ -778,6 +778,216 @@ describe('HttpClient error mapping', () => { }); }); +describe('HttpClient — TLS certificate failures', () => { + const TLS_CASES = [ + { code: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE', reason: 'tls_untrusted' }, + { code: 'SELF_SIGNED_CERT_IN_CHAIN', reason: 'tls_untrusted' }, + { code: 'DEPTH_ZERO_SELF_SIGNED_CERT', reason: 'tls_untrusted' }, + { code: 'CERT_HAS_EXPIRED', reason: 'tls_expired' }, + { code: 'ERR_TLS_CERT_ALTNAME_INVALID', reason: 'tls_hostname_mismatch' }, + ] as const; + + it.each(TLS_CASES)( + '$code fails once with exit 10 and names NODE_EXTRA_CA_CERTS', + async ({ code, reason }) => { + const fetchImpl = vi.fn(async () => { + const cause = Object.assign(new Error(`TLS failure: ${code}`), { code }); + throw new TypeError('fetch failed', { cause }); + }); + const client = makeClient(fetchImpl as unknown as typeof fetch); + const err = (await client.get('/me').catch((e: unknown) => e)) as ApiError; + + expect(err).toBeInstanceOf(TransportError); + expect(err.code).toBe('UNAVAILABLE'); + expect(err.exitCode).toBe(10); + expect(err.message).toContain('NODE_EXTRA_CA_CERTS'); + expect(err.getDetail('reason')).toBe(reason); + expect(err.getDetail('cause')).toBe(code); + // A certificate rejection cannot self-heal by retrying the same request. + expect(fetchImpl).toHaveBeenCalledTimes(1); + }, + ); + + it('keeps a named reason and cause for a closed port', async () => { + const fetchImpl = vi.fn(async () => { + const cause = Object.assign(new Error('connect ECONNREFUSED 127.0.0.1:9'), { + code: 'ECONNREFUSED', + }); + throw new TypeError('fetch failed', { cause }); + }); + const client = makeClient(fetchImpl as unknown as typeof fetch); + const err = (await client.get('/me').catch((e: unknown) => e)) as ApiError; + + expect(err).toBeInstanceOf(TransportError); + expect(err.exitCode).toBe(10); + expect(err.message).toContain('Connection refused'); + expect(err.getDetail('reason')).toBe('connection_refused'); + expect(err.getDetail('cause')).toBe('ECONNREFUSED'); + // Non-certificate causes can be transient and keep the existing budget. + expect(fetchImpl).toHaveBeenCalledTimes(4); + }); + + it.each([ + { code: 'ENOTFOUND', reason: 'dns', messagePart: 'Could not resolve' }, + { code: 'ETIMEDOUT', reason: 'timeout', messagePart: 'timed out' }, + { code: 'ECONNRESET', reason: 'connection_reset', messagePart: 'was reset' }, + { code: 'EHOSTUNREACH', reason: 'host_unreachable', messagePart: 'Could not reach' }, + { code: 'ENETUNREACH', reason: 'host_unreachable', messagePart: 'Could not reach' }, + ] as const)( + 'names $code as $reason while keeping the retry budget', + async ({ code, reason, messagePart }) => { + const fetchImpl = vi.fn(async () => { + const cause = Object.assign(new Error(`network failure: ${code}`), { code }); + throw new TypeError('fetch failed', { cause }); + }); + const client = makeClient(fetchImpl as unknown as typeof fetch); + const err = (await client.get('/me').catch((e: unknown) => e)) as ApiError; + + expect(err).toBeInstanceOf(TransportError); + expect(err.exitCode).toBe(10); + expect(err.message).toContain(messagePart); + expect(err.getDetail('reason')).toBe(reason); + expect(err.getDetail('cause')).toBe(code); + expect(fetchImpl).toHaveBeenCalledTimes(4); + }, + ); + + it('fails once when the proxy demands authentication (407)', async () => { + const fetchImpl = vi.fn(async () => { + const cause = Object.assign(new Error('Proxy Authentication Required (407)'), { + code: 'UND_ERR_INVALID_ARG', + }); + throw new TypeError('fetch failed', { cause }); + }); + const client = makeClient(fetchImpl as unknown as typeof fetch); + const err = (await client.get('/me').catch((e: unknown) => e)) as ApiError; + + expect(err).toBeInstanceOf(TransportError); + expect(err.exitCode).toBe(10); + expect(err.message).toContain('407'); + expect(err.getDetail('reason')).toBe('proxy_authentication'); + expect(err.nextAction).toContain('HTTPS_PROXY'); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); +}); + +describe('HttpClient — block pages that are not the API', () => { + const BLOCK_HTML = + 'Website blocked by your organization' + + '

Access Denied

'; + + it.each([402, 403, 404, 426] as const)( + 'a non-JSON HTTP %d is reported as not_testsprite, not an API verdict', + async status => { + const fetchImpl = vi.fn( + async () => + new Response(BLOCK_HTML, { + status, + headers: { 'content-type': 'text/html; charset=utf-8' }, + }), + ); + const client = makeClient(fetchImpl as unknown as typeof fetch); + const err = (await client.get('/tests/test_abc').catch((e: unknown) => e)) as ApiError; + + expect(err).toBeInstanceOf(ApiError); + expect(err).not.toBeInstanceOf(TransportError); + expect(err.code).toBe('UNAVAILABLE'); + expect(err.exitCode).toBe(10); + expect(err.message).toContain('Website blocked by your organization'); + expect(err.nextAction).toContain('HTTPS_PROXY'); + expect(err.details).toMatchObject({ + reason: 'not_testsprite', + httpStatus: status, + contentType: 'text/html; charset=utf-8', + pageTitle: 'Website blocked by your organization', + }); + // The page came from a proxy/firewall, not the API — no retry budget. + expect(fetchImpl).toHaveBeenCalledTimes(1); + }, + ); + + it('treats an empty 4xx body as not_testsprite too', async () => { + const fetchImpl = vi.fn(async () => new Response('', { status: 404 })); + const client = makeClient(fetchImpl as unknown as typeof fetch); + const err = (await client.get('/tests/test_abc').catch((e: unknown) => e)) as ApiError; + + expect(err.code).toBe('UNAVAILABLE'); + expect(err.exitCode).toBe(10); + expect(err.details).toMatchObject({ reason: 'not_testsprite', httpStatus: 404 }); + expect(err.getDetail('pageTitle')).toBeUndefined(); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + it('keeps the API status mapping for a valid JSON null body', async () => { + const fetchImpl = vi.fn( + async () => + new Response('null', { status: 404, headers: { 'content-type': 'application/json' } }), + ); + const client = makeClient(fetchImpl as unknown as typeof fetch); + await expect(client.get('/tests/test_abc')).rejects.toMatchObject({ + code: 'NOT_FOUND', + exitCode: 4, + }); + }); + + it('decodes entities and strips terminal controls from the page title', async () => { + const html = 'Blocked & logged ]0;pwn'; + const fetchImpl = vi.fn( + async () => new Response(html, { status: 403, headers: { 'content-type': 'text/html' } }), + ); + const client = makeClient(fetchImpl as unknown as typeof fetch); + const err = (await client.get('/tests/test_abc').catch((e: unknown) => e)) as ApiError; + + expect(err.getDetail('pageTitle')).toBe('Blocked & logged ]0;pwn'); + }); + + it('keeps the API status mapping when the response echoes our x-request-id', async () => { + const fetchImpl = vi.fn(async (_input: unknown, init?: RequestInit) => { + const headers = init?.headers as Record | undefined; + return new Response(BLOCK_HTML, { + status: 403, + headers: { + 'content-type': 'text/html; charset=utf-8', + 'x-request-id': headers?.['x-request-id'] ?? '', + }, + }); + }); + const client = makeClient(fetchImpl as unknown as typeof fetch); + const err = (await client.get('/me', { requestId: 'cli_echoed' }).catch((e: unknown) => e)) as + ApiError | undefined; + + expect(err?.code).toBe('AUTH_FORBIDDEN'); + expect(err?.exitCode).toBe(3); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + it('keeps JSON 403 (framework envelope) as AUTH_FORBIDDEN', async () => { + const fetchImpl = vi.fn(async () => errorEnvelopeResponse(403, 'AUTH_FORBIDDEN')); + const client = makeClient(fetchImpl as unknown as typeof fetch); + await expect(client.get('/me')).rejects.toMatchObject({ + code: 'AUTH_FORBIDDEN', + exitCode: 3, + }); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + it('keeps HTML 429 as RATE_LIMITED and retries it', async () => { + const fetchImpl = vi.fn( + async () => + new Response('too many requests', { + status: 429, + headers: { 'content-type': 'text/plain', 'retry-after': '1' }, + }), + ); + const client = makeClient(fetchImpl as unknown as typeof fetch); + await expect(client.get('/me')).rejects.toMatchObject({ + code: 'RATE_LIMITED', + exitCode: 11, + }); + expect(fetchImpl).toHaveBeenCalledTimes(3); + }); +}); + describe('HttpClient debug events', () => { it('emits request → response on success', async () => { const events: DebugEvent[] = []; @@ -826,7 +1036,11 @@ describe('HttpClient transport-edge statuses', () => { async status => { const fetchImpl = vi.fn(async () => new Response('proxy gateway html', { status })); const client = makeClient(fetchImpl as unknown as typeof fetch); - await expect(client.get('/me')).rejects.toBeInstanceOf(TransportError); + const err = (await client.get('/me').catch((e: unknown) => e)) as TransportError; + expect(err).toBeInstanceOf(TransportError); + // The response did arrive, so doctor can still ask whether a tunnel is + // available on this endpoint instead of treating it as unreachable. + expect(err.httpStatus).toBe(status); expect(fetchImpl).toHaveBeenCalledTimes(4); }, ); diff --git a/src/lib/http.ts b/src/lib/http.ts index 4c7562b6..649b198a 100644 --- a/src/lib/http.ts +++ b/src/lib/http.ts @@ -3,6 +3,7 @@ import * as v from 'valibot'; import type { ErrorCode } from './errors.js'; import { ApiError, InterruptError, RequestTimeoutError, TransportError } from './errors.js'; import { buildUserAgent } from './client-tag.js'; +import { classifyFetchFailure, transportFailureError } from './transport-failure.js'; import { BATCH_RERUN_RESPONSE_SCHEMA, BATCH_RUN_FRESH_RESPONSE_SCHEMA, @@ -902,7 +903,25 @@ export class HttpClient { // nested call or from a test-injected fetchImpl), pass it through unchanged // rather than re-wrapping it as a TransportError. if (err instanceof RequestTimeoutError) throw err; - const message = err instanceof Error ? err.message : String(err); + // Name the real network failure from the `fetch failed` cause chain. + // Certificate failures and an authenticating proxy are terminal: the + // same request cannot succeed on a retry, so fail on the first attempt + // instead of burning the backoff budget and reporting the misleading + // generic "Service is temporarily unavailable". + const failure = classifyFetchFailure(err, url); + if (failure?.fatal) { + this.debug({ + kind: 'error', + method, + url, + attempt, + requestId, + errorCode: 'TRANSPORT', + durationMs: Date.now() - startedAt, + }); + throw transportFailureError(failure, requestId); + } + const message = failure?.message ?? (err instanceof Error ? err.message : String(err)); this.debug({ kind: 'error', method, @@ -915,9 +934,14 @@ export class HttpClient { const decision = allowTransportRetry ? transportRetryDecision(attempt, this.random) : { retry: false, delayMs: 0 }; - if (!decision.retry) throw new TransportError(message, requestId); + if (!decision.retry) { + throw failure !== undefined + ? transportFailureError(failure, requestId) + : new TransportError(message, requestId); + } + const failureNote = failure !== undefined ? ` (${failure.reason})` : ''; this.transition( - `Network error on ${shortPath(path)} — retrying in ${Math.round(decision.delayMs / 1000)}s (attempt ${attempt})`, + `Network error on ${shortPath(path)}${failureNote} — retrying in ${Math.round(decision.delayMs / 1000)}s (attempt ${attempt})`, ); this.debug({ kind: 'retry', @@ -1008,17 +1032,44 @@ export class HttpClient { return { body: raw as T, requestId, status: response.status }; } - let rawBody: unknown; + const nonApiCandidate = isNonApiCandidateStatus(response.status); + let errorBody: ReadErrorResponseBody; try { - rawBody = await safeReadJson(response); + errorBody = await readErrorResponseBody(response, nonApiCandidate); } catch (err) { // Interrupt passthrough (see the fetch catch above). if (err instanceof InterruptError) throw err; - // safeReadJson rethrows aborts/timeouts (it swallows only non-abort parse - // errors), so a timeout fired mid-body-read on a non-OK response lands here. + // readErrorResponseBody rethrows aborts/timeouts (it swallows only + // non-abort parse errors), so a timeout fired mid-body-read on a + // non-OK response lands here. this.rethrowIfAbort(err, timeoutSignal, options.signal, requestId, effectiveSignal); throw err; } + const rawBody = errorBody.json; + + // A 4xx that is not a JSON API envelope and does not echo our request id + // did not come from TestSprite. A firewall, captive portal, WAF, or + // authenticating proxy answered instead. Reporting its status through the + // API's error catalog (403 -> AUTH_FORBIDDEN, 402 -> buy credits, ...) + // sends scripts and agents to rotate keys, top up, or upgrade for a + // request the API never received. Fail once, and name the page. + if ( + !errorBody.jsonParsed && + nonApiCandidate && + !responseEchoesRequestId(response, requestId) + ) { + this.debug({ + kind: 'error', + method, + url, + attempt, + status: response.status, + requestId, + errorCode: 'TRANSPORT', + durationMs, + }); + throw nonApiResponseError(response, requestId, errorBody.text, this.baseUrl); + } // Edge proxies / load balancers return 408/502/504 without our error // envelope on transient outages. These are transport-level retries, @@ -1039,7 +1090,9 @@ export class HttpClient { ? transportRetryDecision(attempt, this.random) : { retry: false, delayMs: 0 }; if (!decision.retry) { - throw new TransportError(`HTTP ${response.status} from ${url}`, requestId); + throw new TransportError(`HTTP ${response.status} from ${url}`, requestId, { + httpStatus: response.status, + }); } this.transition( `HTTP ${response.status} from ${shortPath(path)} — transport error, retrying in ${Math.round(decision.delayMs / 1000)}s (attempt ${attempt})`, @@ -1330,15 +1383,166 @@ function unrefTimer(timer: ReturnType): void { if (typeof unref === 'function') unref.call(timer); } -async function safeReadJson(response: Response): Promise { +interface ReadErrorResponseBody { + /** Parsed JSON body, or `null` when the body was not valid JSON. */ + json: unknown; + /** False when parsing failed, so a real JSON `null` is not mistaken for a page. */ + jsonParsed: boolean; + /** Raw body text, captured only when the response could be a non-API page. */ + text: string | undefined; +} + +/** + * Read a non-OK response body. + * + * The CLI expects API errors to be JSON. When one is not, the raw text matters: + * a proxy/firewall block page usually carries a `` that says exactly what + * happened ("Website blocked by your organization"), and that title is more + * useful than the status code. `captureText` is set only for the 4xx statuses + * that can be a non-API page, so ordinary JSON error bodies pay no extra copy. + * + * A `Response` can only be consumed once, so the text comes from a clone taken + * before `.json()`; the clone is best-effort (some runtimes/test doubles cannot + * clone), and a missing clone just means no title is reported. + */ +async function readErrorResponseBody( + response: Response, + captureText: boolean, +): Promise<ReadErrorResponseBody> { + let clone: Response | undefined; + if (captureText) { + try { + clone = response.clone(); + } catch { + clone = undefined; + } + } + try { - return await response.json(); + return { json: await response.json(), jsonParsed: true, text: undefined }; } catch (err) { // Don't swallow interrupts, aborts, or timeouts as a null body — the caller // must preserve the interruption or classify a timeout correctly. if (err instanceof InterruptError) throw err; if (isAbortError(err) || isTimeoutError(err)) throw err; - return null; + let text: string | undefined; + if (clone !== undefined) { + try { + text = await clone.text(); + } catch (readErr) { + if (readErr instanceof InterruptError) throw readErr; + if (isAbortError(readErr) || isTimeoutError(readErr)) throw readErr; + text = undefined; + } + } + return { json: null, jsonParsed: false, text }; + } +} + +/** + * 4xx statuses whose non-JSON body is treated as "something between this + * machine and the API answered". 408 and 429 keep their existing transport / + * rate-limit handling, and 5xx keeps today's retry behaviour. + */ +function isNonApiCandidateStatus(status: number): boolean { + return status >= 400 && status < 500 && status !== 408 && status !== 429; +} + +/** True when the response carries back the exact `x-request-id` we sent. */ +function responseEchoesRequestId(response: Response, requestId: string): boolean { + const echoed = response.headers.get('x-request-id'); + return echoed !== null && echoed.trim() === requestId; +} + +/** Extract and lightly clean a page title, bounded so an error stays readable. */ +function extractPageTitle(text: string | undefined): string | undefined { + if (text === undefined || text.length === 0) return undefined; + const match = /<title[^>]*>([\s\S]*?)<\/title>/i.exec(text); + if (match === null) return undefined; + const title = decodeHtmlEntities(match[1]!.replace(/<[^>]*>/g, '')) + // The page is untrusted input. Strip Unicode control characters so a + // malicious title cannot emit ANSI escapes to the terminal before the + // title is collapsed. + .replace(/\p{Cc}/gu, ' ') + .replace(/\s+/g, ' ') + .trim(); + if (title.length === 0) return undefined; + return title.length > 200 ? `${title.slice(0, 197)}...` : title; +} + +/** Decode the common HTML entities a block-page title can contain. */ +function decodeHtmlEntities(value: string): string { + const named: Record<string, string> = { + amp: '&', + lt: '<', + gt: '>', + quot: '"', + apos: "'", + nbsp: ' ', + }; + return value.replace(/&(#x[0-9a-f]+|#\d+|[a-z]+);/gi, (entity, body: string) => { + if (body.startsWith('#x') || body.startsWith('#X')) { + const code = Number.parseInt(body.slice(2), 16); + return isSafeCodePoint(code) ? String.fromCodePoint(code) : entity; + } + if (body.startsWith('#')) { + const code = Number.parseInt(body.slice(1), 10); + return isSafeCodePoint(code) ? String.fromCodePoint(code) : entity; + } + return named[body.toLowerCase()] ?? entity; + }); +} + +function isSafeCodePoint(code: number): boolean { + return Number.isFinite(code) && code >= 0 && code <= 0x10ffff; +} + +/** + * Typed (exit 10) error for a 4xx response that did not come from the API. + * + * Deliberately not routed through the status -> error-code table: a 403 block + * page must not look like an invalid API key, a 402 page must not look like an + * empty wallet, and a 426 page must not look like an obsolete CLI. + */ +function nonApiResponseError( + response: Response, + requestId: string, + text: string | undefined, + baseUrl: string, +): ApiError { + const contentType = response.headers.get('content-type') ?? undefined; + const pageTitle = extractPageTitle(text); + const contentTypeNote = contentType !== undefined ? ` (content-type: ${contentType})` : ''; + const titleNote = pageTitle !== undefined ? ` The page title is "${pageTitle}".` : ''; + const host = safeHost(baseUrl); + return new ApiError( + { + code: 'UNAVAILABLE', + message: + `Something between this machine and the TestSprite API answered HTTP ${response.status}` + + `${contentTypeNote} instead of the API.${titleNote} A proxy, firewall, captive portal, or ` + + 'WAF intercepted the request, so the API key was not rejected.', + nextAction: + `Open the same URL in a browser to see the page. Ask IT to allow ${host}, configure ` + + 'HTTPS_PROXY (and NODE_EXTRA_CA_CERTS when TLS is inspected) if the network requires a ' + + 'proxy, and check --endpoint-url / TESTSPRITE_API_URL. Do not rotate the API key for this response.', + requestId, + details: { + reason: 'not_testsprite', + httpStatus: response.status, + ...(contentType !== undefined ? { contentType } : {}), + ...(pageTitle !== undefined ? { pageTitle } : {}), + }, + }, + response.status, + ); +} + +function safeHost(url: string): string { + try { + return new URL(url).host; + } catch { + return 'the TestSprite API host'; } } diff --git a/src/lib/transport-failure.ts b/src/lib/transport-failure.ts new file mode 100644 index 00000000..7bb84604 --- /dev/null +++ b/src/lib/transport-failure.ts @@ -0,0 +1,329 @@ +/** + * Named transport-failure classification for the shared HTTP layer. + * + * Node's fetch reports almost every network problem as the same opaque + * `TypeError: fetch failed`; the real reason lives on the `cause` chain. When + * the CLI surfaces only the outer message, a TLS-interception problem, a DNS + * typo, a closed port and an authenticating proxy all read identically, and the + * operator (or a coding agent) is sent to fix the wrong thing. + * + * This module walks that chain once and names the cause. Certificate failures + * and proxy authentication are terminal: retrying re-sends the exact same + * request into the exact same wall, so they must fail on the first attempt. + * Other causes (DNS, refused, timeout, reset) keep the existing retry budget — + * they can be transient — but the final error still names the cause. + */ +import { TransportError, type ApiError } from './errors.js'; + +/** + * `details.reason` values that mean "the request never produced a TestSprite + * API answer". `not_testsprite` is a response from something in the middle + * (proxy/firewall/captive portal); the rest are failures before a response. + */ +export const TRANSPORT_FAILURE_REASONS = [ + 'not_testsprite', + 'tls_untrusted', + 'tls_expired', + 'tls_hostname_mismatch', + 'dns', + 'connection_refused', + 'host_unreachable', + 'connection_reset', + 'timeout', + 'proxy_authentication', +] as const; + +export type TransportFailureReason = (typeof TRANSPORT_FAILURE_REASONS)[number]; + +export interface TransportFailure { + reason: Exclude<TransportFailureReason, 'not_testsprite'>; + /** Node/undici code (or a short equivalent) safe to expose in details. */ + cause: string; + /** Human-readable one-paragraph explanation, with the remedy inline. */ + message: string; + /** Actionable remediation for the rendered `nextAction`. */ + nextAction: string; + /** True when retrying the same request cannot succeed. */ + fatal: boolean; +} + +/** True when `error` carries one of the named transport-failure reasons. */ +export function isTransportFailure(error: ApiError): boolean { + if (error.code !== 'UNAVAILABLE') return false; + const reason = error.getDetail<string>('reason', (v): v is string => typeof v === 'string'); + return reason !== undefined && (TRANSPORT_FAILURE_REASONS as readonly string[]).includes(reason); +} + +interface ErrorEvidence { + codes: Set<string>; + messages: string[]; +} + +const MAX_CAUSE_DEPTH = 6; + +/** Collect `code` values and messages from the whole `cause`/AggregateError graph. */ +function collectErrorEvidence(error: unknown): ErrorEvidence { + const codes = new Set<string>(); + const messages: string[] = []; + const seen = new Set<unknown>(); + + const walk = (node: unknown, depth: number): void => { + if (node === null || node === undefined || depth > MAX_CAUSE_DEPTH) return; + if (typeof node !== 'object') return; + if (seen.has(node)) return; + seen.add(node); + + const record = node as { + code?: unknown; + message?: unknown; + cause?: unknown; + errors?: unknown; + }; + if (typeof record.code === 'string' && record.code.length > 0) codes.add(record.code); + if (typeof record.message === 'string' && record.message.length > 0) { + messages.push(record.message); + } + walk(record.cause, depth + 1); + if (Array.isArray(record.errors)) { + for (const nested of record.errors) walk(nested, depth + 1); + } + }; + + walk(error, 0); + return { codes, messages }; +} + +function firstMatchingCode(codes: Set<string>, candidates: readonly string[]): string | undefined { + for (const candidate of candidates) { + if (codes.has(candidate)) return candidate; + } + return undefined; +} + +/** Short host for messages; falls back to a neutral phrase for a bad URL. */ +function targetHost(targetUrl: string | undefined): string { + if (targetUrl === undefined) return 'the API endpoint'; + try { + return new URL(targetUrl).host; + } catch { + return 'the API endpoint'; + } +} + +const TLS_UNTRUSTED_CODES = [ + 'UNABLE_TO_VERIFY_LEAF_SIGNATURE', + 'SELF_SIGNED_CERT_IN_CHAIN', + 'DEPTH_ZERO_SELF_SIGNED_CERT', + 'UNABLE_TO_GET_ISSUER_CERT', + 'UNABLE_TO_GET_ISSUER_CERT_LOCALLY', + 'CERT_SIGNATURE_FAILURE', + 'CERT_UNTRUSTED', + 'CERT_REJECTED', + 'CERT_REVOKED', + 'INVALID_CA', + 'PATH_LENGTH_EXCEEDED', +] as const; + +const TLS_EXPIRED_CODES = ['CERT_HAS_EXPIRED', 'CERT_NOT_YET_VALID'] as const; + +const TLS_HOSTNAME_CODES = ['ERR_TLS_CERT_ALTNAME_INVALID'] as const; + +const DNS_CODES = ['ENOTFOUND', 'EAI_AGAIN', 'EAI_NONAME', 'EAI_FAIL', 'EAI_NODATA'] as const; + +const TIMEOUT_CODES = [ + 'ETIMEDOUT', + 'ESOCKETTIMEDOUT', + 'UND_ERR_CONNECT_TIMEOUT', + 'UND_ERR_HEADERS_TIMEOUT', + 'UND_ERR_BODY_TIMEOUT', +] as const; + +const RESET_CODES = [ + 'ECONNRESET', + 'ECONNABORTED', + 'EPIPE', + 'UND_ERR_SOCKET', + 'UND_ERR_CLOSED', +] as const; + +const UNREACHABLE_CODES = ['EHOSTUNREACH', 'ENETUNREACH'] as const; + +const NODE_EXTRA_CA_HINT = + 'Node.js does not read the operating-system certificate store; set NODE_EXTRA_CA_CERTS to the PEM ' + + 'CA bundle that should be trusted.'; + +function proxyAuthenticationCode(evidence: ErrorEvidence): string | undefined { + const proxyAuthMessage = evidence.messages.find( + message => + /proxy authentication required/i.test(message) || + /proxy response \(407\)/i.test(message) || + /HTTP 407\b/i.test(message), + ); + if (proxyAuthMessage === undefined) return undefined; + return evidence.codes.has('UND_ERR_INVALID_ARG') ? 'UND_ERR_INVALID_ARG' : 'HTTP 407'; +} + +/** + * Classify a fetch rejection. Returns `undefined` when nothing recognisable is + * in the cause chain, so the caller keeps its historical generic handling. + */ +export function classifyFetchFailure( + error: unknown, + targetUrl?: string, +): TransportFailure | undefined { + const evidence = collectErrorEvidence(error); + const target = targetHost(targetUrl); + + const proxyCode = proxyAuthenticationCode(evidence); + if (proxyCode !== undefined) { + return { + reason: 'proxy_authentication', + cause: proxyCode, + fatal: true, + message: + `The HTTP proxy in front of ${target} requires authentication (HTTP 407), so the request ` + + 'never reached TestSprite. The API key was not rejected.', + nextAction: + 'Configure the proxy credentials in HTTPS_PROXY/HTTP_PROXY (for example ' + + 'https://user:password@proxy.example:8080), or ask IT for the proxy that allows outbound ' + + 'HTTPS. Then retry. Do not change the API key.', + }; + } + + const untrusted = firstMatchingCode(evidence.codes, TLS_UNTRUSTED_CODES); + if (untrusted !== undefined) { + return { + reason: 'tls_untrusted', + cause: untrusted, + fatal: true, + message: + `TLS certificate verification failed for ${target} (${untrusted}). The certificate chain ` + + 'is untrusted — usually a corporate proxy, antivirus, or self-signed certificate. ' + + NODE_EXTRA_CA_HINT, + nextAction: + 'Set NODE_EXTRA_CA_CERTS to the organisation PEM CA bundle that signs intercepted traffic ' + + '(for example NODE_EXTRA_CA_CERTS=/path/to/corporate-root-ca.pem), then retry. Do not set ' + + 'NODE_TLS_REJECT_UNAUTHORIZED=0: it disables certificate checks for requests that carry ' + + 'your API key.', + }; + } + + const expired = firstMatchingCode(evidence.codes, TLS_EXPIRED_CODES); + if (expired !== undefined) { + return { + reason: 'tls_expired', + cause: expired, + fatal: true, + message: + `The TLS certificate for ${target} is expired or not yet valid (${expired}). Check the ` + + 'system clock. If a proxy re-signs HTTPS, ' + + "set NODE_EXTRA_CA_CERTS to that proxy's PEM CA bundle so Node trusts it.", + nextAction: + 'Correct the system clock, then retry. Behind a TLS-inspecting proxy, set NODE_EXTRA_CA_CERTS ' + + 'to the organisation PEM CA bundle. Do not disable certificate verification.', + }; + } + + const hostname = firstMatchingCode(evidence.codes, TLS_HOSTNAME_CODES); + if (hostname !== undefined) { + return { + reason: 'tls_hostname_mismatch', + cause: hostname, + fatal: true, + message: + `The TLS certificate presented for ${target} is not valid for that host (${hostname}). A ` + + 'proxy may be re-signing traffic for a different name, or --endpoint-url / ' + + 'TESTSPRITE_API_URL points at the wrong host. If a proxy re-signs HTTPS, ' + + "set NODE_EXTRA_CA_CERTS to that proxy's PEM CA bundle.", + nextAction: + 'Check --endpoint-url / TESTSPRITE_API_URL, then set NODE_EXTRA_CA_CERTS to the ' + + 'organisation PEM CA bundle when the network re-signs TLS. Do not disable certificate verification.', + }; + } + + const dns = firstMatchingCode(evidence.codes, DNS_CODES); + if (dns !== undefined) { + return { + reason: 'dns', + cause: dns, + fatal: false, + message: + `Could not resolve ${target} (${dns}). Check the --endpoint-url / TESTSPRITE_API_URL spelling ` + + 'and your DNS, VPN, or proxy configuration.', + nextAction: + 'Correct the endpoint host or restore DNS/VPN connectivity, then retry. Run `testsprite doctor` ' + + 'to check the endpoint and credentials.', + }; + } + + if (evidence.codes.has('ECONNREFUSED')) { + return { + reason: 'connection_refused', + cause: 'ECONNREFUSED', + fatal: false, + message: + `Connection refused by ${target} (ECONNREFUSED). Nothing is accepting connections on that ` + + 'host and port — the port may be closed, or the service may not be running.', + nextAction: + 'Check the --endpoint-url host and port, and whether a local service or tunnel should be ' + + 'listening there. Then retry.', + }; + } + + const unreachable = firstMatchingCode(evidence.codes, UNREACHABLE_CODES); + if (unreachable !== undefined) { + return { + reason: 'host_unreachable', + cause: unreachable, + fatal: false, + message: + `Could not reach ${target} (${unreachable}). No route to that host — a VPN, firewall, or ` + + 'routing rule is likely blocking the connection.', + nextAction: + 'Check the VPN, firewall and proxy configuration (and --endpoint-url / TESTSPRITE_API_URL), ' + + 'then retry.', + }; + } + + const timeout = firstMatchingCode(evidence.codes, TIMEOUT_CODES); + if (timeout !== undefined) { + return { + reason: 'timeout', + cause: timeout, + fatal: false, + message: + `The connection to ${target} timed out (${timeout}). A firewall, VPN, or proxy may be ` + + 'silently dropping the connection.', + nextAction: + 'Check the network, VPN, firewall and proxy settings, then retry. If the backend is slow, ' + + 'raise the deadline with --request-timeout or TESTSPRITE_REQUEST_TIMEOUT_MS.', + }; + } + + const reset = firstMatchingCode(evidence.codes, RESET_CODES); + if (reset !== undefined) { + return { + reason: 'connection_reset', + cause: reset, + fatal: false, + message: + `The connection to ${target} was reset (${reset}). A proxy, firewall, or load balancer may ` + + 'be terminating the connection before the API answers.', + nextAction: + 'Check the proxy/firewall logs, then retry. If it persists, report it with the requestId.', + }; + } + + return undefined; +} + +/** Build the typed (exit 10) error for a classified failure. */ +export function transportFailureError( + failure: TransportFailure, + requestId: string, +): TransportError { + return new TransportError(failure.message, requestId, { + nextAction: failure.nextAction, + details: { reason: failure.reason, cause: failure.cause }, + }); +}