diff --git a/.changeset/fix-network-request-timeout.md b/.changeset/fix-network-request-timeout.md new file mode 100644 index 00000000..e5993ec7 --- /dev/null +++ b/.changeset/fix-network-request-timeout.md @@ -0,0 +1,5 @@ +--- +"clerk": patch +--- + +Add a default 60s timeout to all outbound CLI network requests. Previously a stalled connection to a Clerk API could hang a command indefinitely (with no error and no way to recover other than Ctrl-C); requests now abort with a clear, tagged error after 60s. A caller-supplied `AbortSignal` still composes with this default, so tighter per-call budgets continue to win. diff --git a/package.json b/package.json index e9f77b9f..2656a0a2 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "build": "bun run --filter @clerk/cli-core build", "dev": "bun run --cwd packages/cli-core dev", "test": "bun test 'packages/cli-core/src/' 'packages/extras/src/' 'scripts/' --parallel --only-failures", - "test:e2e": "bun test 'test/e2e/' --retry 1 --parallel --only-failures", + "test:e2e": "bun test 'test/e2e/' --retry 1 --parallel=4 --only-failures", "test:e2e:op": "bun run scripts/run-e2e-op.ts", "e2e:refresh-fixtures": "bun run scripts/refresh-e2e-fixtures.ts", "typecheck": "bun run --filter './packages/*' typecheck && tsc --noEmit -p scripts/tsconfig.json && tsc --noEmit -p test/e2e/tsconfig.json", diff --git a/packages/cli-core/src/lib/fetch.test.ts b/packages/cli-core/src/lib/fetch.test.ts index 39f03188..675cdb7e 100644 --- a/packages/cli-core/src/lib/fetch.test.ts +++ b/packages/cli-core/src/lib/fetch.test.ts @@ -41,4 +41,46 @@ describe("loggedFetch", () => { expect(init.headers.get("Authorization")).toBe("Bearer abc"); expect(init.headers.get("User-Agent")).toMatch(/^Clerk-CLI\//); }); + + // A server that accepts the connection but never responds. The mock rejects + // only when the request's AbortSignal fires, so it exercises the real timeout + // path: without a default timeout this hangs until bun's test timeout. + const hangingFetch = () => + ((_url: unknown, init: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + init.signal?.addEventListener("abort", () => reject(init.signal!.reason)); + })) as unknown as typeof fetch; + + test("aborts with a clear, tagged error after the default timeout when the server never responds", async () => { + globalThis.fetch = hangingFetch(); + await expect( + loggedFetch("https://example.test/hang", { tag: "plapi", timeoutMs: 30 }), + ).rejects.toThrow(/plapi: request timed out after 30ms/); + }, 2000); + + test("a shorter caller signal wins over the default timeout and is not masked by the timeout message", async () => { + globalThis.fetch = hangingFetch(); + const caller = AbortSignal.timeout(20); + // Must reject (the onFulfilled branch throws if it unexpectedly resolves)... + const err = await loggedFetch("https://example.test/hang", { + tag: "plapi", + timeoutMs: 10_000, + signal: caller, + }).then( + () => { + throw new Error("expected loggedFetch to reject, but it resolved"); + }, + (e: unknown) => e, + ); + // ...with the caller's 20ms abort, not relabeled as our 10s default timeout. + expect(String(err)).not.toMatch(/timed out after 10000ms/); + }, 2000); + + test("returns the response for a fast request without aborting", async () => { + globalThis.fetch = mock( + async () => new Response("ok", { status: 200 }), + ) as unknown as typeof fetch; + const res = await loggedFetch("https://example.test/ok", { tag: "plapi", timeoutMs: 5_000 }); + expect(res.status).toBe(200); + }); }); diff --git a/packages/cli-core/src/lib/fetch.ts b/packages/cli-core/src/lib/fetch.ts index ffebe505..62d80585 100644 --- a/packages/cli-core/src/lib/fetch.ts +++ b/packages/cli-core/src/lib/fetch.ts @@ -14,7 +14,10 @@ import { buildUserAgent } from "./user-agent.ts"; const USER_AGENT = buildUserAgent(); -export type LoggedFetchInit = RequestInit & { tag: string }; +/** Native `fetch()` has no timeout, so a stalled connection would hang forever. */ +const DEFAULT_REQUEST_TIMEOUT_MS = 60_000; + +export type LoggedFetchInit = RequestInit & { tag: string; timeoutMs?: number }; /** * Normalized response shape returned by the higher-level API request wrappers @@ -29,16 +32,30 @@ export interface ApiResponse { } export async function loggedFetch(url: URL | string, options: LoggedFetchInit): Promise { - const { tag, ...init } = options; + const { tag, timeoutMs = DEFAULT_REQUEST_TIMEOUT_MS, signal: callerSignal, ...init } = options; const method = init.method ?? "GET"; const urlStr = url.toString(); const headers = new Headers(init.headers); if (!headers.has("user-agent")) headers.set("User-Agent", USER_AGENT); log.debug(`${tag}: ${method} ${urlStr}`); - const response = await withNetworkAccess( - { operation: "connect", target: urlStr, label: tag }, - async () => fetch(url, { ...init, headers }), - ); + + // A caller signal (e.g. keyless.ts's tighter 15s) composes with our default. + const timeoutSignal = AbortSignal.timeout(timeoutMs); + const signal = callerSignal ? AbortSignal.any([callerSignal, timeoutSignal]) : timeoutSignal; + + let response: Response; + try { + response = await withNetworkAccess( + { operation: "connect", target: urlStr, label: tag }, + async () => fetch(url, { ...init, headers, signal }), + ); + } catch (err) { + // Only relabel when our timeout fired, not a caller abort or network error. + if (timeoutSignal.aborted && !callerSignal?.aborted) { + throw new Error(`${tag}: request timed out after ${timeoutMs}ms — ${method} ${urlStr}`); + } + throw err; + } if (!response.ok) { // Clone so the caller can still consume the body for error construction. const body = await response.clone().text(); diff --git a/test/e2e/lib/dev-server.ts b/test/e2e/lib/dev-server.ts index 8ea78661..c9729d0b 100644 --- a/test/e2e/lib/dev-server.ts +++ b/test/e2e/lib/dev-server.ts @@ -101,8 +101,6 @@ async function tryStart(opts: { const stderrLines: string[] = []; const stdoutLines: string[] = []; - log(`starting dev server: npx ${fullCmd.join(" ")} on port ${port}`); - const proc = Bun.spawn(["npx", ...fullCmd], { cwd: projectDir, stdout: "pipe", @@ -170,7 +168,6 @@ async function tryStart(opts: { } if (await canConnect(host, port, 1000)) { - log(`dev server ready (accepting TCP on ${host}:${port})`); return { kind: "ready", value: { proc, port, host, stdout: stdoutLines, stderr: stderrLines }, @@ -222,7 +219,6 @@ export async function startDevServer(opts: { /** Kill a dev server process, falling back to SIGKILL after 5 seconds. */ export async function killDevServer(proc: Subprocess): Promise { - log("killing dev server"); proc.kill("SIGTERM"); const timeout = setTimeout(() => { @@ -234,6 +230,4 @@ export async function killDevServer(proc: Subprocess): Promise { } finally { clearTimeout(timeout); } - - log("dev server stopped"); } diff --git a/test/e2e/lib/fixture-setup.ts b/test/e2e/lib/fixture-setup.ts index 08486393..2b06df9b 100644 --- a/test/e2e/lib/fixture-setup.ts +++ b/test/e2e/lib/fixture-setup.ts @@ -42,6 +42,20 @@ async function copyFixture(fixtureDir: string, projectDir: string): Promise { + await Bun.write( + join(projectDir, ".npmrc"), + "fetch-timeout=20000\nfetch-retries=2\nfetch-retry-mintimeout=1000\nfetch-retry-maxtimeout=8000\n", + ); +} + /** * Best-effort recursive remove. Cleanup runs after the test has already * passed, so a stray filesystem error here must not fail the test. Bun's @@ -56,6 +70,36 @@ async function safeRm(path: string): Promise { } } +/** + * Run a step with a hard timeout, retrying once on a fresh subprocess. In human + * mode `clerk link`/`clerk init` shell out to git and can intermittently stall + * in a non-fetch path (a git subprocess, a prompt) that the CLI's own request + * timeout doesn't bound — which would otherwise burn the whole 300s beforeAll + * budget. Promise.race abandons a hung subprocess (no stream deadlock), and the + * retry lands on a clean run; beforeAll is not retried, so a brief orphan can't + * cascade. + */ +async function withRetry(label: string, timeoutMs: number, fn: () => Promise): Promise { + for (let attempt = 1; attempt <= 2; attempt++) { + let timer: ReturnType | undefined; + const timeout = new Promise((_, reject) => { + timer = setTimeout( + () => reject(new Error(`${label} timed out after ${timeoutMs}ms`)), + timeoutMs, + ); + }); + try { + await Promise.race([fn(), timeout]); + return; + } catch (err) { + if (attempt === 2) throw err; + log(`${label} attempt ${attempt} failed (${err}); retrying`); + } finally { + clearTimeout(timer); + } + } +} + /** * Pre-link the project to the test Clerk application using an isolated * CLERK_CONFIG_DIR, so `clerk init` finds an existing link and skips the @@ -65,7 +109,10 @@ async function linkProject(projectDir: string, configDir: string): Promise const appId = requireEnv("CLERK_CLI_TEST_APP_ID"); const platformAPIKey = requireEnv("CLERK_PLATFORM_API_KEY"); - const result = await Bun.$`bun ${CLI_PATH} --mode human link --app ${appId}` + // Agent mode keeps link non-interactive: if a retry re-runs it on a project + // the first (hung-then-killed) attempt already linked, agent mode prints + // "already linked" and exits 0 instead of blocking on a human confirm prompt. + const result = await Bun.$`bun ${CLI_PATH} --mode agent link --app ${appId}` .cwd(projectDir) .env({ CLERK_CONFIG_DIR: configDir, @@ -152,31 +199,31 @@ export type Fixture = { export async function setupFixture(name: FixtureName): Promise { const config = fixtures[name]; const fixtureDir = join(FIXTURES_DIR, name); - log("setup started"); // Resolve symlinks (macOS /var -> /private/var) so profile keys match across commands const tmp = await realpath(tmpdir()); const projectDir = await mkdtemp(join(tmp, `clerk-e2e-${name}-`)); const configDir = await mkdtemp(join(tmp, "clerk-e2e-config-")); await copyFixture(fixtureDir, projectDir); + await writeNpmrc(projectDir); log("fixture copied"); let publishableKey = ""; let secretKey = ""; try { - // Git-init before linking so the profile key matches for later commands - await gitInit(projectDir); + // Git-init before linking so the profile key matches for later commands. + // Step markers are debug-gated (CLERK_E2E_DEBUG) and pinpoint which step + // stalls if setup ever hits the 300s beforeAll budget. + await withRetry("git init", 30_000, () => gitInit(projectDir)); log("git init done"); - - // The magic happens here, we actually test out `clerk link` and `clerk init` - await linkProject(projectDir, configDir); + // Budgets sit above loggedFetch's 60s request timeout so a genuinely slow + // API call is handled there; withRetry only trips on a non-fetch stall. + await withRetry("clerk link", 90_000, () => linkProject(projectDir, configDir)); log("clerk link done"); - - await runClerkInit(projectDir, configDir); + await withRetry("clerk init", 120_000, () => runClerkInit(projectDir, configDir)); log("clerk init done"); - // Verify clerk init wrote env files and extract keys. const envVars = await parseEnvFiles(projectDir); const publishableKeyName = await detectPublishableKeyName(projectDir); @@ -191,11 +238,15 @@ export async function setupFixture(name: FixtureName): Promise { throw new Error(`${secretKeyName} not found in env files written by clerk init.`); } - const install = await Bun.$`npm ci --ignore-scripts --legacy-peer-deps` - .cwd(projectDir) - .quiet() - .nothrow(); - assertSuccess("npm ci failed", install); + // fetch-timeout/retries come from the project .npmrc (writeNpmrc); --no-audit + // and --no-fund drop npm's advisory network round-trips during `ci`. + await withRetry("npm ci", 120_000, async () => { + const install = await Bun.$`npm ci --ignore-scripts --legacy-peer-deps --no-audit --no-fund` + .cwd(projectDir) + .quiet() + .nothrow(); + assertSuccess("npm ci failed", install); + }); log("npm ci done"); } catch (err) { await safeRm(projectDir); @@ -203,13 +254,9 @@ export async function setupFixture(name: FixtureName): Promise { throw new Error("setup failed", { cause: err }); } - log("setup complete"); - const cleanup = async () => { - log("cleanup started"); await safeRm(projectDir); await safeRm(configDir); - log("cleanup done"); }; return { diff --git a/test/e2e/lib/fixture-test.ts b/test/e2e/lib/fixture-test.ts index 6812101e..87cab863 100644 --- a/test/e2e/lib/fixture-test.ts +++ b/test/e2e/lib/fixture-test.ts @@ -101,20 +101,16 @@ export function createFixtureHarness(name: FixtureName): FixtureHarness { let users: Users | null = null; beforeAll(async () => { - log("beforeAll started"); fixture = await setupFixture(name); users = createUsers(fixture); - log("beforeAll finished"); }, 300_000); afterEach(async () => { await users?.cleanup(); - }); + }, 30_000); // BAPI deletes can exceed bun's 5s default under load afterAll(async () => { - log("afterAll started"); await fixture?.cleanup(); - log("afterAll finished"); }, 60_000); return () => { @@ -142,14 +138,12 @@ export function runFixtureTests(harness: FixtureHarness): void { const { projectDir, config } = fixture; // Build first so type generation artifacts are available for tsc. - log("build started"); const build = await Bun.$`npx ${config.buildCmd}`.cwd(projectDir).quiet().nothrow(); if (build.exitCode !== 0) { throw new Error( `${config.buildCmd.join(" ")} failed:\n${build.stdout.toString()}\n${build.stderr.toString()}`, ); } - log("build succeeded"); }, { timeout: 300_000 }, // 5 minutes - install + build can be slow) ); @@ -164,14 +158,12 @@ export function runFixtureTests(harness: FixtureHarness): void { // framework-specific type generation), otherwise plain tsc. const useTypecheck = await hasTypecheckScript(projectDir); const command = useTypecheck ? "npm run typecheck" : "bunx tsc --noEmit"; - log(`typecheck started (${command} in ${projectDir})`); const shell = useTypecheck ? await Bun.$`npm run typecheck 2>&1`.cwd(projectDir).quiet().nothrow() : await Bun.$`bunx tsc --noEmit 2>&1`.cwd(projectDir).quiet().nothrow(); if (shell.exitCode !== 0) { throw new Error(`${command} failed in ${projectDir}:\n${shell.text()}`); } - log("typecheck succeeded"); }, { timeout: 300_000 }, // 5 minutes - install + typecheck can be slow ); @@ -198,7 +190,6 @@ export function runFileExistsTest(harness: FixtureHarness, expectedFiles: string ); const existing = found.filter(Boolean); expect(existing.length).toBeGreaterThanOrEqual(1); - log(`found: ${existing.join(", ")}`); }); } @@ -265,11 +256,9 @@ export function runBrowserTests(harness: FixtureHarness): void { context, options: frontendApiUrl ? { frontendApiUrl } : undefined, }); - log(`navigating to http://${host}:${port}`); await page.goto(`http://${host}:${port}`, { waitUntil: "load" }); // 5. Sign in - log("signing in"); await clerk.signIn({ page, signInParams: { @@ -281,7 +270,6 @@ export function runBrowserTests(harness: FixtureHarness): void { // 6. Verify Clerk loaded await clerk.loaded({ page }); - log("clerk has been loaded"); // 7. Check to see that the user is now on the window object. await page.waitForFunction( @@ -289,7 +277,6 @@ export function runBrowserTests(harness: FixtureHarness): void { null, { timeout: 10_000 }, ); - log("auth flow passed"); // Log any console errors as warnings (non-fatal) if (consoleErrors.length > 0) { diff --git a/test/e2e/lib/logger.ts b/test/e2e/lib/logger.ts index 96aa173e..d27f9b86 100644 --- a/test/e2e/lib/logger.ts +++ b/test/e2e/lib/logger.ts @@ -2,7 +2,7 @@ const startTime = Date.now(); const isDebug = process.env.CLERK_E2E_DEBUG === "1" || process.env.CLERK_E2E_DEBUG === "true"; -/** Log a timestamped message with fixture name for tracing execution order. */ +/** Emit a timestamped diagnostic line when CLERK_E2E_DEBUG is set. */ export function log(message: string): void { if (!isDebug) return; const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); diff --git a/test/e2e/lib/test-user.ts b/test/e2e/lib/test-user.ts index d679d65d..518f82ea 100644 --- a/test/e2e/lib/test-user.ts +++ b/test/e2e/lib/test-user.ts @@ -53,8 +53,6 @@ export async function createTestUser(configDir: string, target: TestUserTarget): skip_password_checks: true, }); - log(`creating test user: ${email}`); - const result = await Bun.$`bun ${CLI_PATH} users create -d ${body} --json --yes ${targetArgs(target)}` .env(clerkEnv(configDir, target)) @@ -69,7 +67,6 @@ export async function createTestUser(configDir: string, target: TestUserTarget): } const user: { id: string } = JSON.parse(result.stdout.toString()); - log(`test user created: ${user.id}`); return { id: user.id, email, password }; } @@ -80,8 +77,6 @@ export async function deleteTestUser( configDir: string, target: TestUserTarget, ): Promise { - log(`deleting test user: ${userId}`); - const result = await Bun.$`bun ${CLI_PATH} api /users/${userId} -X DELETE --yes ${targetArgs(target)}` .env(clerkEnv(configDir, target)) @@ -93,7 +88,5 @@ export async function deleteTestUser( const stderr = result.stderr.toString().trim(); const detail = stderr || stdout || "(no output)"; log(`warning: failed to delete test user ${userId}: ${detail}`); - } else { - log(`test user deleted: ${userId}`); } }