Skip to content

Commit 55e3322

Browse files
committed
fix(search): give the alpha/search relay its own total deadline (search.timeoutMs)
connectTimeoutMs is documented as the DNS/TCP/TLS/header-arrival budget, but the relay reused it as the TOTAL upstream deadline. alpha/search is non-streaming — headers arrive only when the search completes — so a typical 10s connect budget killed every long-running search. Mirror OcxImagesConfig.timeoutMs with a dedicated config.search.timeoutMs (default 200s), move the 504 test to the new knob, and add a regression test proving a short connectTimeoutMs no longer cuts a slow search. (PR #89 follow-up)
1 parent 84319e6 commit 55e3322

3 files changed

Lines changed: 52 additions & 3 deletions

File tree

src/server/search.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@ import { readJsonRequestBody } from "./request-decompress";
2626
import type { RequestLogContext } from "./request-log";
2727
import { codexLogAccountId, decodeRequestErrorResponse, sidecarOutcomeRecorder } from "./responses";
2828

29+
/**
30+
* Default TOTAL deadline for one search relay. alpha/search is non-streaming JSON — response
31+
* headers arrive only when the search finishes — so the budget must cover the whole request.
32+
* Overridable via config.search.timeoutMs; never config.connectTimeoutMs, whose documented
33+
* contract is the DNS/TCP/TLS/header-arrival budget (a 10s connect budget would kill every
34+
* long-running search).
35+
*/
2936
const SEARCH_UPSTREAM_TIMEOUT_MS = 200_000;
3037
const SEARCH_RESPONSE_MAX_BYTES = 16 * 1024 * 1024;
3138

@@ -103,7 +110,7 @@ export async function handleSearch(
103110
if (upstream.provider.headers) Object.assign(headers, upstream.provider.headers);
104111
for (const [name, value] of authHeaders) headers[name] = value;
105112
const url = `${upstream.provider.baseUrl}/alpha/search`;
106-
const timeoutMs = config.connectTimeoutMs ?? SEARCH_UPSTREAM_TIMEOUT_MS;
113+
const timeoutMs = config.search?.timeoutMs ?? SEARCH_UPSTREAM_TIMEOUT_MS;
107114
const linkedSignal = signalWithTimeout(timeoutMs, req.signal);
108115
const sidecarExit = sidecarEnter("search");
109116
try {

src/types.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,8 @@ export interface OcxConfig {
328328
visionSidecar?: OcxVisionSidecarConfig;
329329
/** /v1/images relay for codex's built-in image_gen tool. */
330330
images?: OcxImagesConfig;
331+
/** /v1/alpha/search relay for codex's built-in web search client. */
332+
search?: OcxSearchConfig;
331333
/** Codex multi-account pool. */
332334
codexAccounts?: CodexAccount[];
333335
/** Active pool account id for next session. undefined = main (passthrough as-is). */
@@ -378,6 +380,15 @@ export interface OcxImagesConfig {
378380
timeoutMs?: number;
379381
}
380382

383+
export interface OcxSearchConfig {
384+
/**
385+
* Total upstream deadline (ms) for one /v1/alpha/search relay. Default 200000. The endpoint
386+
* is non-streaming JSON (headers arrive only when the search completes), so this is a whole-
387+
* request budget — deliberately NOT connectTimeoutMs, which is a header-arrival budget.
388+
*/
389+
timeoutMs?: number;
390+
}
391+
381392
export interface OcxVisionSidecarConfig {
382393
/** Master switch. Default: enabled when a forward (ChatGPT) provider exists and the caller is logged in. */
383394
enabled?: boolean;

tests/server-search.test.ts

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ test("relays search upstream error status and body verbatim", async () => {
254254
}
255255
});
256256

257-
test("a hung search upstream times out with 504 after config.connectTimeoutMs", async () => {
257+
test("a hung search upstream times out with 504 after config.search.timeoutMs", async () => {
258258
const upstream = Bun.serve({
259259
port: 0,
260260
fetch(req) {
@@ -265,7 +265,7 @@ test("a hung search upstream times out with 504 after config.connectTimeoutMs",
265265
});
266266
saveConfig({
267267
...forwardConfig(upstream.url.toString().replace(/\/$/, "")),
268-
connectTimeoutMs: 100,
268+
search: { timeoutMs: 100 },
269269
} as OcxConfig);
270270

271271
const server = startServer(0);
@@ -284,6 +284,37 @@ test("a hung search upstream times out with 504 after config.connectTimeoutMs",
284284
}
285285
}, 5_000);
286286

287+
test("a short connectTimeoutMs does NOT cut a slow search (total deadline is search.timeoutMs)", async () => {
288+
// Regression: alpha/search is non-streaming, so its headers arrive only when the search
289+
// completes. Reusing connectTimeoutMs as the relay deadline killed every search longer than
290+
// the header-arrival budget (often ~10s in real configs).
291+
const upstream = Bun.serve({
292+
port: 0,
293+
async fetch() {
294+
await new Promise(resolve => setTimeout(resolve, 300));
295+
return Response.json({ output: "slow but fine" });
296+
},
297+
});
298+
saveConfig({
299+
...forwardConfig(upstream.url.toString().replace(/\/$/, "")),
300+
connectTimeoutMs: 50,
301+
} as OcxConfig);
302+
303+
const server = startServer(0);
304+
try {
305+
const response = await fetch(new URL("/v1/alpha/search", server.url), {
306+
method: "POST",
307+
headers: { "content-type": "application/json", authorization: "Bearer chatgpt-user-token" },
308+
body: JSON.stringify({ id: "search-session", model: "gpt-test" }),
309+
});
310+
expect(response.status).toBe(200);
311+
expect(await response.json()).toEqual({ output: "slow but fine" });
312+
} finally {
313+
await server.stop(true);
314+
await upstream.stop(true);
315+
}
316+
}, 5_000);
317+
287318
test("GET /v1/alpha/search still falls through to the JSON 404 guard", async () => {
288319
saveConfig(forwardConfig("https://chatgpt.example/backend-api/codex"));
289320

0 commit comments

Comments
 (0)