Skip to content

fix(combo): add per-model timeout to prevent custom combos from hanging indefinitely #2717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
diegosouzapw merged 1 commit into from
May 26, 2026
Merged

fix(combo): add per-model timeout to prevent custom combos from hanging indefinitely #2717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 41 additions & 14 deletions open-sse/services/combo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ function isAllAccountsRateLimitedResponse(
const MAX_COMBO_DEPTH = 3;
const MAX_FALLBACK_WAIT_MS = 5000;
const MAX_GLOBAL_ATTEMPTS = 30;
const COMBO_MODEL_TIMEOUT_MS = 30_000; // 30s per model attempt within a combo (default FETCH_TIMEOUT_MS=600s)

function resolveDelayMs(value: unknown, fallback: number): number {
const numericValue = Number(value);
Expand Down Expand Up @@ -1645,21 +1646,47 @@ export async function handleComboChat({
: handleSingleModel;
// ─────────────────────────────────────────────────────────────────────────

// ── Per-model timeout wrapper ────────────────────────────────────────────
// Default FETCH_TIMEOUT_MS is 600s per model. For combos, we use a shorter
// per-model timeout so slow/hanging models don't block fallback.
const handleSingleModelWithTimeout = async (b, modelStr, target?) => {
let timeoutId;
const timeoutPromise = new Promise((resolve) => {
timeoutId = setTimeout(() => {
log.warn("COMBO", `Model ${modelStr} exceeded ${COMBO_MODEL_TIMEOUT_MS}ms timeout — falling back`);
resolve(new Response(
JSON.stringify({ error: { message: `Model ${modelStr} timed out` } }),
{ status: 524, headers: { 'Content-Type': 'application/json' } }
));
}, COMBO_MODEL_TIMEOUT_MS);
});
try {
return await Promise.race([
handleSingleModelWrapped(b, modelStr, target).catch((err) => {
return new Response(JSON.stringify({ error: { message: err.message } }), { status: 502, headers: { "Content-Type": "application/json" } });
}),
timeoutPromise,
]);
} finally {
clearTimeout(timeoutId);
}
};
Comment on lines +1652 to +1673

@gemini-code-assist gemini-code-assist Bot May 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Issue 1: Potential Crash on Null/Undefined Errors

In the .catch block of handleSingleModelWrapped, err.message is accessed directly. If err is null, undefined, or a non-object (e.g., a string or raw network error), this will throw a TypeError, potentially causing an unhandled promise rejection or crashing the process.

Issue 2: Lack of Abort Signal Responsiveness

While handleComboChat checks signal?.aborted between attempts, handleSingleModelWithTimeout does not listen to the signal during the active Promise.race. If a client disconnects while a model is hanging, the server will continue to wait for the full 30-second timeout (or until the model responds) before aborting the loop. This leads to wasted resources and unnecessary upstream API costs.

Solution

We can resolve both issues by:

  1. Guarding the error message extraction using err instanceof Error or fallback string conversion.
  2. Racing the model call and timeout promise against an abortPromise that resolves immediately to a 499 status when the client disconnects.
  const handleSingleModelWithTimeout = async (b, modelStr, target?) => {
    if (signal?.aborted) {
      return new Response(
        JSON.stringify({ error: { message: "Client disconnected" } }),
        { status: 499, headers: { "Content-Type": "application/json" } }
      );
    }
    let timeoutId;
    let abortHandler;
    const timeoutPromise = new Promise((resolve) => {
      timeoutId = setTimeout(() => {
        log.warn("COMBO", "Model " + modelStr + " exceeded " + COMBO_MODEL_TIMEOUT_MS + "ms timeout — falling back");
        resolve(new Response(
          JSON.stringify({ error: { message: "Model " + modelStr + " timed out" } }),
          { status: 524, headers: { "Content-Type": "application/json" } }
        ));
      }, COMBO_MODEL_TIMEOUT_MS);
    });
    const abortPromise = new Promise((resolve) => {
      if (signal) {
        abortHandler = () => {
          resolve(new Response(
            JSON.stringify({ error: { message: "Client disconnected" } }),
            { status: 499, headers: { "Content-Type": "application/json" } }
          ));
        };
        signal.addEventListener("abort", abortHandler, { once: true });
      }
    });
    try {
      return await Promise.race([
        handleSingleModelWrapped(b, modelStr, target).catch((err) => {
          const message = err instanceof Error ? err.message : String(err || "Unknown error");
          return new Response(JSON.stringify({ error: { message } }), { status: 502, headers: { "Content-Type": "application/json" } });
        }),
        timeoutPromise,
        abortPromise,
      ]);
    } finally {
      clearTimeout(timeoutId);
      if (signal && abortHandler) {
        signal.removeEventListener("abort", abortHandler);
      }
    }
  };


// Route to pinned model if context caching specifies one (Fix #679)
if (pinnedModel) {
log.info(
"COMBO",
`Bypassing strategy — routing directly to pinned context model: ${pinnedModel}`
);
return handleSingleModelWrapped(body, pinnedModel);
return handleSingleModelWithTimeout(body, pinnedModel);
}

// Route to round-robin handler if strategy matches
if (strategy === "round-robin") {
return handleRoundRobinCombo({
body,
combo,
handleSingleModel: handleSingleModelWrapped,
handleSingleModel: handleSingleModelWithTimeout,
isModelAvailable,
log,
settings,
Expand Down Expand Up @@ -1703,7 +1730,7 @@ export async function handleComboChat({
const pipelineRaw = await handlePipelineCombo({
body,
combo,
handleChatCore: handleSingleModel,
handleChatCore: handleSingleModelWithTimeout,
log,
settings,
signal,
Expand Down Expand Up @@ -2155,7 +2182,7 @@ export async function handleComboChat({
);
}
}
const result = await handleSingleModelWrapped(body, modelStr, {
const result = await handleSingleModelWithTimeout(body, modelStr, {
...target,
failoverBeforeRetry: config.failoverBeforeRetry,
});
Expand Down Expand Up @@ -2218,14 +2245,6 @@ export async function handleComboChat({
relayOptions?.sessionId &&
!(body as Record<string, unknown>)?.[SKIP_UNIVERSAL_HANDOFF_FLAG]
) {
recordSessionModelUsage(
relayOptions.sessionId,
combo.name,
modelStr,
provider,
target.connectionId
);

const prevModel = getLastSessionModel(relayOptions.sessionId, combo.name);
if (prevModel && prevModel !== modelStr) {
const handoffSourceMessages =
Expand All @@ -2242,9 +2261,17 @@ export async function handleComboChat({
prevModel,
currModel: modelStr,
universalConfig: universalHandoffConfig,
handleSingleModel: handleSingleModelWrapped,
handleSingleModel: handleSingleModelWithTimeout,
});
}

recordSessionModelUsage(
relayOptions.sessionId,
combo.name,
modelStr,
provider,
target.connectionId
);
}
// Context-relay intentionally splits responsibilities:
// combo.ts decides whether a successful turn should generate a handoff,
Expand Down Expand Up @@ -2279,7 +2306,7 @@ export async function handleComboChat({
model: modelStr,
expiresAt: resetCandidates[0] || null,
config: relayConfig,
handleSingleModel: handleSingleModelWrapped,
handleSingleModel: handleSingleModelWithTimeout,
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/db/contextHandoffs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ export function getLastSessionModel(sessionId: string, comboName: string): strin
`SELECT model_str
FROM session_model_history
WHERE session_id = ? AND combo_name = ?
ORDER BY used_at DESC
ORDER BY id DESC
LIMIT 1`
)
.get(sessionId, comboName) as { model_str: string } | undefined;
Expand Down