From 3f2bf86c3c202b7c6ba9a8cb1d32d586eb55cc8a Mon Sep 17 00:00:00 2001 From: backryun Date: Sat, 25 Jul 2026 14:53:13 +0900 Subject: [PATCH] fix(sse): call the real abort-signal helper in the Gemini Business executor (#8485) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gemini-business.ts` built its upstream fetch options with `combineAbortSignals(...)`, which is defined nowhere in the repository. The module imports `mergeAbortSignals` from `./base.ts` on line 31 and never used it — a rename that was only half applied. Because the call sits inside the fetch options object literal, the ReferenceError was thrown while *constructing* the arguments, before `fetch()` ran, and the surrounding try/catch turned it into `makeErrorResult(502, "Gemini Business network error: ...")`. So every Gemini Business request failed with what reads like an upstream outage. The provider is registered and reachable (`open-sse/executors/index.ts`), so this affects the whole provider, not an edge case. `mergeAbortSignals(primary, secondary)` requires two real signals while `ExecuteInput.signal` is `AbortSignal | null | undefined`, so the call is guarded and falls back to the timeout alone — the same shape huggingchat, grok-web, claude-web, and ninerouter already use. Why it went unnoticed: this file is only type-checked by `open-sse/tsconfig.json`, whose runs abort at `TS5101` (the deprecated `baseUrl`) before any file is checked, and `typecheck:core` covers a curated 26-file allowlist that excludes every executor. Removing that config error is #8473; this bug is what the first full run surfaced. TDD: the two new tests fail on the parent commit — `execute()` never reaches the stubbed `fetch` — and pass with the fix. They also cover the null-signal path, since that is where an unguarded `mergeAbortSignals` would throw next. --- open-sse/executors/gemini-business.ts | 7 +- tests/unit/gemini-business-provider.test.ts | 86 +++++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/open-sse/executors/gemini-business.ts b/open-sse/executors/gemini-business.ts index 9dadc37875..ea68969582 100644 --- a/open-sse/executors/gemini-business.ts +++ b/open-sse/executors/gemini-business.ts @@ -150,13 +150,18 @@ export class GeminiBusinessExecutor extends BaseExecutor { headers["Authorization"] = computeSapisidHash(sapisid, baseOrigin); } + // Cap the upstream call, and honor the caller's cancellation when there is one. + // `ExecuteInput.signal` is optional while mergeAbortSignals() needs two real signals, + // so fall back to the timeout alone — same guard the other web executors use. + const timeoutSignal = AbortSignal.timeout(GEMINI_BUSINESS_FETCH_TIMEOUT_MS); + let response: Response; try { response = await fetch(streamUrl, { method: "POST", headers, body: formBody.toString(), - signal: combineAbortSignals(signal, AbortSignal.timeout(GEMINI_BUSINESS_FETCH_TIMEOUT_MS)), + signal: signal ? mergeAbortSignals(signal, timeoutSignal) : timeoutSignal, }); } catch (err) { const message = err instanceof Error ? err.message : "fetch failed"; diff --git a/tests/unit/gemini-business-provider.test.ts b/tests/unit/gemini-business-provider.test.ts index b87d96bda0..2c7b41a992 100644 --- a/tests/unit/gemini-business-provider.test.ts +++ b/tests/unit/gemini-business-provider.test.ts @@ -77,6 +77,92 @@ test("GeminiBusinessExecutor.execute returns 400 when no user message is provide assert.ok(text.includes("No user message found")); }); +// ─── Upstream request path ────────────────────────────────────────────────── + +/** + * Regression guard: `execute()` built its fetch options with `combineAbortSignals(...)`, + * a function that exists nowhere in the codebase (the module imports `mergeAbortSignals` + * and never used it). Evaluating the options object threw `ReferenceError` *before* fetch + * was called; the surrounding try/catch turned that into a generic 502 "network error", + * so every Gemini Business request failed while looking like an upstream outage. + * + * It went unnoticed because `open-sse/tsconfig.json` could not be type-checked (the + * deprecated `baseUrl` aborted the run with TS5101) and `typecheck:core` only covers a + * curated 26-file allowlist that excludes this executor. + */ +test("GeminiBusinessExecutor.execute reaches the upstream fetch and passes an abort signal", async () => { + const ex = new GeminiBusinessExecutor(); + const originalFetch = globalThis.fetch; + let fetchCalled = false; + let receivedSignal: unknown; + + // Same wire shape the parseStreamResponse tests below use: inner[4][0] is a + // [metadata, text_list] pair. An empty body would take the "returned no text" 502 + // branch and mask what this test is actually asserting. + const inner = new Array(80).fill(null); + inner[4] = [[null, ["Hello from Gemini Business"]]]; + const upstreamBody = `[["wrb.fr", null, ${JSON.stringify(JSON.stringify(inner))}]]`; + + globalThis.fetch = (async (_url: unknown, init?: { signal?: unknown }) => { + fetchCalled = true; + receivedSignal = init?.signal; + return new Response(upstreamBody, { status: 200 }); + }) as typeof globalThis.fetch; + + try { + const result = await ex.execute({ + model: "gemini-2.5-pro", + body: { messages: [{ role: "user", content: "hello" }] }, + stream: false, + credentials: { apiKey: "__Secure-1PSID=fake; __Secure-1PSIDTS=fake" }, + signal: new AbortController().signal, + }); + + assert.equal(fetchCalled, true, "execute() must reach the upstream fetch"); + assert.ok( + receivedSignal instanceof AbortSignal, + "the upstream fetch must receive a combined AbortSignal" + ); + assert.notEqual( + result.response.status, + 502, + "a ReferenceError while building fetch options must not surface as an upstream 502" + ); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("GeminiBusinessExecutor.execute still applies a timeout when the caller passes no signal", async () => { + const ex = new GeminiBusinessExecutor(); + const originalFetch = globalThis.fetch; + let receivedSignal: unknown; + + globalThis.fetch = (async (_url: unknown, init?: { signal?: unknown }) => { + receivedSignal = init?.signal; + return new Response("", { status: 200 }); + }) as typeof globalThis.fetch; + + try { + // `ExecuteInput.signal` is `AbortSignal | null | undefined`; mergeAbortSignals() + // requires two real signals, so the null case must fall back to the timeout alone. + await ex.execute({ + model: "gemini-2.5-pro", + body: { messages: [{ role: "user", content: "hello" }] }, + stream: false, + credentials: { apiKey: "__Secure-1PSID=fake; __Secure-1PSIDTS=fake" }, + signal: null, + }); + + assert.ok( + receivedSignal instanceof AbortSignal, + "a timeout signal must still be applied when the caller supplies none" + ); + } finally { + globalThis.fetch = originalFetch; + } +}); + // ─── parseStreamResponse ──────────────────────────────────────────────────── test("parseStreamResponse extracts text from a single wrb.fr chunk", () => {