From 81f88c0fb48d2c1d3bdfef398475e6cff89b94ae Mon Sep 17 00:00:00 2001 From: Philipp Winterle Date: Thu, 17 Sep 2026 15:45:23 +0200 Subject: [PATCH] fix: Let the bridge client outlast the bridge's own request timeout The UI operation bridge gives each operation REQUEST_TIMEOUT_MS (30s) to complete, but every request from the client side used a fixed 2s socket timeout. Any operation that took longer than two seconds - a large events.get-response-body, or a UI that was briefly busy - failed with a bare ETIMEDOUT while the bridge was still waiting happily, so the server's 30s budget was unreachable and its error never arrived. The timeout now depends on what the request is waiting for: discovery requests keep failing fast at 2s, so a dead socket path doesn't hold up the paths behind it, while /api/execute waits just past the bridge's own limit, which lets the bridge's error - naming the operation - through. socketRequest is exported so the test can drive one known socket. Going through apiRequest would walk its candidate paths, and on macOS the last of those is the real server on the developer's machine. --- src/api/bridge-client.ts | 22 ++++- src/api/ui-operation-bridge.ts | 2 +- test/integration/bridge-client.spec.ts | 117 +++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 4 deletions(-) create mode 100644 test/integration/bridge-client.spec.ts diff --git a/src/api/bridge-client.ts b/src/api/bridge-client.ts index 227af49c..d738b5a7 100644 --- a/src/api/bridge-client.ts +++ b/src/api/bridge-client.ts @@ -5,7 +5,23 @@ import { promisify } from 'util'; import { getDeferred } from '@httptoolkit/util'; -import { getSocketPath } from './ui-operation-bridge'; +import { getSocketPath, REQUEST_TIMEOUT_MS } from './ui-operation-bridge'; + +// Discovery requests are answered from memory, so a dead socket path has to fail +// fast to leave time for the paths behind it: +const DISCOVERY_TIMEOUT_MS = 2000; + +// Executing an operation means waiting for the UI, which the bridge itself allows +// REQUEST_TIMEOUT_MS to answer. Staying just above that keeps that budget usable +// and lets the bridge's own error - which names the operation - reach the caller, +// instead of failing here first with a bare ETIMEDOUT. +const EXECUTE_TIMEOUT_MS = REQUEST_TIMEOUT_MS + 5000; + +export function requestTimeout(urlPath: string): number { + return urlPath === '/api/execute' + ? EXECUTE_TIMEOUT_MS + : DISCOVERY_TIMEOUT_MS; +} const execFileAsync = promisify(execFile); let darwinTempDirPromise: Promise | undefined; @@ -70,7 +86,7 @@ export async function apiRequest( throw createBridgeConnectionError(socketAttempts); } -function socketRequest( +export function socketRequest( socketPath: string, method: 'GET' | 'POST', urlPath: string, @@ -84,7 +100,7 @@ function socketRequest( headers: { 'Content-Type': 'application/json' }, - timeout: 2000 + timeout: requestTimeout(urlPath) }, (res) => { const chunks: Buffer[] = []; res.on('error', (err: any) => { diff --git a/src/api/ui-operation-bridge.ts b/src/api/ui-operation-bridge.ts index d2754cae..95bb5959 100644 --- a/src/api/ui-operation-bridge.ts +++ b/src/api/ui-operation-bridge.ts @@ -43,7 +43,7 @@ interface PendingRequest { timer: ReturnType; } -const REQUEST_TIMEOUT_MS = 30_000; +export const REQUEST_TIMEOUT_MS = 30_000; export async function getSocketPath(): Promise { if (process.platform === 'win32') { diff --git a/test/integration/bridge-client.spec.ts b/test/integration/bridge-client.spec.ts new file mode 100644 index 00000000..20c9eff3 --- /dev/null +++ b/test/integration/bridge-client.spec.ts @@ -0,0 +1,117 @@ +import * as os from 'os'; +import * as path from 'path'; +import * as fs from 'fs'; + +import { expect } from 'chai'; +import WebSocket, { WebSocketServer } from 'ws'; + +import { + UiOperationBridge, + HtkOperation, + REQUEST_TIMEOUT_MS +} from '../../src/api/ui-operation-bridge'; +import { socketRequest, requestTimeout } from '../../src/api/bridge-client'; + +const TEST_OPERATIONS: HtkOperation[] = [{ + name: 'proxy.get-config', + description: 'Get the proxy configuration', + category: 'proxy', + tiers: ['free'], + inputSchema: { type: 'object', properties: {} } +}]; + +// Connects a mock UI to the bridge, authenticates it and waits for readiness, +// exactly as the real UI does on startup. +async function connectMockUi(bridge: UiOperationBridge) { + const wss = new WebSocketServer({ port: 0 }); + + const pair = await new Promise<{ clientWs: WebSocket, wss: WebSocketServer }>((resolve) => { + wss.on('connection', (serverSideWs) => bridge.setWebSocket(serverSideWs as any)); + wss.on('listening', () => { + const { port } = wss.address() as { port: number }; + const clientWs = new WebSocket(`ws://127.0.0.1:${port}`); + clientWs.on('open', () => resolve({ clientWs, wss })); + }); + }); + + const authResult = new Promise((resolve) => { + pair.clientWs.on('message', function handler(data) { + if (JSON.parse(data.toString()).type !== 'auth-result') return; + pair.clientWs.removeListener('message', handler); + resolve(); + }); + }); + pair.clientWs.send(JSON.stringify({ type: 'auth', jwt: false })); + await authResult; + + pair.clientWs.send(JSON.stringify({ type: 'operations', operations: TEST_OPERATIONS })); + await new Promise((resolve) => bridge.once('ready', resolve)); + + return pair; +} + +describe("Bridge client", function () { + + // The slow test below deliberately waits out the previous 2 second client timeout: + this.timeout(10_000); + + let bridge: UiOperationBridge; + let pair: { clientWs: WebSocket, wss: WebSocketServer }; + let socketDir: string; + let socketPath: string; + + beforeEach(async () => { + socketDir = fs.mkdtempSync(path.join(os.tmpdir(), 'htk-bridge-client-test-')); + socketPath = path.join(socketDir, 'test.sock'); + + bridge = new UiOperationBridge(); + await bridge.startApiServer(socketPath); + pair = await connectMockUi(bridge); + }); + + afterEach(() => { + if (pair.clientWs.readyState === WebSocket.OPEN) pair.clientWs.close(); + pair.wss.close(); + bridge.destroy(); + + try { fs.rmSync(socketDir, { recursive: true, force: true }); } catch {} + }); + + it("should wait for slow operations, up to the bridge's own request timeout", async () => { + const slowResult = { port: 8000, ip: '127.0.0.1' }; + + // Serializing a large body in the UI can easily take longer than a moment: + pair.clientWs.on('message', (data) => { + const msg = JSON.parse(data.toString()); + if (msg.type !== 'request') return; + setTimeout(() => { + pair.clientWs.send(JSON.stringify({ + type: 'response', + id: msg.id, + result: slowResult + })); + }, 3000); + }); + + const result = await socketRequest(socketPath, 'POST', '/api/execute', { + name: 'proxy.get-config', + args: {} + }); + + expect(result).to.deep.equal(slowResult); + }); + + it("should not give up before the bridge itself would", () => { + // The bridge allows each operation REQUEST_TIMEOUT_MS to complete. If the + // client gives up first, that budget is unreachable and the bridge's own + // error message never gets to the user: + expect(requestTimeout('/api/execute')).to.be.greaterThan(REQUEST_TIMEOUT_MS); + }); + + it("should still give up quickly while looking for a server", () => { + // Discovery tries each candidate socket path in turn, so a dead path has + // to fail fast rather than hold up the ones behind it: + expect(requestTimeout('/api/status')).to.be.lessThan(REQUEST_TIMEOUT_MS); + expect(requestTimeout('/api/operations')).to.be.lessThan(REQUEST_TIMEOUT_MS); + }); +});