Merge pull request #2969 from herjarsa/fix/opencode-read-timeout-2470
fix(ui): bound OpenCode read requests so half-open sockets cannot freeze bootstrap (#2470)
This commit is contained in:
@@ -0,0 +1,157 @@
|
||||
import { beforeEach, describe, expect, mock, test } from 'bun:test';
|
||||
|
||||
// Regression tests for issue #2470: sessions stuck on "loading sessions"
|
||||
// forever after managed OpenCode connection goes half-open.
|
||||
//
|
||||
// The SDK client fetch wrapper must bound read requests so a socket that
|
||||
// neither resolves nor rejects fails after `requestTimeoutMs`, releasing the
|
||||
// directory bootstrap concurrency slot. Long-lived streams must be excluded:
|
||||
// POST (prompt/shell/summarize/command) and the `/event` SSE stream.
|
||||
|
||||
type CapturedFetch = (input: string | URL | Request, init?: RequestInit) => Promise<Response>;
|
||||
type RuntimeFetch = (input: string | URL | Request, init?: RequestInit) => Promise<Response>;
|
||||
|
||||
// `mock(...)` returns a Mock that exposes mockImplementation; keep a typed
|
||||
// reference so per-test overrides stay type-safe without re-importing.
|
||||
const runtimeFetchMock = mock<RuntimeFetch>(async () => new Response('', { status: 200 }));
|
||||
|
||||
let capturedFetch: CapturedFetch | null = null;
|
||||
|
||||
(mock as unknown as { restore?: () => void }).restore?.();
|
||||
|
||||
mock.module('@opencode-ai/sdk/v2', () => ({
|
||||
createOpencodeClient: mock((opts: { fetch: CapturedFetch }) => {
|
||||
capturedFetch = opts.fetch;
|
||||
return {};
|
||||
}),
|
||||
}));
|
||||
|
||||
mock.module('@/contexts/runtimeAPIRegistry', () => ({
|
||||
getRegisteredRuntimeAPIs: mock(() => null),
|
||||
}));
|
||||
|
||||
mock.module('@/lib/runtime-url', () => ({
|
||||
getRuntimeUrlResolver: mock(() => ({ api: (path: string) => path })),
|
||||
}));
|
||||
|
||||
mock.module('@/lib/runtime-switch', () => ({
|
||||
getRuntimeApiBaseUrl: mock(() => ''),
|
||||
getRuntimeKey: mock(() => 'test-runtime'),
|
||||
}));
|
||||
|
||||
mock.module('@/lib/runtime-fetch', () => ({
|
||||
runtimeFetch: runtimeFetchMock,
|
||||
}));
|
||||
|
||||
mock.module('@/lib/startupTrace', () => ({
|
||||
markStartupTrace: mock(() => undefined),
|
||||
}));
|
||||
|
||||
const { createRuntimeOpencodeClient } = await import(
|
||||
`./client?timeout-final=${Date.now()}`
|
||||
);
|
||||
|
||||
beforeEach(() => {
|
||||
capturedFetch = null;
|
||||
runtimeFetchMock.mockImplementation(async () => new Response('', { status: 200 }));
|
||||
});
|
||||
|
||||
describe('createRuntimeOpencodeClient fetch wrapper (#2470)', () => {
|
||||
test('AbortSignal.timeout fires inside a test environment (sanity)', async () => {
|
||||
const sig = AbortSignal.timeout(20);
|
||||
const fired = await new Promise<boolean>((resolve) => {
|
||||
sig.addEventListener('abort', () => resolve(true));
|
||||
setTimeout(() => resolve(false), 200);
|
||||
});
|
||||
expect(fired).toBe(true);
|
||||
});
|
||||
|
||||
test('createRuntimeOpencodeClient is exported', () => {
|
||||
expect(typeof createRuntimeOpencodeClient).toBe('function');
|
||||
});
|
||||
|
||||
test('a GET whose socket never settles rejects with normalized "request timed out" error', async () => {
|
||||
let firedAt = 0;
|
||||
let calledAt = Date.now();
|
||||
runtimeFetchMock.mockImplementation(async (_input: string | URL | Request, init?: RequestInit) => {
|
||||
calledAt = Date.now();
|
||||
return new Promise<Response>((_resolve, reject) => {
|
||||
init?.signal?.addEventListener('abort', () => {
|
||||
firedAt = Date.now();
|
||||
reject(new DOMException('Aborted', 'AbortError'));
|
||||
});
|
||||
setTimeout(
|
||||
() => reject(new Error('TIMEOUT_TEST_FAIL: signal never fired')),
|
||||
1000,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
createRuntimeOpencodeClient({ baseUrl: '', requestTimeoutMs: 25 });
|
||||
expect(capturedFetch).not.toBeNull();
|
||||
|
||||
const start = Date.now();
|
||||
await expect(
|
||||
capturedFetch!('http://opencode.test/api/session'),
|
||||
).rejects.toThrow(/request timed out/);
|
||||
const elapsed = Date.now() - start;
|
||||
expect(firedAt).toBeGreaterThanOrEqual(calledAt);
|
||||
expect(elapsed).toBeLessThan(500);
|
||||
});
|
||||
|
||||
test('POST requests (long-running prompt/shell/summarize) are NOT timed out', async () => {
|
||||
runtimeFetchMock.mockImplementation(async () => {
|
||||
await new Promise((r) => setTimeout(r, 200));
|
||||
return new Response('ok', { status: 200 });
|
||||
});
|
||||
|
||||
createRuntimeOpencodeClient({ baseUrl: '', requestTimeoutMs: 25 });
|
||||
expect(capturedFetch).not.toBeNull();
|
||||
|
||||
const response = await capturedFetch!(
|
||||
'http://opencode.test/session/prompt',
|
||||
{ method: 'POST' },
|
||||
);
|
||||
expect(await response.text()).toBe('ok');
|
||||
});
|
||||
|
||||
test('the /event SSE stream is NOT timed out', async () => {
|
||||
runtimeFetchMock.mockImplementation(async () => {
|
||||
await new Promise((r) => setTimeout(r, 200));
|
||||
return new Response('event: ping\n\n', {
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'text/event-stream' },
|
||||
});
|
||||
});
|
||||
|
||||
createRuntimeOpencodeClient({ baseUrl: '', requestTimeoutMs: 25 });
|
||||
expect(capturedFetch).not.toBeNull();
|
||||
|
||||
const response = await capturedFetch!(
|
||||
'http://opencode.test/api/global/event',
|
||||
);
|
||||
expect(response.headers.get('Content-Type')).toBe('text/event-stream');
|
||||
});
|
||||
|
||||
test('caller-provided abort signal still wins (no normalized timeout error)', async () => {
|
||||
runtimeFetchMock.mockImplementation(async (_input: string | URL | Request, init?: RequestInit) =>
|
||||
new Promise<Response>((_, reject) => {
|
||||
init?.signal?.addEventListener('abort', () => {
|
||||
reject(new DOMException('Aborted', 'AbortError'));
|
||||
});
|
||||
}),
|
||||
);
|
||||
|
||||
createRuntimeOpencodeClient({ baseUrl: '', requestTimeoutMs: 25 });
|
||||
expect(capturedFetch).not.toBeNull();
|
||||
|
||||
const callerController = new AbortController();
|
||||
setTimeout(() => callerController.abort(), 5);
|
||||
|
||||
await expect(
|
||||
capturedFetch!('http://opencode.test/api/session', {
|
||||
signal: callerController.signal,
|
||||
}),
|
||||
).rejects.toThrow('Aborted');
|
||||
});
|
||||
});
|
||||
@@ -200,10 +200,86 @@ const createTimeoutSignal = (timeoutMs: number): { signal: AbortSignal; cleanup:
|
||||
};
|
||||
};
|
||||
|
||||
const createRuntimeOpencodeClient = (config: { baseUrl: string; directory?: string }): OpencodeClient => {
|
||||
/**
|
||||
* Upper bound for non-streaming OpenCode read requests. Without it, a socket
|
||||
* that neither resolves nor rejects (the half-open state described in #2470)
|
||||
* keeps the bootstrap concurrency slot busy forever and the UI stays on
|
||||
* "loading sessions". Long-lived streams (POST prompts, the /event SSE) are
|
||||
* explicitly excluded in {@link createRuntimeOpencodeClient}.
|
||||
*/
|
||||
const OPENCODE_REQUEST_TIMEOUT_MS = 30_000;
|
||||
|
||||
const isEventStreamUrl = (input: string | URL | Request): boolean => {
|
||||
const url = typeof input === 'string'
|
||||
? input
|
||||
: input instanceof URL
|
||||
? input.toString()
|
||||
: input.url;
|
||||
return url.includes('/event');
|
||||
};
|
||||
|
||||
type RuntimeOpencodeClientConfig = {
|
||||
baseUrl: string;
|
||||
directory?: string;
|
||||
/** Read-request timeout in ms. Overridable so tests can use short value. */
|
||||
requestTimeoutMs?: number;
|
||||
};
|
||||
|
||||
export const createRuntimeOpencodeClient = (config: RuntimeOpencodeClientConfig): OpencodeClient => {
|
||||
const requestTimeoutMs = config.requestTimeoutMs ?? OPENCODE_REQUEST_TIMEOUT_MS;
|
||||
return createOpencodeClient({
|
||||
...config,
|
||||
fetch: runtimeFetch,
|
||||
fetch: async (input: string | URL | Request, init?: RequestInit) => {
|
||||
const method = String(
|
||||
init?.method ?? (input instanceof Request ? input.method : 'GET'),
|
||||
).toUpperCase();
|
||||
if (isEventStreamUrl(input) || method === 'POST') {
|
||||
return runtimeFetch(input, init);
|
||||
}
|
||||
const timeout = createTimeoutSignal(requestTimeoutMs);
|
||||
const callerSignal = init?.signal;
|
||||
const supportsAny = typeof AbortSignal !== 'undefined'
|
||||
&& typeof (AbortSignal as { any?: unknown }).any === 'function';
|
||||
let signal: AbortSignal;
|
||||
let detachFallback: (() => void) | null = null;
|
||||
if (callerSignal && supportsAny) {
|
||||
signal = (AbortSignal as typeof AbortSignal & { any: (signals: AbortSignal[]) => AbortSignal })
|
||||
.any([callerSignal, timeout.signal]);
|
||||
} else if (callerSignal) {
|
||||
// No AbortSignal.any: compose manually. Silently dropping the timeout
|
||||
// here would disable the fix on exactly the bootstrap reads it
|
||||
// targets, since those carry a cancellation signal.
|
||||
const controller = new AbortController();
|
||||
const abortFromCaller = () => controller.abort(callerSignal.reason);
|
||||
const abortFromTimeout = () => controller.abort(timeout.signal.reason);
|
||||
if (callerSignal.aborted) {
|
||||
abortFromCaller();
|
||||
} else if (timeout.signal.aborted) {
|
||||
abortFromTimeout();
|
||||
} else {
|
||||
callerSignal.addEventListener('abort', abortFromCaller, { once: true });
|
||||
timeout.signal.addEventListener('abort', abortFromTimeout, { once: true });
|
||||
detachFallback = () => {
|
||||
callerSignal.removeEventListener('abort', abortFromCaller);
|
||||
timeout.signal.removeEventListener('abort', abortFromTimeout);
|
||||
};
|
||||
}
|
||||
signal = controller.signal;
|
||||
} else {
|
||||
signal = timeout.signal;
|
||||
}
|
||||
try {
|
||||
return await runtimeFetch(input, { ...init, signal });
|
||||
} catch (error) {
|
||||
if (timeout.signal.aborted && !callerSignal?.aborted) {
|
||||
throw new Error(`OpenCode request timed out after ${requestTimeoutMs}ms`);
|
||||
}
|
||||
throw error;
|
||||
} finally {
|
||||
detachFallback?.();
|
||||
timeout.cleanup();
|
||||
}
|
||||
},
|
||||
});
|
||||
};
|
||||
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
|
||||
import { retry } from './retry';
|
||||
|
||||
describe('retry transient classification (#2470)', () => {
|
||||
test("'terminated' (undici half-open socket teardown) is retried", async () => {
|
||||
let attempts = 0;
|
||||
await expect(
|
||||
retry(async () => {
|
||||
attempts += 1;
|
||||
throw new TypeError('terminated');
|
||||
}),
|
||||
).rejects.toThrow('terminated');
|
||||
expect(attempts).toBe(3);
|
||||
});
|
||||
|
||||
test("normalized 'request timed out' (SDK read timeout) is retried", async () => {
|
||||
let attempts = 0;
|
||||
await expect(
|
||||
retry(async () => {
|
||||
attempts += 1;
|
||||
throw new Error('OpenCode request timed out after 30000ms');
|
||||
}),
|
||||
).rejects.toThrow('request timed out');
|
||||
expect(attempts).toBe(3);
|
||||
});
|
||||
|
||||
test('caller-initiated abort (AbortError) is NOT retried', async () => {
|
||||
let attempts = 0;
|
||||
await expect(
|
||||
retry(async () => {
|
||||
attempts += 1;
|
||||
throw new DOMException('Aborted', 'AbortError');
|
||||
}),
|
||||
).rejects.toThrow('Aborted');
|
||||
expect(attempts).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -6,6 +6,10 @@ export interface RetryOptions {
|
||||
retryIf?: (error: unknown) => boolean
|
||||
}
|
||||
|
||||
// undici tears down half-open upstream connection with `TypeError: terminated`
|
||||
// (exact failure from the #2470 logs); the SDK client also rejects reads with
|
||||
// normalized "request timed out" error after OPENCODE_REQUEST_TIMEOUT_MS.
|
||||
// Both are transient — managed process may be restarting.
|
||||
const TRANSIENT_MESSAGES = [
|
||||
"load failed",
|
||||
"network connection was lost",
|
||||
@@ -18,6 +22,8 @@ const TRANSIENT_MESSAGES = [
|
||||
"opencode api unavailable",
|
||||
"503",
|
||||
"502",
|
||||
"terminated",
|
||||
"request timed out",
|
||||
]
|
||||
|
||||
function isTransientError(error: unknown): boolean {
|
||||
|
||||
Vendored
+10
-1
@@ -32,8 +32,17 @@ declare module "bun:test" {
|
||||
export function beforeEach(fn: () => void | Promise<void>): void;
|
||||
export function afterEach(fn: () => void | Promise<void>): void;
|
||||
export function afterAll(fn: () => void | Promise<void>): void;
|
||||
export function mock<T extends (...args: never[]) => unknown>(fn?: T): T;
|
||||
// Mock<T> matches the bun:test runtime mock: T (callable) plus spy methods.
|
||||
// Tests that need to swap implementations at runtime cast through `Mock<T>`.
|
||||
export interface Mock<T extends (...args: never[]) => unknown> {
|
||||
(...args: Parameters<T>): ReturnType<T>;
|
||||
mockImplementation(fn: T): Mock<T>;
|
||||
mockReturnValue(value: ReturnType<T>): Mock<T>;
|
||||
mockReset(): Mock<T>;
|
||||
}
|
||||
export function mock<T extends (...args: never[]) => unknown>(fn?: T): Mock<T>;
|
||||
export namespace mock {
|
||||
function module(moduleName: string, factory: () => Record<string, unknown>): void;
|
||||
function restore(): void;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user