mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-12 02:02:13 +03:00
fix(resilience): set rateLimitedUntil on connection test failure for recovery (#9623)
This commit is contained in:
1
changelog.d/fixes/9623-connection-test-recovery.md
Normal file
1
changelog.d/fixes/9623-connection-test-recovery.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(resilience): failed connection test now sets a short cooldown so connections recover after transient outages (#9623)
|
||||
@@ -701,6 +701,17 @@ export async function testSingleConnection(connectionId: string, validationModel
|
||||
? makeDiagnosis("ok", "local", null, null)
|
||||
: classifyFailure({ error: result.error, statusCode: result.statusCode, provider }));
|
||||
|
||||
// #9623: a failed connection test must not paint the connection permanently red.
|
||||
// Previously a non-terminal failure wrote `testStatus: "error"` with
|
||||
// `rateLimitedUntil: null` — since the cooldown filter only ever skips entries
|
||||
// whose rateLimitedUntil is in the future, a null cooldown left the connection
|
||||
// permanently unavailable after a transient outage. Give non-terminal test
|
||||
// failures a short cooldown so the lazy-recovery path retries them.
|
||||
const terminalTestStatuses = new Set(["banned", "expired", "credits_exhausted"]);
|
||||
const isTerminalFailure =
|
||||
!result.valid && terminalTestStatuses.has(String(diagnosis.code ?? diagnosis.type ?? "").toLowerCase());
|
||||
const testFailureCooldownMs = result.valid ? 0 : 30_000; // 30s retry window
|
||||
|
||||
const updateData: Record<string, any> = {
|
||||
testStatus: result.valid ? "active" : "error",
|
||||
lastError: result.valid ? null : result.error,
|
||||
@@ -709,7 +720,12 @@ export async function testSingleConnection(connectionId: string, validationModel
|
||||
lastErrorType: result.valid ? null : diagnosis.type,
|
||||
lastErrorSource: result.valid ? null : diagnosis.source,
|
||||
errorCode: result.valid ? null : diagnosis.code || result.statusCode || null,
|
||||
rateLimitedUntil: result.valid ? null : connection.rateLimitedUntil || null,
|
||||
rateLimitedUntil:
|
||||
result.valid || isTerminalFailure
|
||||
? result.valid
|
||||
? null
|
||||
: connection.rateLimitedUntil || null
|
||||
: new Date(Date.now() + testFailureCooldownMs).toISOString(),
|
||||
};
|
||||
|
||||
if (result.valid) {
|
||||
|
||||
52
tests/unit/repro-9623.test.ts
Normal file
52
tests/unit/repro-9623.test.ts
Normal file
@@ -0,0 +1,52 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
// #9623: Failed connection test leaves testStatus=error with no recovery path.
|
||||
// Previously the route wrote testStatus:"error" + rateLimitedUntil:null, which the
|
||||
// lazy-recovery cooldown filter never matches (it only skips FUTURE rateLimitedUntil),
|
||||
// leaving the connection permanently unavailable after a transient outage.
|
||||
// Fix: non-terminal test failures now get a short future cooldown (30s) so they recover.
|
||||
|
||||
test("#9623 fix: non-terminal test failure sets a future rateLimitedUntil", () => {
|
||||
// Simulate the fixed updateData logic
|
||||
const now = Date.now();
|
||||
const terminalTestStatuses = new Set(["banned", "expired", "credits_exhausted"]);
|
||||
const valid = false;
|
||||
const diagnosis = { code: "network_error", type: "upstream" }; // non-terminal
|
||||
const isTerminalFailure = terminalTestStatuses.has(String(diagnosis.code).toLowerCase());
|
||||
const testFailureCooldownMs = 30_000;
|
||||
|
||||
const rateLimitedUntil =
|
||||
valid || isTerminalFailure
|
||||
? valid
|
||||
? null
|
||||
: null
|
||||
: new Date(now + testFailureCooldownMs).toISOString();
|
||||
|
||||
assert.ok(
|
||||
rateLimitedUntil !== null,
|
||||
"non-terminal failure should set a future rateLimitedUntil"
|
||||
);
|
||||
const cooldownTime = new Date(rateLimitedUntil as string).getTime();
|
||||
assert.ok(
|
||||
cooldownTime > now,
|
||||
"rateLimitedUntil must be in the future so the lazy-recovery path retries"
|
||||
);
|
||||
assert.ok(
|
||||
cooldownTime <= now + 30_000,
|
||||
"cooldown should be bounded (30s)"
|
||||
);
|
||||
});
|
||||
|
||||
test("#9623 guard: terminal failures stay terminal (no fake recovery)", () => {
|
||||
const terminalTestStatuses = new Set(["banned", "expired", "credits_exhausted"]);
|
||||
const diagnosis = { code: "banned", type: "terminal" };
|
||||
const isTerminalFailure = terminalTestStatuses.has(String(diagnosis.code).toLowerCase());
|
||||
assert.equal(isTerminalFailure, true, "banned must be terminal");
|
||||
});
|
||||
|
||||
test("#9623: success resets cooldown to null", () => {
|
||||
const valid = true;
|
||||
const rateLimitedUntil = valid ? null : new Date(Date.now() + 30_000).toISOString();
|
||||
assert.equal(rateLimitedUntil, null, "successful test clears cooldown");
|
||||
});
|
||||
Reference in New Issue
Block a user