mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-02 21:32:10 +03:00
fix(sse): call the real abort-signal helper in the Gemini Business executor (#8485)
`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.
This commit is contained in:
@@ -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";
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
Reference in New Issue
Block a user