mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-19 13:23:50 +03:00
resolveProxyForConnection cached a scope pool's first resolution result for the life of the per-connection cache, so a chat-path request never saw the pool's round-robin/sticky/random strategy advance again — only the narrow #13578 set-aside escape hatch could break the freeze. resolveProxyForScopeFromRegistry (used directly by every existing rotation test) always re-ran the strategy and rotated correctly. The cache now treats a registry-sourced pool result as due for re-resolution on every call (falling through to the same cascade the direct registry callers use), except for the two populations that need a stable egress across requests: EGRESS_BUCKETED_LOCK_PROVIDERS (opencode's quota is bucketed by egress IP) and grok-web (its cf_clearance cookie is pinned to the IP/UA/TLS fingerprint that earned it). Regression test: tests/unit/proxy-pool-chat-path-rotation-13575.test.ts, RED before the fix (resolveProxyForConnection returned the same host 6/6 times for a 3-member pool), GREEN after. Updated tests/unit/proxy-pool-skips-refused-member.test.ts's three assertions that encoded the frozen-cache contract to the corrected always-rotates-except-pinned contract; all other cases in that file and in tests/unit/proxy-pool-rotation-6365.test.ts pass unchanged.
This commit is contained in:
committed by
GitHub
parent
b0955042dc
commit
f3ab24b8c7
1
changelog.d/fixes/13575-proxy-pool-chat-path-rotation.md
Normal file
1
changelog.d/fixes/13575-proxy-pool-chat-path-rotation.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(db): make the chat-path proxy resolver (`resolveProxyForConnection`) rotate a multi-member pool the same way the registry resolver already does — its per-connection cache was freezing on the first pool member forever instead of re-running the scope's round-robin/sticky/random strategy on each request, unless the connection needs a stable egress (opencode's egress-bucketed quota, grok-web's IP-pinned `cf_clearance`) (#13575)
|
||||
@@ -15,6 +15,7 @@ import { isProxySkipRecentlyFailedEnabled } from "@/shared/utils/featureFlags";
|
||||
import { invalidateDbCache } from "./readCache";
|
||||
import { encrypt, decrypt } from "./encryption";
|
||||
import { getProxyRegistryGeneration, resolveProxyForScopeFromRegistry } from "./proxies";
|
||||
import { isEgressBucketedLockScope } from "@omniroute/open-sse/config/providerErrorRules.ts";
|
||||
import { getComboModelProvider as getComboEntryProvider } from "@/lib/combos/steps";
|
||||
import { requestBodyLimitMbFromEnv } from "@/shared/constants/bodySize";
|
||||
import { DEFAULT_RESPONSES_PREVIOUS_RESPONSE_ID_MODE } from "@/shared/constants/responsesPreviousResponseId";
|
||||
@@ -527,6 +528,41 @@ function isCachedPoolMemberSetAside(entry: ProxyResolutionCacheEntry): boolean {
|
||||
return isProxySkipRecentlyFailedEnabled();
|
||||
}
|
||||
|
||||
// Providers that need a STABLE egress across requests, never rotated under them by this
|
||||
// cache-invalidation path (#13575): opencode's free-tier quota is bucketed by egress IP
|
||||
// (EGRESS_BUCKETED_LOCK_PROVIDERS — rotating would fragment one connection's quota across
|
||||
// several IPs), and grok-web's cf_clearance cookie is pinned to the IP/User-Agent/TLS
|
||||
// fingerprint that earned it (src/shared/providers/webSessionCredentials.ts "grok-web" —
|
||||
// rotating the egress would turn every subsequent request into a Cloudflare 403).
|
||||
function requiresStableEgress(provider: string | null): boolean {
|
||||
if (!provider) return false;
|
||||
return isEgressBucketedLockScope(provider) || provider.toLowerCase() === "grok-web";
|
||||
}
|
||||
|
||||
// The chat-path cache (below) exists so a hot connection does not pay the full resolution
|
||||
// cascade on every request, but it must not FREEZE a rotating pool's choice: the registry
|
||||
// resolver (resolveProxyForScopeFromRegistry, called directly by every #6365 rotation test)
|
||||
// re-runs its strategy on every call and rotates correctly, while the cache here returned
|
||||
// the same first-resolved member forever (#13575). A cached member is stale whenever it came
|
||||
// from a live scope pool (source: "registry") and the connection is not in the two populations
|
||||
// above that need a pinned egress instead: the caller then falls through to the full cascade,
|
||||
// which re-invokes resolveProxyForScopeFromRegistry and applies the pool's own selection
|
||||
// strategy (round-robin advances, sticky holds until its window elapses, random reshuffles) —
|
||||
// no new strategy is introduced here.
|
||||
function isCachedPoolMemberDue(
|
||||
entry: ProxyResolutionCacheEntry,
|
||||
db: ReturnType<typeof getDbInstance>,
|
||||
connectionId: string
|
||||
): boolean {
|
||||
const { result } = entry;
|
||||
if (result.source !== "registry" || result.proxy == null) return false;
|
||||
const row = db
|
||||
.prepare("SELECT provider FROM provider_connections WHERE id = ?")
|
||||
.get(connectionId) as { provider?: string } | undefined;
|
||||
const provider = typeof row?.provider === "string" ? row.provider : null;
|
||||
return !requiresStableEgress(provider);
|
||||
}
|
||||
|
||||
export async function resolveProxyForConnection(
|
||||
connectionId: string,
|
||||
apiKeyId?: string,
|
||||
@@ -542,18 +578,18 @@ export async function resolveProxyForConnection(
|
||||
registryGeneration: getProxyRegistryGeneration(),
|
||||
refusalSeq: getProxyRefusalSeq(),
|
||||
};
|
||||
const db = getDbInstance();
|
||||
const cached = proxyResolutionCache.get(cacheKey);
|
||||
if (
|
||||
cached &&
|
||||
cached.generation === stamp.generation &&
|
||||
cached.registryGeneration === stamp.registryGeneration &&
|
||||
!isCachedPoolMemberSetAside(cached)
|
||||
!isCachedPoolMemberSetAside(cached) &&
|
||||
!isCachedPoolMemberDue(cached, db, connectionId)
|
||||
) {
|
||||
return cached.result;
|
||||
}
|
||||
|
||||
const db = getDbInstance();
|
||||
|
||||
// Step 1: Check global proxyEnabled setting
|
||||
// Read only the proxyEnabled key for performance instead of loading all settings.
|
||||
let globalProxyEnabled = true;
|
||||
|
||||
120
tests/unit/proxy-pool-chat-path-rotation-13575.test.ts
Normal file
120
tests/unit/proxy-pool-chat-path-rotation-13575.test.ts
Normal file
@@ -0,0 +1,120 @@
|
||||
/**
|
||||
* Regression test for issue #13575.
|
||||
*
|
||||
* The chat path (`resolveProxyForConnection`) must rotate across a multi-member
|
||||
* pool the same way the registry-level resolver does (`resolveProxyForScopeFromRegistry`),
|
||||
* instead of freezing on the first pick for the life of the per-connection cache.
|
||||
* Providers with a stable-egress requirement (egress-bucketed quota, IP-pinned web
|
||||
* session credentials) must stay pinned to a single member.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-proxy-13575-rotation-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
process.env.API_KEY_SECRET = "test-secret";
|
||||
|
||||
const core = await import("../../src/lib/db/core.ts");
|
||||
const proxiesDb = await import("../../src/lib/db/proxies.ts");
|
||||
const providersDb = await import("../../src/lib/db/providers.ts");
|
||||
const settingsDb = await import("../../src/lib/db/settings.ts");
|
||||
|
||||
let proxySeq = 0;
|
||||
async function makeProxy() {
|
||||
proxySeq++;
|
||||
const proxy = await proxiesDb.createProxy({
|
||||
name: `Pool proxy ${proxySeq}`,
|
||||
type: "http",
|
||||
host: `10.9.1.${proxySeq}`,
|
||||
port: 9100 + proxySeq,
|
||||
status: "active",
|
||||
});
|
||||
return proxy!;
|
||||
}
|
||||
|
||||
async function makeConnection(provider = "openai"): Promise<string> {
|
||||
const conn = await providersDb.createProviderConnection({
|
||||
provider,
|
||||
authType: "apiKey",
|
||||
name: `Conn ${Date.now()} ${Math.random()}`,
|
||||
apiKey: "sk-test",
|
||||
});
|
||||
return (conn as { id: string }).id;
|
||||
}
|
||||
|
||||
test.after(async () => {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
|
||||
});
|
||||
|
||||
test("#13575: resolveProxyForConnection rotates round-robin across a multi-member pool", async () => {
|
||||
const a = await makeProxy();
|
||||
const b = await makeProxy();
|
||||
const c = await makeProxy();
|
||||
const connId = await makeConnection("openai");
|
||||
await proxiesDb.addProxyToScopePool("account", connId, a.id);
|
||||
await proxiesDb.addProxyToScopePool("account", connId, b.id);
|
||||
await proxiesDb.addProxyToScopePool("account", connId, c.id);
|
||||
|
||||
const chatPathHosts: string[] = [];
|
||||
for (let i = 0; i < 6; i++) {
|
||||
const r = (await settingsDb.resolveProxyForConnection(connId)) as {
|
||||
proxy?: { host?: string } | null;
|
||||
};
|
||||
chatPathHosts.push(r.proxy?.host ?? "null");
|
||||
}
|
||||
|
||||
const distinctChatHosts = new Set(chatPathHosts);
|
||||
assert.ok(
|
||||
distinctChatHosts.size > 1,
|
||||
`expected resolveProxyForConnection to rotate across the pool, but got: ${JSON.stringify(chatPathHosts)}`
|
||||
);
|
||||
assert.deepEqual(distinctChatHosts, new Set([a.host, b.host, c.host]));
|
||||
});
|
||||
|
||||
test("#13575: egress-bucketed-quota providers (opencode) stay pinned on the chat path", async () => {
|
||||
const a = await makeProxy();
|
||||
const b = await makeProxy();
|
||||
const connId = await makeConnection("opencode");
|
||||
await proxiesDb.addProxyToScopePool("account", connId, a.id);
|
||||
await proxiesDb.addProxyToScopePool("account", connId, b.id);
|
||||
|
||||
const hosts: string[] = [];
|
||||
for (let i = 0; i < 6; i++) {
|
||||
const r = (await settingsDb.resolveProxyForConnection(connId)) as {
|
||||
proxy?: { host?: string } | null;
|
||||
};
|
||||
hosts.push(r.proxy?.host ?? "null");
|
||||
}
|
||||
|
||||
assert.deepEqual(
|
||||
new Set(hosts),
|
||||
new Set([hosts[0]]),
|
||||
`expected an egress-bucketed-quota connection to stay pinned to one member, got: ${JSON.stringify(hosts)}`
|
||||
);
|
||||
});
|
||||
|
||||
test("#13575: IP-pinned web-session providers (grok-web) stay pinned on the chat path", async () => {
|
||||
const a = await makeProxy();
|
||||
const b = await makeProxy();
|
||||
const connId = await makeConnection("grok-web");
|
||||
await proxiesDb.addProxyToScopePool("account", connId, a.id);
|
||||
await proxiesDb.addProxyToScopePool("account", connId, b.id);
|
||||
|
||||
const hosts: string[] = [];
|
||||
for (let i = 0; i < 6; i++) {
|
||||
const r = (await settingsDb.resolveProxyForConnection(connId)) as {
|
||||
proxy?: { host?: string } | null;
|
||||
};
|
||||
hosts.push(r.proxy?.host ?? "null");
|
||||
}
|
||||
|
||||
assert.deepEqual(
|
||||
new Set(hosts),
|
||||
new Set([hosts[0]]),
|
||||
`expected an IP-pinned web-session connection to stay pinned to one member, got: ${JSON.stringify(hosts)}`
|
||||
);
|
||||
});
|
||||
@@ -5,9 +5,10 @@ import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
// With PROXY_SKIP_RECENTLY_FAILED on, pool selection skips members that just failed, for
|
||||
// every rotation strategy, and the per-connection resolution cache stops re-serving such a
|
||||
// member (once per set-aside event, never a DB cascade per request). With every member set
|
||||
// aside, or the flag off (the default), selection is exactly what it was.
|
||||
// every rotation strategy. The per-connection resolution used by the chat path
|
||||
// (resolveProxyForConnection) re-runs the registry cascade on every call for a multi-member
|
||||
// pool (#13575) and skips a set-aside member on top of that. With every member set aside, or
|
||||
// the flag off (the default), set-aside skipping does not apply but rotation continues.
|
||||
|
||||
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-pool-skip-refused-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
@@ -167,34 +168,39 @@ test("a DB override turning the flag off wins over the environment", async () =>
|
||||
assert.deepEqual(await picks(2), [members[0].host, members[2].host]);
|
||||
});
|
||||
|
||||
test("a connection's cached pool member is not re-served once set aside", async () => {
|
||||
// #13575: the chat-path resolveProxyForConnection() no longer freezes a multi-member
|
||||
// pool's choice for the connection's lifetime — it re-runs the registry cascade on every
|
||||
// call (like every other pool consumer) so round-robin/sticky/random keep working exactly
|
||||
// as resolveProxyForScopeFromRegistry() already does when called directly. A member set
|
||||
// aside is still skipped on top of that rotation, same as before.
|
||||
test("a connection's chat-path resolution skips a member set aside", async () => {
|
||||
const [a, b, c] = await pool(3, "account", "conn-pool");
|
||||
const first = await settingsDb.resolveProxyForConnection("conn-pool");
|
||||
assert.equal((first as { proxy: { host: string } }).proxy.host, a.host);
|
||||
assert.strictEqual(await settingsDb.resolveProxyForConnection("conn-pool"), first);
|
||||
|
||||
setAside(a);
|
||||
setAside(b);
|
||||
const next = await settingsDb.resolveProxyForConnection("conn-pool");
|
||||
assert.equal((next as { proxy: { host: string } }).proxy.host, b.host);
|
||||
assert.equal((next as { proxy: { host: string } }).proxy.host, c.host);
|
||||
|
||||
memory.noteProxyRecovered(keyOf(a), "proxy_unreachable");
|
||||
assert.equal(memory.isProxyAvoided(keyOf(a)), false);
|
||||
memory.noteProxyRecovered(keyOf(b), "proxy_unreachable");
|
||||
assert.equal(memory.isProxyAvoided(keyOf(b)), false);
|
||||
assert.deepEqual(
|
||||
[await pick("account", "conn-pool"), await pick("account", "conn-pool")],
|
||||
[c.host, a.host]
|
||||
[a.host, b.host]
|
||||
);
|
||||
});
|
||||
|
||||
test("with the flag off a connection keeps its cached pool member even once set aside", async () => {
|
||||
const [a] = await pool(3, "account", "conn-off");
|
||||
test("with the flag off a connection's chat-path resolution still rotates normally", async () => {
|
||||
const [a, b] = await pool(3, "account", "conn-off");
|
||||
delete process.env.PROXY_SKIP_RECENTLY_FAILED;
|
||||
const first = await settingsDb.resolveProxyForConnection("conn-off");
|
||||
assert.equal((first as { proxy: { host: string } }).proxy.host, a.host);
|
||||
setAside(a);
|
||||
assert.strictEqual(await settingsDb.resolveProxyForConnection("conn-off"), first);
|
||||
const next = await settingsDb.resolveProxyForConnection("conn-off");
|
||||
assert.equal((next as { proxy: { host: string } }).proxy.host, b.host);
|
||||
});
|
||||
|
||||
test("with every member set aside the cascade re-runs once, not on every request", async () => {
|
||||
test("with every member set aside, chat-path resolution keeps rotating like the registry", async () => {
|
||||
// Round-robin advances its persisted cursor on each cascade run, so the cursor counts
|
||||
// how many times the registry pool was actually queried for this connection.
|
||||
const [a, b] = await pool(2, "account", "conn-all");
|
||||
@@ -218,10 +224,9 @@ test("with every member set aside the cascade re-runs once, not on every request
|
||||
assert.equal((second as { proxy: { host: string } }).proxy.host, b.host);
|
||||
assert.equal(cursor(), 2);
|
||||
|
||||
for (let i = 0; i < 5; i++) {
|
||||
assert.strictEqual(await settingsDb.resolveProxyForConnection("conn-all"), second);
|
||||
}
|
||||
assert.equal(cursor(), 2, "a member set aside before the entry was cached must not bypass it");
|
||||
const third = await settingsDb.resolveProxyForConnection("conn-all");
|
||||
assert.equal((third as { proxy: { host: string } }).proxy.host, a.host);
|
||||
assert.equal(cursor(), 3);
|
||||
});
|
||||
|
||||
test("a legacy single-proxy level stays cached even when its proxy is set aside", async () => {
|
||||
|
||||
Reference in New Issue
Block a user