Skip to content

fix: improve account rotation with request timeouts and interruption safety #523

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

Open
9nunya wants to merge 8 commits into
base: main
Choose a base branch
from
Open

fix: improve account rotation with request timeouts and interruption safety #523

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
52d9b51
fix: improve account rotation with request timeouts and interruption …
9nunya Mar 2, 2026
3757ec3
fix: add AbortSignal.any polyfill for Node.js <20.17.0 compatibility
9nunya Mar 2, 2026
f12df55
fix: refine proactive quota refresh and stabilize identifier lookup
9nunya Mar 2, 2026
b1306a4
fix: increase default timeout and implement streaming multiplier
9nunya Mar 2, 2026
203d4db
fix: prioritize user abort signal check in request loop catch block
9nunya Mar 2, 2026
a4745c1
fix: refine proactive refresh logic and persist timeout cooldowns
9nunya Mar 2, 2026
506698b
fix: wrap account persistence in try-catch to prevent rotation failur…
9nunya Mar 2, 2026
2b9b575
fix: enhance AbortSignal compatibility and fix listener leaks
9nunya Mar 2, 2026
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
22 changes: 11 additions & 11 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

89 changes: 83 additions & 6 deletions src/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import { AccountManager, type ModelFamily, parseRateLimitReason, calculateBackof
import { createAutoUpdateCheckerHook } from "./hooks/auto-update-checker";
import { loadConfig, initRuntimeConfig, type AntigravityConfig } from "./plugin/config";
import { createSessionRecoveryHook, getRecoverySuccessToast } from "./plugin/recovery";
import { checkAccountsQuota } from "./plugin/quota";
import { checkAccountsQuota, triggerAsyncQuotaRefreshForAll } from "./plugin/quota";
import { initDiskSignatureCache } from "./plugin/cache";
import { createProactiveRefreshQueue, type ProactiveRefreshQueue } from "./plugin/refresh-queue";
import { initLogger, createLogger } from "./plugin/logger";
Expand Down Expand Up @@ -80,6 +80,32 @@ let childSessionParentID: string | undefined = undefined;

const log = createLogger("plugin");

/**
* [Node.js Compatibility Polyfill]
* AbortSignal.any() was added in Node 20.17.0.
* This project supports Node >= 20.0.0, so we provide a polyfill
* for older 20.x releases.
*/
if (typeof (AbortSignal as any).any !== "function") {
(AbortSignal as any).any = function (signals: AbortSignal[]): AbortSignal {
const controller = new AbortController();
for (const signal of signals) {
if (signal.aborted) {
controller.abort(signal.reason);
return controller.signal;
}
signal.addEventListener(
"abort",
() => {
controller.abort(signal.reason);
},
{ once: true }
);
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== AbortSignal.any polyfill block =="
sed -n '83,110p' src/plugin.ts

echo
echo "== Check for listener cleanup in polyfill =="
rg -n 'removeEventListener|cleanup|listeners' src/plugin.ts

echo
echo "== Full polyfill function (lines 83-120) =="
sed -n '83,120p' src/plugin.ts

Repository: NoeFabris/opencode-antigravity-auth

Length of output: 2461

Polyfill should clean up sibling listeners after first abort.

When any signal aborts, remaining listeners attached to other signals are never detached. The { once: true } option only removes listeners after they fire, not before. In fallback runtimes (Node 20.0-20.16), repeated requests with overlapping signals cause listeners to accumulate on those signals, creating a memory leak.

Proposed fix 
 (AbortSignal as any).any = function (signals: AbortSignal[]): AbortSignal {
   const controller = new AbortController();
+  const listeners = new Map<AbortSignal, () => void>();
+  const cleanup = () => {
+    for (const [s, fn] of listeners) s.removeEventListener("abort", fn);
+    listeners.clear();
+  };
   for (const signal of signals) {
     if (signal.aborted) {
       controller.abort(signal.reason);
       return controller.signal;
     }
-    signal.addEventListener(
-      "abort",
-      () => {
-        controller.abort(signal.reason);
-      },
-      { once: true }
-    );
+    const onAbort = () => {
+      cleanup();
+      controller.abort(signal.reason);
+    };
+    listeners.set(signal, onAbort);
+    signal.addEventListener("abort", onAbort, { once: true });
   }
   return controller.signal;
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
(AbortSignal as any).any = function (signals: AbortSignal[]): AbortSignal {
const controller = new AbortController();
for (const signal of signals) {
if (signal.aborted) {
controller.abort(signal.reason);
return controller.signal;
}
signal.addEventListener(
"abort",
() => {
controller.abort(signal.reason);
},
{ once: true }
);
(AbortSignal as any).any = function (signals: AbortSignal[]): AbortSignal {
const controller = new AbortController();
const listeners = new Map<AbortSignal, () => void>();
const cleanup = () => {
for (const [s, fn] of listeners) s.removeEventListener("abort", fn);
listeners.clear();
};
for (const signal of signals) {
if (signal.aborted) {
controller.abort(signal.reason);
return controller.signal;
}
const onAbort = () => {
cleanup();
controller.abort(signal.reason);
};
listeners.set(signal, onAbort);
signal.addEventListener("abort", onAbort, { once: true });
}
return controller.signal;
};
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/plugin.ts` around lines 90 - 103, The polyfill for (AbortSignal as
any).any currently adds listeners but never detaches sibling listeners when one
signal aborts; update the any implementation to track each listener (e.g., store
a Map<AbortSignal, EventListener>) when attaching via addEventListener, and in
both the immediate-aborted branch and inside the abort callback call
controller.abort(signal.reason) then iterate over the other signals and call
removeEventListener with the stored listener to detach them, ensuring you still
return controller.signal; also ensure you clean up listeners if the returned
controller.signal is aborted from elsewhere to avoid leaks.

}
return controller.signal;
};
}

// Module-level toast debounce to persist across requests (fixes toast spam)
const rateLimitToastCooldowns = new Map<string, number>();
const RATE_LIMIT_TOAST_COOLDOWN_MS = 5000;
Expand All @@ -90,7 +116,7 @@ let softQuotaToastShown = false;
let rateLimitToastShown = false;

// Module-level reference to AccountManager for access from auth.login
let activeAccountManager: import("./plugin/accounts").AccountManager | null = null;
let activeAccountManager: AccountManager | null = null;

function cleanupToastCooldowns(): void {
if (rateLimitToastCooldowns.size > MAX_TOAST_COOLDOWN_ENTRIES) {
Expand Down Expand Up @@ -157,9 +183,8 @@ async function triggerAsyncQuotaRefreshForAccount(

const results = await checkAccountsQuota([singleAccount], client, providerId);

if (results[0]?.status === "ok" && results[0]?.quota?.groups) {
accountManager.updateQuotaCache(accountIndex, results[0].quota.groups);
accountManager.requestSaveToDisk();
if (results[0]?.status === "ok" && results[0]?.quota?.groups && results[0]?.refreshToken) {
accountManager.updateQuotaCache(results[0].refreshToken, results[0].quota.groups);
}
} catch (err) {
log.debug(`quota-refresh-failed email=${accountKey}`, { error: String(err) });
Expand Down Expand Up @@ -1963,6 +1988,8 @@ export const createAntigravityPlugin = (providerId: string) => async (
continue;
}

let effectiveTimeoutMs = (config.request_timeout_seconds ?? 600) * 1000;
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 2, 2026

Choose a reason for hiding this comment

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

declared but immediately overwritten at line 2100, making this initialization unnecessary

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/plugin.ts
Line: 2029

Comment:
declared but immediately overwritten at line 2100, making this initialization unnecessary

How can I resolve this? If you propose a fix, please make it concise.


try {
const prepared = prepareAntigravityRequest(
input,
Expand Down Expand Up @@ -2022,7 +2049,29 @@ export const createAntigravityPlugin = (providerId: string) => async (
tokenConsumed = getTokenTracker().consume(account.index);
}

const response = await fetch(prepared.request, prepared.init);
// Check if we should proactively refresh all quotas (only on first endpoint attempt)
if (i === 0 && accountManager.shouldRefreshAllQuotas(family, config.soft_quota_threshold_percent, softQuotaCacheTtlMs, model)) {
pushDebug("proactive-quota-refresh: pool is mostly blocked, refreshing all");
void triggerAsyncQuotaRefreshForAll(accountManager, client, providerId);
}

// Create a combined signal for timeout and user abort
const timeoutMs = (config.request_timeout_seconds ?? 600) * 1000;
// For streaming, we allow up to 3x the timeout (max 30 mins) to account for long generations
// while still catching truly "stuck" connections.
effectiveTimeoutMs = prepared.streaming
? Math.min(timeoutMs * 3, 1800000)
: timeoutMs;

const timeoutSignal = AbortSignal.timeout(effectiveTimeoutMs);
const combinedSignal = abortSignal
? (AbortSignal as any).any([abortSignal, timeoutSignal])
Copy link
Contributor

@kilo-code-bot kilo-code-bot bot Mar 2, 2026

Choose a reason for hiding this comment

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

CRITICAL: AbortSignal.any() compatibility issue - may cause runtime errors

AbortSignal.any() was added in Node.js 20.17.0, but this project supports Node.js >=20.0.0. Users on Node.js 20.0.0 - 20.16.x will encounter a runtime error.

Suggested change
? (AbortSignal as any).any([abortSignal, timeoutSignal])
// Create a combined signal for timeout and user abort
const timeoutMs = (config.request_timeout_seconds ?? 180) * 1000;
const timeoutSignal = AbortSignal.timeout(timeoutMs);
const combinedSignal = abortSignal
? (abortSignal.any ? abortSignal.any([abortSignal, timeoutSignal]) : mergeAbortSignals(abortSignal, timeoutSignal))
: timeoutSignal;

Alternatively, you could add a polyfill at the top of the file:

// Polyfill for older Node.js versions
if (typeof AbortSignal.any === 'undefined') {
  AbortSignal.any = function(signals: AbortSignal[]): AbortSignal {
    const controller = new AbortController();
    for (const signal of signals) {
      if (signal.aborted) {
        controller.abort(signal.reason);
        break;
      }
      signal.addEventListener('abort', () => {
        controller.abort(signal.reason);
      }, { once: true });
    }
    return controller.signal;
  };
}

: timeoutSignal;

const response = await fetch(prepared.request, {
...prepared.init,
signal: combinedSignal
});
pushDebug(`status=${response.status} ${response.statusText}`);


Expand Down Expand Up @@ -2403,6 +2452,34 @@ export const createAntigravityPlugin = (providerId: string) => async (
tokenConsumed = false;
}

// [CRITICAL] Check for user interruption FIRST before any error classification
if (abortSignal?.aborted) {
// User pressed ESC - stop everything immediately to prevent spin loop and memory leak
pushDebug("user-interrupted: stopping request loop");
throw error;
}

// Check for timeout errors
if (error instanceof Error && (error.name === "AbortError" || error.name === "TimeoutError")) {
// This was a request timeout (stuck account)
const actualTimeoutSec = Math.round(effectiveTimeoutMs / 1000);
pushDebug(`request-timeout: account ${account.index} stuck for ${actualTimeoutSec}s, rotating`);
getHealthTracker().recordFailure(account.index);
accountManager.markAccountCoolingDown(account, 60000, "network-error");

// Persist cooldown immediately so it survives restarts
await accountManager.saveToDisk();

await showToast(
`⏳ Account stuck (${actualTimeoutSec}s). Rotating to next available...`,
"warning"
);

shouldSwitchAccount = true;
lastError = error;
break;
}

// Handle recoverable thinking errors - retry with forced recovery
if (error instanceof Error && error.message === "THINKING_RECOVERY_NEEDED") {
// Only retry once with forced recovery to avoid infinite loops
Expand Down
Loading