mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-17 04:12:17 +03:00
fix(sse): skip already-refused route per request on 429 (#13795)
On a 429 the opencode executor now records the refused proxy's key in the request-local tried-set, exactly like the 403/451, 5xx, stall and network arms already did — so a second account sharing that same proxy is not dialed and refused again before the loop reaches a genuinely different route (direct, or another proxy). Reviewed against the tip that already carries your 38 merges from this evening: this is additive to the flags that landed today (`OPENCODE_RATE_LIMITED_429_EARLY_STOP`, `PROXY_SKIP_RECENTLY_FAILED`, `OPENCODE_USER_BLOCKED_ROTATION`, `OPENCODE_TRANSIENT_FAILOVER_BACKOFF`) and does not double-skip when combined with them; direct accounts have a null proxy key and correctly record nothing. Validated as a combined board first (this PR merged with the 11 siblings of the same batch on the release tip): eslint on every changed file with the suppressions file, typecheck:core, check:open-sse-typecheck, complexity, cognitive-complexity, changelog-integrity, i18n new-key coverage, docs-sync, migration-numbering, provider-consistency and a duplicate-identifier audit all green, plus 275 passing / 0 failing focused node:test cases across the 28 test files the batch touches. Then re-validated alone on the fresh tip before this merge: conflicts re-resolved, file sizes rebaselined for this PR's own growth, eslint and this PR's focused tests re-run. Thanks @maxmad64bis!
This commit is contained in:
1
changelog.d/fixes/13795-opencode-429-proxy-dedup.md
Normal file
1
changelog.d/fixes/13795-opencode-429-proxy-dedup.md
Normal file
@@ -0,0 +1 @@
|
||||
- **fix(sse):** stop re-sending a request to an already-refused route after a 429 — each refused route is tried once per request ([#13795](https://github.com/diegosouzapw/OmniRoute/pull/13795)) — thanks @maxmad64bis
|
||||
@@ -621,8 +621,9 @@ export class OpencodeExecutor extends BaseExecutor {
|
||||
// persistently malformed upstream.
|
||||
const emptyRejectionBudget = this.accounts.length === 1 ? 1 : 0;
|
||||
// Tried set: proxy keys already proven unusable for this request's
|
||||
// model (geo-blocked, or transient 5xx). Request-local only — nothing
|
||||
// persists past execute().
|
||||
// model (geo-blocked, transient 5xx, or already-429 this request).
|
||||
// Request-local only — nothing persists past execute(). Cross-request
|
||||
// set-aside (noteProxyRefusal) applies on top when enabled.
|
||||
const geoTriedProxyKeys = new Set<string>();
|
||||
// Opt-in (PROXY_SKIP_RECENTLY_FAILED, default off): members the provider just refused
|
||||
// (received refusal or refused TCP probe) are skipped. Off = plain rotation.
|
||||
@@ -788,6 +789,8 @@ export class OpencodeExecutor extends BaseExecutor {
|
||||
const status = result.response.status;
|
||||
if (status === 429) {
|
||||
this.markCooldown(account);
|
||||
const key = proxyKeyOf(account.proxy);
|
||||
if (key !== null) geoTriedProxyKeys.add(key);
|
||||
// The provider refused through this member: set it aside beyond the account
|
||||
// cooldown. A direct account has a null key and is never set aside.
|
||||
const setAsideMs = skipRecentlyFailed
|
||||
@@ -808,7 +811,7 @@ export class OpencodeExecutor extends BaseExecutor {
|
||||
}
|
||||
log?.warn?.(
|
||||
"OPENCODE",
|
||||
`${cid}Rate limited (429) on account ${masked}` +
|
||||
`${cid}Rate limited (429) on account ${masked} (proxy ${key ?? "direct"})` +
|
||||
(setAsideMs ? `, member set aside for ${Math.round(setAsideMs / 1000)}s` : "") +
|
||||
", rotating to next…"
|
||||
);
|
||||
|
||||
91
tests/unit/opencode-429-proxy-dedup.test.ts
Normal file
91
tests/unit/opencode-429-proxy-dedup.test.ts
Normal file
@@ -0,0 +1,91 @@
|
||||
/**
|
||||
* Per-request refused-route skip on 429 — a request never re-sends to a route
|
||||
* the upstream just refused with 429 (keyed by host:port of the refused route).
|
||||
*
|
||||
* Goes through `execute()` with a stubbed fetch: the candidate predicate is
|
||||
* a non-exported closure and the geo-block helper is out of scope — no test
|
||||
* level without a new interface. Fixture entries injected via
|
||||
* `providerSpecificData` (`syncAccountsFromCredentials`), non-premium model
|
||||
* (avoids the 402 guard), stubbed fetch (the fire-and-forget reachability
|
||||
* probe never gates the first dispatch — `proxyFetch.ts:686-697`).
|
||||
* Sequential runs, no parallelism.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { OpencodeExecutor } = await import("../../open-sse/executors/opencode.ts");
|
||||
|
||||
const log = { debug() {}, info() {}, warn() {}, error() {} };
|
||||
|
||||
function proxy(host: string, port: number) {
|
||||
return { type: "http", host, port };
|
||||
}
|
||||
|
||||
function entriesFor(entries: Array<{ fingerprint: string; proxy: unknown }>) {
|
||||
return {
|
||||
providerSpecificData: {
|
||||
fingerprints: entries.map((e) => e.fingerprint),
|
||||
accountProxies: entries.map((e) => ({ fingerprint: e.fingerprint, proxy: e.proxy })),
|
||||
},
|
||||
} as never;
|
||||
}
|
||||
|
||||
async function runWith429Stub(credentials: never): Promise<{ calls: number; status: number }> {
|
||||
const exec = new OpencodeExecutor("opencode");
|
||||
let calls = 0;
|
||||
const originalFetch = globalThis.fetch;
|
||||
globalThis.fetch = (async () => {
|
||||
calls++;
|
||||
return new Response(JSON.stringify({ error: { message: "rate limited" } }), {
|
||||
status: 429,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
}) as typeof globalThis.fetch;
|
||||
try {
|
||||
const result = await exec.execute({
|
||||
model: "deepseek-v4-flash-free",
|
||||
body: { messages: [{ role: "user", content: "hi" }], stream: false },
|
||||
stream: false,
|
||||
signal: null,
|
||||
credentials,
|
||||
log,
|
||||
});
|
||||
return { calls, status: (result as { response: Response }).response.status };
|
||||
} finally {
|
||||
globalThis.fetch = originalFetch;
|
||||
}
|
||||
}
|
||||
|
||||
test("refused route skipped: refused route tried once per request", async () => {
|
||||
const shared = proxy("127.0.0.1", 18091);
|
||||
const { calls } = await runWith429Stub(
|
||||
entriesFor([
|
||||
{ fingerprint: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", proxy: shared },
|
||||
{ fingerprint: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", proxy: shared },
|
||||
])
|
||||
);
|
||||
assert.strictEqual(calls, 1, `same route dialed twice: ${calls} calls, expected 1`);
|
||||
});
|
||||
|
||||
test("refused routes fully excluded drain lastResult 429 with no new error", async () => {
|
||||
const shared = proxy("127.0.0.1", 18092);
|
||||
const { calls, status } = await runWith429Stub(
|
||||
entriesFor([
|
||||
{ fingerprint: "cccccccccccccccccccccccccccccccc", proxy: shared },
|
||||
{ fingerprint: "dddddddddddddddddddddddddddddddd", proxy: shared },
|
||||
])
|
||||
);
|
||||
assert.strictEqual(status, 429);
|
||||
assert.ok(calls <= 2, `calls beyond budget: ${calls}`);
|
||||
});
|
||||
|
||||
test("refused route skip keeps distinct routes: two distinct routes produce two calls", async () => {
|
||||
const { calls, status } = await runWith429Stub(
|
||||
entriesFor([
|
||||
{ fingerprint: "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", proxy: proxy("127.0.0.1", 18093) },
|
||||
{ fingerprint: "ffffffffffffffffffffffffffffffff", proxy: proxy("127.0.0.1", 18094) },
|
||||
])
|
||||
);
|
||||
assert.strictEqual(calls, 2, `over-exclusion: ${calls} calls, expected 2`);
|
||||
assert.strictEqual(status, 429);
|
||||
});
|
||||
Reference in New Issue
Block a user