From 39bb71a62baa4e951190f89cf09deea892ed99de Mon Sep 17 00:00:00 2001 From: herjarsa Date: Mon, 17 Aug 2026 11:37:20 +0200 Subject: [PATCH] fix(ui): bound OpenCode read requests so half-open sockets cannot freeze bootstrap (#2470) The SDK client fetch wrapper now applies a 30s timeout to non-streaming reads. Without it, a socket that neither resolves nor rejects keeps the directory bootstrap concurrency slot busy forever and the UI stays on "loading sessions". Long-lived streams (POST prompts, the /event SSE) are explicitly excluded so they are not cut off mid-flight. The normalized "request timed out" error is added to the retry allowlist alongside undici's "terminated" (the exact failure observed in #2470 when undici tears down a half-open upstream connection); both are transient while the managed OpenCode process restarts. Caller- initiated aborts keep their original error shape so a user-cancelled request is not retried. Tests cover: GET timeout fires after the bound, POST is not timed out, /event SSE is not timed out, caller abort wins, AbortError is not retried, and the SDK normalized error is retried 3x. --- .../src/lib/opencode/client-timeout.test.ts | 157 ++++++++++++++++++ packages/ui/src/lib/opencode/client.ts | 55 +++++- packages/ui/src/sync/retry.test.ts | 38 +++++ packages/ui/src/sync/retry.ts | 6 + packages/ui/src/types/bun-test.d.ts | 11 +- 5 files changed, 264 insertions(+), 3 deletions(-) create mode 100644 packages/ui/src/lib/opencode/client-timeout.test.ts create mode 100644 packages/ui/src/sync/retry.test.ts diff --git a/packages/ui/src/lib/opencode/client-timeout.test.ts b/packages/ui/src/lib/opencode/client-timeout.test.ts new file mode 100644 index 00000000..fd45a4a7 --- /dev/null +++ b/packages/ui/src/lib/opencode/client-timeout.test.ts @@ -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; +type RuntimeFetch = (input: string | URL | Request, init?: RequestInit) => Promise; + +// `mock(...)` returns a Mock that exposes mockImplementation; keep a typed +// reference so per-test overrides stay type-safe without re-importing. +const runtimeFetchMock = mock(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((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((_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((_, 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'); + }); +}); diff --git a/packages/ui/src/lib/opencode/client.ts b/packages/ui/src/lib/opencode/client.ts index 37e12620..ae4000fb 100644 --- a/packages/ui/src/lib/opencode/client.ts +++ b/packages/ui/src/lib/opencode/client.ts @@ -199,10 +199,61 @@ 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'; + const signal: AbortSignal = callerSignal && supportsAny + ? (AbortSignal as typeof AbortSignal & { any: (signals: AbortSignal[]) => AbortSignal }) + .any([callerSignal, timeout.signal]) + : (callerSignal ?? 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 { + timeout.cleanup(); + } + }, }); }; diff --git a/packages/ui/src/sync/retry.test.ts b/packages/ui/src/sync/retry.test.ts new file mode 100644 index 00000000..d1819302 --- /dev/null +++ b/packages/ui/src/sync/retry.test.ts @@ -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); + }); +}); diff --git a/packages/ui/src/sync/retry.ts b/packages/ui/src/sync/retry.ts index 8c7aafea..0b3b0a0d 100644 --- a/packages/ui/src/sync/retry.ts +++ b/packages/ui/src/sync/retry.ts @@ -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 { diff --git a/packages/ui/src/types/bun-test.d.ts b/packages/ui/src/types/bun-test.d.ts index 921149c1..f7304d74 100644 --- a/packages/ui/src/types/bun-test.d.ts +++ b/packages/ui/src/types/bun-test.d.ts @@ -31,8 +31,17 @@ declare module "bun:test" { export function beforeEach(fn: () => void | Promise): void; export function afterEach(fn: () => void | Promise): void; export function afterAll(fn: () => void | Promise): void; - export function mock unknown>(fn?: T): T; + // Mock matches the bun:test runtime mock: T (callable) plus spy methods. + // Tests that need to swap implementations at runtime cast through `Mock`. + export interface Mock unknown> { + (...args: Parameters): ReturnType; + mockImplementation(fn: T): Mock; + mockReturnValue(value: ReturnType): Mock; + mockReset(): Mock; + } + export function mock unknown>(fn?: T): Mock; export namespace mock { function module(moduleName: string, factory: () => Record): void; + function restore(): void; } }