Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-network-request-timeout.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
42 changes: 42 additions & 0 deletions packages/cli-core/src/lib/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Response>((_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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

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);
});
});
29 changes: 23 additions & 6 deletions packages/cli-core/src/lib/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -29,16 +32,30 @@ export interface ApiResponse {
}

export async function loggedFetch(url: URL | string, options: LoggedFetchInit): Promise<Response> {
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();
Expand Down
6 changes: 0 additions & 6 deletions test/e2e/lib/dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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<void> {
log("killing dev server");
proc.kill("SIGTERM");

const timeout = setTimeout(() => {
Expand All @@ -234,6 +230,4 @@ export async function killDevServer(proc: Subprocess): Promise<void> {
} finally {
clearTimeout(timeout);
}

log("dev server stopped");
}
80 changes: 62 additions & 18 deletions test/e2e/lib/fixture-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,20 @@ async function copyFixture(fixtureDir: string, projectDir: string): Promise<void
await cp(fixtureDir, projectDir, { recursive: true });
}

/**
* npm's default fetch-timeout is 300s, so one stalled registry connection in
* `clerk init`'s SDK install or `npm ci` can consume the entire 300s beforeAll
* budget. Cap it low so a stalled fetch aborts fast and retries on a fresh
* connection — and stays well under the withRetry step budgets. Both npm runs
* use projectDir as cwd, so a project-level `.npmrc` covers them.
*/
async function writeNpmrc(projectDir: string): Promise<void> {
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
Expand All @@ -56,6 +70,36 @@ async function safeRm(path: string): Promise<void> {
}
}

/**
* 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<void>): Promise<void> {
for (let attempt = 1; attempt <= 2; attempt++) {
let timer: ReturnType<typeof setTimeout> | undefined;
const timeout = new Promise<never>((_, 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
Expand Down Expand Up @@ -152,31 +196,31 @@ export type Fixture = {
export async function setupFixture(name: FixtureName): Promise<Fixture> {
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);
Expand All @@ -191,25 +235,25 @@ export async function setupFixture(name: FixtureName): Promise<Fixture> {
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);
await safeRm(configDir);
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 {
Expand Down
15 changes: 1 addition & 14 deletions test/e2e/lib/fixture-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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)
);
Expand All @@ -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
);
Expand All @@ -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(", ")}`);
});
}

Expand Down Expand Up @@ -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: {
Expand All @@ -281,15 +270,13 @@ 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(
() => typeof window.Clerk !== "undefined" && window.Clerk.user != null,
null,
{ timeout: 10_000 },
);
log("auth flow passed");

// Log any console errors as warnings (non-fatal)
if (consoleErrors.length > 0) {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/lib/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading