From c9db560b2cf15572a713817c2704937134439837 Mon Sep 17 00:00:00 2001 From: Sylvain Utard Date: Mon, 2 Nov 2026 13:33:55 +0100 Subject: [PATCH] fix(http): raise default connect timeout and explain CLI timeouts Query streams keep the --connect-timeout timer running until response headers arrive, so a 5s default aborted queries that took a few seconds to start responding with a bare "Request timed out". Raise the default to 30s and make timeout errors name the flag that fired, say the limit is client-side, and show how to raise it. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 + COMMANDS.md | 2 +- README.md | 2 +- cli-reference.json | 2 +- cli/src/cli.ts | 2 +- cli/src/lib/http.test.ts | 47 +++++++++++++++++-- cli/src/lib/http.ts | 75 +++++++++++++++++++++++++------ cli/src/lib/transport-defaults.ts | 2 +- 8 files changed, 112 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dad64fc8..a0623bf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Generate, preview, and upgrade data apps with data-app 0.74.0. +- Raise the default `--connect-timeout` from 5 to 30 seconds, so queries that take a few seconds to start responding no longer time out. +- Timeout errors now name the CLI flag that fired, explain that the limit is client-side (the request may still be running on the server, and slow networks can cause it), and say how to raise it. ### Fixed diff --git a/COMMANDS.md b/COMMANDS.md index d50e826c..dfc108de 100644 --- a/COMMANDS.md +++ b/COMMANDS.md @@ -16,7 +16,7 @@ Altertable CLI v1.13.2 • Query and manage your data platform from the terminal | `--no-agent` | Disable agent mode, including automatic detection | | `--no-color` | Disable terminal colors and styling | | `--profile ` | Use a named profile for this command only | -| `--connect-timeout ` | HTTP connect timeout in seconds (default 5) | +| `--connect-timeout ` | HTTP connect timeout in seconds (default 30) | | `--read-timeout ` | HTTP read timeout in seconds (default 60; 0 = no limit for streams) | | `--ignore-ssl-errors` | Skip TLS certificate verification for HTTPS requests (for development only, e.g. self-signed local servers) | diff --git a/README.md b/README.md index aeb7d39b..eab706e8 100644 --- a/README.md +++ b/README.md @@ -506,7 +506,7 @@ These flags apply to every command and may be placed before or after commands: | `--no-agent` | Disable agent mode, including automatic detection | | `--agent` | Agent preset: structured JSON output, no pager, colors, or terminal styling | | `--debug`, `-d` | Enable debug output | -| `--connect-timeout ` | HTTP connect timeout in seconds (default: `5`) | +| `--connect-timeout ` | HTTP connect timeout in seconds (default: `30`) | | `--read-timeout ` | HTTP read timeout in seconds (default: `60`; `0` = unlimited for streams) | Per-request read timeout on `query`, `upload`, and `upsert`: diff --git a/cli-reference.json b/cli-reference.json index 4d6ea431..f152aa5c 100644 --- a/cli-reference.json +++ b/cli-reference.json @@ -92,7 +92,7 @@ "name": "connect-timeout", "aliases": [], "type": "string", - "description": "HTTP connect timeout in seconds (default 5)", + "description": "HTTP connect timeout in seconds (default 30)", "required": false, "repeatable": false, "scope": "global", diff --git a/cli/src/cli.ts b/cli/src/cli.ts index e706f18e..4b9a6d7e 100644 --- a/cli/src/cli.ts +++ b/cli/src/cli.ts @@ -86,7 +86,7 @@ const ROOT_ARGS = defineArguments({ }, 'connect-timeout': { type: 'string', - description: 'HTTP connect timeout in seconds (default 5)', + description: 'HTTP connect timeout in seconds (default 30)', flagScope: 'global', }, 'read-timeout': { diff --git a/cli/src/lib/http.test.ts b/cli/src/lib/http.test.ts index 7f4a092d..ae8829c6 100644 --- a/cli/src/lib/http.test.ts +++ b/cli/src/lib/http.test.ts @@ -27,6 +27,7 @@ import { shouldIgnoreSslErrors, } from '@/lib/http.ts'; import { redactResponseBodyForDebug } from '@/lib/redact.ts'; +import { DEFAULT_CONNECT_TIMEOUT_MS } from '@/lib/transport-defaults.ts'; import { delay } from '@/test-utils/time.ts'; const fakeBearerKey = 'atm_fake_test_key_for_redaction'; @@ -328,7 +329,7 @@ describe('context timeout resolution', () => { url: 'https://example.com', authHeader: 'Authorization: Bearer test', }) - ).toBe(5_000 + 30_000); + ).toBe(DEFAULT_CONNECT_TIMEOUT_MS + 30_000); }); test('stream read timeout 0 uses connect-only abort when context unset', () => { @@ -350,7 +351,7 @@ describe('context timeout resolution', () => { authHeader: 'Authorization: Bearer test', readTimeoutMs: 0, }) - ).toBe(5_000); + ).toBe(DEFAULT_CONNECT_TIMEOUT_MS); }); }); @@ -562,8 +563,9 @@ describe('httpSendStream live timeouts', () => { expect(error).toBeInstanceOf(TimeoutError); expect((error as TimeoutError).exitCode).toBe(9); expect((error as TimeoutError).message).toBe( - 'Request timed out: POST https://example.com/query' + 'CLI timed out after 5ms reading the response: POST https://example.com/query' ); + expect((error as TimeoutError).details).toContain('--read-timeout'); } expect(abortObserved).toBe(true); } finally { @@ -571,6 +573,45 @@ describe('httpSendStream live timeouts', () => { } }); + test('connect timeout names the CLI flag and explains it is client-side', async () => { + delete process.env.ALTERTABLE_MOCK_HTTP_FILE; + const originalFetch = globalThis.fetch; + + globalThis.fetch = Object.assign( + (_url: Parameters[0], init?: Parameters[1]) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + reject(new DOMException('aborted', 'AbortError')); + }); + }), + { preconnect: originalFetch.preconnect } + ); + + try { + await httpSendStream({ + method: 'POST', + url: 'https://example.com/query', + authHeader: 'Authorization: Bearer test', + body: '{}', + connectTimeoutMs: 5, + readTimeoutMs: 0, + }); + expect.unreachable('request should have timed out'); + } catch (error) { + expect(error).toBeInstanceOf(TimeoutError); + expect((error as TimeoutError).exitCode).toBe(9); + expect((error as TimeoutError).message).toBe( + 'CLI timed out after 5ms waiting for the server to respond: POST https://example.com/query' + ); + const details = (error as TimeoutError).details ?? ''; + expect(details).toContain('enforced by the CLI (--connect-timeout)'); + expect(details).toContain('slow network'); + expect(details).toContain('Raise it with --connect-timeout .'); + } finally { + globalThis.fetch = originalFetch; + } + }); + test('positive readTimeoutMs also aborts stalled error response bodies', async () => { delete process.env.ALTERTABLE_MOCK_HTTP_FILE; const originalFetch = globalThis.fetch; diff --git a/cli/src/lib/http.ts b/cli/src/lib/http.ts index 3d22c7f8..d7df946c 100644 --- a/cli/src/lib/http.ts +++ b/cli/src/lib/http.ts @@ -206,10 +206,34 @@ function unwrapErrorDetail(error: unknown): string { return messages.join(': '); } -function timeoutError(options: HttpSendOptions, cause: unknown): TimeoutError { +/** Which CLI-side limit an abort came from, so the error can name the flag to raise. */ +type TimeoutBudget = { + flags: string; + durationMs: number; + waitingFor: string; +}; + +function formatDuration(durationMs: number): string { + return durationMs % 1_000 === 0 + ? `${durationMs / 1_000}s` + : `${durationMs}ms`; +} + +function timeoutError( + options: HttpSendOptions, + budget: TimeoutBudget, + cause: unknown +): TimeoutError { return new TimeoutError( - `Request timed out: ${options.method} ${options.url}`, - { cause } + `CLI timed out after ${formatDuration(budget.durationMs)} ${budget.waitingFor}: ${options.method} ${options.url}`, + { + cause, + details: [ + `This limit is enforced by the CLI (${budget.flags}), not reported by the server; the request may still be running server-side.`, + 'A slow network or a long-running request can both trigger it.', + `Raise it with ${budget.flags.replace(' + ', ' or ')} .`, + ].join('\n'), + } ); } @@ -228,10 +252,11 @@ function connectionError( function transportError( options: HttpSendOptions, signal: AbortSignal, + budget: () => TimeoutBudget, cause: unknown ): NetworkError { return signal.aborted - ? timeoutError(options, cause) + ? timeoutError(options, budget(), cause) : connectionError(options, cause); } @@ -239,6 +264,7 @@ function wrapResponseStream( stream: ReadableStream, clearTimeoutAfterRead: () => void, signal: AbortSignal, + budget: () => TimeoutBudget, options: HttpStreamOptions ): ReadableStream { const reader = stream.getReader(); @@ -256,7 +282,7 @@ function wrapResponseStream( } catch (error) { clearTimeoutAfterRead(); reader.releaseLock(); - controller.error(transportError(options, signal, error)); + controller.error(transportError(options, signal, budget, error)); } }, async cancel(reason) { @@ -518,6 +544,14 @@ async function executeLiveRequest(options: HttpSendOptions): Promise { const headers = buildRequestHeaders(options); const timeoutMs = resolveFetchTimeoutMs(options); const signal = AbortSignal.timeout(timeoutMs); + const budget = (): TimeoutBudget => ({ + flags: + resolveReadTimeoutMs(options) > 0 + ? '--connect-timeout + --read-timeout' + : '--connect-timeout', + durationMs: timeoutMs, + waitingFor: 'waiting for the server', + }); logHttpRequest(options); logDebug(`Request: ${options.method} ${options.url}`); @@ -536,14 +570,14 @@ async function executeLiveRequest(options: HttpSendOptions): Promise { buildFetchInit(options, headers, signal) ); } catch (error) { - throw transportError(options, signal, error); + throw transportError(options, signal, budget, error); } let responseBody: string; try { responseBody = await response.text(); } catch (error) { - throw transportError(options, signal, error); + throw transportError(options, signal, budget, error); } logHttpDebug([ @@ -580,11 +614,23 @@ async function executeLiveStream( clearTimeout(timeout); timeout = undefined; }; - const startTimeout = (durationMs: number) => { + let activeBudget: TimeoutBudget = { + flags: '--connect-timeout', + durationMs: connectTimeoutMs, + waitingFor: 'waiting for the server to respond', + }; + const budget = () => activeBudget; + const startTimeout = (next: TimeoutBudget) => { clearActiveTimeout(); - timeout = setTimeout(() => abortController.abort(), durationMs); + activeBudget = next; + timeout = setTimeout(() => abortController.abort(), next.durationMs); + }; + const readBudget: TimeoutBudget = { + flags: '--read-timeout', + durationMs: readTimeoutMs, + waitingFor: 'reading the response', }; - startTimeout(connectTimeoutMs); + startTimeout(activeBudget); logHttpRequest(options); logDebug(`Request: ${options.method} ${options.url}`); @@ -598,7 +644,7 @@ async function executeLiveStream( clearActiveTimeout(); } catch (error) { clearActiveTimeout(); - throw transportError(options, abortController.signal, error); + throw transportError(options, abortController.signal, budget, error); } if (response.status >= 200 && response.status < 300) { @@ -606,25 +652,26 @@ async function executeLiveStream( throw new NetworkError('Response body is missing.'); } if (readTimeoutMs > 0) { - startTimeout(readTimeoutMs); + startTimeout(readBudget); } return wrapResponseStream( response.body, clearActiveTimeout, abortController.signal, + budget, options ); } if (readTimeoutMs > 0) { - startTimeout(readTimeoutMs); + startTimeout(readBudget); } let responseBody: string; try { responseBody = await response.text(); } catch (error) { - throw transportError(options, abortController.signal, error); + throw transportError(options, abortController.signal, budget, error); } finally { clearActiveTimeout(); } diff --git a/cli/src/lib/transport-defaults.ts b/cli/src/lib/transport-defaults.ts index c8801991..45ada279 100644 --- a/cli/src/lib/transport-defaults.ts +++ b/cli/src/lib/transport-defaults.ts @@ -1,3 +1,3 @@ -export const DEFAULT_CONNECT_TIMEOUT_MS = 5_000; +export const DEFAULT_CONNECT_TIMEOUT_MS = 30_000; export const DEFAULT_READ_TIMEOUT_MS = 60_000; export const STREAM_READ_TIMEOUT_MS = 0;