From 987771409b3fa758acb13f43fc9c363b478fc19e Mon Sep 17 00:00:00 2001 From: tujack39 Date: Fri, 23 Oct 2026 22:13:24 +0800 Subject: [PATCH] fix(env): report environment preflight timeout as REQUEST_TIMEOUT --- src/index.ts | 3 +- src/lib/environment-tunnel.spec.ts | 67 ++++++++++++++++++++++++++++-- src/lib/environment-tunnel.ts | 28 +++++++++++-- src/lib/errors.test.ts | 18 ++++++++ src/lib/errors.ts | 19 +++++++-- 5 files changed, 124 insertions(+), 11 deletions(-) mode change 100755 => 100644 src/index.ts diff --git a/src/index.ts b/src/index.ts old mode 100755 new mode 100644 index d2e3d6b0..4d83c13a --- a/src/index.ts +++ b/src/index.ts @@ -504,8 +504,9 @@ try { code: 'REQUEST_TIMEOUT', message: err.message, nextAction: + err.nextAction ?? 'Increase --request-timeout or set TESTSPRITE_REQUEST_TIMEOUT_MS. ' + - 'Check that the backend is reachable and not overloaded.', + 'Check that the backend is reachable and not overloaded.', requestId: err.requestId, details: { timeoutMs: err.timeoutMs }, }, diff --git a/src/lib/environment-tunnel.spec.ts b/src/lib/environment-tunnel.spec.ts index 8735e023..04708b13 100644 --- a/src/lib/environment-tunnel.spec.ts +++ b/src/lib/environment-tunnel.spec.ts @@ -1,8 +1,12 @@ import { describe, expect, it, vi } from 'vitest'; -import { ApiError } from './errors.js'; -import type { HttpClient } from './http.js'; +import { ApiError, RequestTimeoutError } from './errors.js'; +import { HttpClient } from './http.js'; import { assertLocalPortListening, parseLoopbackTargetUrl } from './local-target.js'; -import { readEnvironmentPreflight, resolveEnvironmentRunTarget } from './environment-tunnel.js'; +import { + ENVIRONMENT_PREFLIGHT_TIMEOUT_MS, + readEnvironmentPreflight, + resolveEnvironmentRunTarget, +} from './environment-tunnel.js'; function resolver(environments: unknown) { const get = vi.fn(async () => ({ environments })); @@ -28,6 +32,63 @@ describe('environment preflight and diagnostics', () => { await readEnvironmentPreflight({ get } as unknown as Pick, '/tests/a'); expect(get).toHaveBeenCalledTimes(1); }); + it('classifies a stalled preflight read as REQUEST_TIMEOUT (exit 7), not a raw abort', async () => { + vi.useFakeTimers(); + try { + const get = vi.fn((_path: string, options: { signal: AbortSignal }) => { + return new Promise((_resolve, reject) => { + options.signal.addEventListener('abort', () => reject(options.signal.reason)); + }); + }); + const promise = readEnvironmentPreflight( + { get } as unknown as Pick, + '/tests/a', + ); + const assertion = expect(promise).rejects.toBeInstanceOf(RequestTimeoutError); + await vi.advanceTimersByTimeAsync(ENVIRONMENT_PREFLIGHT_TIMEOUT_MS); + await assertion; + const err = (await promise.catch(e => e)) as RequestTimeoutError; + expect(err.code).toBe('REQUEST_TIMEOUT'); + expect(err.exitCode).toBe(7); + expect(err.timeoutMs).toBe(ENVIRONMENT_PREFLIGHT_TIMEOUT_MS); + // The 5s preflight deadline is fixed; the guidance must name the + // preflight read and never point at --request-timeout, which does + // not govern it. + expect(err.message).toContain('Environment preflight read timed out'); + expect(err.message).not.toContain('--request-timeout'); + expect(err.nextAction).toContain('environment preflight read'); + expect(err.nextAction).not.toContain('--request-timeout'); + } finally { + vi.useRealTimers(); + } + }); + it('passes the preflight REQUEST_TIMEOUT through a real HttpClient untouched', async () => { + vi.useFakeTimers(); + try { + const fetchImpl = vi.fn((_input: unknown, init?: { signal?: AbortSignal }) => { + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => reject(init.signal?.reason)); + }); + }); + const client = new HttpClient({ + baseUrl: 'https://api.example.com/api/cli/v1', + apiKey: '', + fetchImpl: fetchImpl as unknown as typeof fetch, + sleep: () => Promise.resolve(), + random: () => 0, + requestTimeoutMs: 120_000, + }); + const promise = readEnvironmentPreflight(client, '/tests/a'); + const assertion = expect(promise).rejects.toBeInstanceOf(RequestTimeoutError); + await vi.advanceTimersByTimeAsync(ENVIRONMENT_PREFLIGHT_TIMEOUT_MS); + await assertion; + const err = (await promise.catch(e => e)) as RequestTimeoutError; + expect(err.code).toBe('REQUEST_TIMEOUT'); + expect(err.exitCode).toBe(7); + } finally { + vi.useRealTimers(); + } + }); it('falls back after a failed test read without reading environments', async () => { const get = vi.fn(async (_path: string, _options: unknown) => { throw new Error('unavailable'); diff --git a/src/lib/environment-tunnel.ts b/src/lib/environment-tunnel.ts index ddc7f442..31dc7890 100644 --- a/src/lib/environment-tunnel.ts +++ b/src/lib/environment-tunnel.ts @@ -47,10 +47,30 @@ export function readEnvironmentPreflight( client: Pick, path: string, ): Promise { - return client.get(path, { - retry: false, - signal: AbortSignal.timeout(ENVIRONMENT_PREFLIGHT_TIMEOUT_MS), - }); + // Abort with a RequestTimeoutError as the reason, not AbortSignal.timeout's + // raw DOMException: HttpClient passes a RequestTimeoutError through + // untouched, so a stalled preflight read surfaces as REQUEST_TIMEOUT + // (exit 7) instead of reaching the top-level catch as an uncaught + // exception. The 5s deadline here is fixed — --request-timeout does not + // govern it — so the message and nextAction name the preflight read + // rather than pointing at the flag. + const controller = new AbortController(); + const timer = setTimeout(() => { + controller.abort( + new RequestTimeoutError(ENVIRONMENT_PREFLIGHT_TIMEOUT_MS, 'local', { + message: + `Environment preflight read timed out after ` + + `${ENVIRONMENT_PREFLIGHT_TIMEOUT_MS / 1000}s (client-side).`, + nextAction: + 'The environment preflight read timed out. ' + + 'Check that the backend is reachable and not overloaded, then retry.', + }), + ); + }, ENVIRONMENT_PREFLIGHT_TIMEOUT_MS); + timer.unref?.(); + return client + .get(path, { retry: false, signal: controller.signal }) + .finally(() => clearTimeout(timer)); } export function assertNoTunnelOptions(args: { diff --git a/src/lib/errors.test.ts b/src/lib/errors.test.ts index 5c266ecf..80be6f17 100644 --- a/src/lib/errors.test.ts +++ b/src/lib/errors.test.ts @@ -692,6 +692,24 @@ describe('RequestTimeoutError', () => { const err = new RequestTimeoutError(1_500); expect(err.message).toContain('2s'); }); + + it('leaves nextAction unset by default so renderers use the generic advice', () => { + const err = new RequestTimeoutError(5_000); + expect(err.nextAction).toBeUndefined(); + }); + + it('honors caller-specific message and nextAction overrides', () => { + const err = new RequestTimeoutError(5_000, 'local', { + message: 'Environment preflight read timed out after 5s (client-side).', + nextAction: 'The environment preflight read timed out. Check the backend, then retry.', + }); + expect(err.message).toBe('Environment preflight read timed out after 5s (client-side).'); + expect(err.message).not.toContain('--request-timeout'); + expect(err.nextAction).toContain('environment preflight read'); + expect(err.code).toBe('REQUEST_TIMEOUT'); + expect(err.exitCode).toBe(7); + expect(err.timeoutMs).toBe(5_000); + }); }); describe('INSUFFICIENT_CREDITS detection', () => { diff --git a/src/lib/errors.ts b/src/lib/errors.ts index b87452ac..99f69158 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -440,22 +440,35 @@ export class TransportError extends ApiError { * as `TimeoutError` from the polling path. Use `--request-timeout ` or * `TESTSPRITE_REQUEST_TIMEOUT_MS` to extend the deadline when targeting a * slow backend. + * + * A caller working against a fixed internal deadline the `--request-timeout` + * flag does not govern (e.g. the environment preflight read) passes + * `message` / `nextAction` overrides so the rendered guidance names that + * deadline instead of pointing at the flag. */ export class RequestTimeoutError extends CLIError { readonly requestId: string; readonly timeoutMs: number; + /** Caller-specific envelope guidance; unset keeps the generic advice. */ + readonly nextAction?: string; - constructor(timeoutMs: number, requestId = 'local') { + constructor( + timeoutMs: number, + requestId = 'local', + options: { message?: string; nextAction?: string } = {}, + ) { const timeoutSec = Math.round(timeoutMs / 1000); super( - `Request timed out after ${timeoutSec}s (client-side). ` + - `Use --request-timeout or TESTSPRITE_REQUEST_TIMEOUT_MS to extend the deadline.`, + options.message ?? + `Request timed out after ${timeoutSec}s (client-side). ` + + `Use --request-timeout or TESTSPRITE_REQUEST_TIMEOUT_MS to extend the deadline.`, 7, 'REQUEST_TIMEOUT', ); this.name = 'RequestTimeoutError'; this.requestId = requestId; this.timeoutMs = timeoutMs; + if (options.nextAction !== undefined) this.nextAction = options.nextAction; } }