fix(api): flag provider topology error state by current status, not stale history (#3619) (#3666)

Closes #3619
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-11 12:55:02 -03:00
committed by GitHub
parent 8c7f37340c
commit 47155fa3a0
3 changed files with 44 additions and 1 deletions

View File

@@ -34,6 +34,7 @@
### 🔧 Bug Fixes
- **Provider Topology**: stop flagging healthy providers as errored based on stale historical failures; use current request status (#3619)
- **OpenCode Free**: fetch the live model catalog from the provider's `modelsUrl` for the no-auth model picker instead of serving a stale hardcoded list (#3611)
- **Hermes Agent**: honour the `HERMES_HOME` env var when writing/reading the agent config instead of always using `~/.hermes` (#3628). Introduced `getHermesHome()` / `getHermesConfigPath()` helpers (read at call-time) and routed all four hardcoded callsites through them so OmniRoute's config lands in the same directory that the Hermes PowerShell installer configures on Windows.
- **MITM/cert**: remove the duplicated "Command failed:" prefix in system-command error messages ([#3641](https://github.com/diegosouzapw/OmniRoute/issues/3641)): `execFileText` was prepending its own `"Command failed: "` prefix on top of Node's `execFile` error message, which already begins with `"Command failed: <cmd>"` for non-zero exits. The error message now surfaces Node's message directly (no double prefix), with stderr appended only when non-empty.

View File

@@ -71,7 +71,12 @@ export async function GET() {
lastProviderTs = requestTs;
}
const errorTs = lastErrorAt ? Date.parse(lastErrorAt) : 0;
// Only flag as errorProvider if the provider's MOST RECENT request was itself
// a failure. A provider with a historical lastErrorAt but a recent success
// (lastStatus 2xx/3xx) must not be shown as currently errored (#3619).
const isCurrentlyInError =
lastStatus !== null && (lastStatus < 200 || lastStatus >= 400);
const errorTs = isCurrentlyInError && lastErrorAt ? Date.parse(lastErrorAt) : 0;
if (Number.isFinite(errorTs) && errorTs > errorProviderTs) {
errorProvider = provider;
errorProviderTs = errorTs;

View File

@@ -85,6 +85,43 @@ test("GET /api/provider-metrics includes provider recency and error topology", a
assert.equal(body.topology.errorProvider, "openai");
});
test("GET /api/provider-metrics errorProvider must NOT flag a provider whose most recent request succeeded", async () => {
// Arrange: providerA had an error long ago but recovered (last request = 200).
// providerB never errored.
// Bug (pre-fix): errorProvider = "providerA" because lastErrorAt > 0.
// Fix (post-fix): errorProvider = "" because lastStatus for providerA is 200.
const db = core.getDbInstance();
db.prepare(
`INSERT INTO call_logs (id, timestamp, provider, status, duration, error_summary)
VALUES (?, ?, ?, ?, ?, ?)`
).run("a-old-error", "2026-01-01T00:00:00.000Z", "providerA", 500, 100, "old error");
db.prepare(
`INSERT INTO call_logs (id, timestamp, provider, status, duration, error_summary)
VALUES (?, ?, ?, ?, ?, ?)`
).run("a-recent-ok", "2026-06-01T00:00:00.000Z", "providerA", 200, 80, null);
db.prepare(
`INSERT INTO call_logs (id, timestamp, provider, status, duration, error_summary)
VALUES (?, ?, ?, ?, ?, ?)`
).run("b-ok", "2026-05-01T00:00:00.000Z", "providerB", 200, 60, null);
const response = await providerMetricsRoute.GET();
const body = (await response.json()) as ProviderMetricsResponse;
assert.equal(response.status, 200);
// providerA's last request succeeded — must NOT appear as errorProvider
assert.notEqual(
body.topology.errorProvider,
"providerA",
"providerA recovered (lastStatus=200) and must not be marked as errorProvider"
);
// No provider is currently in error — errorProvider should be empty
assert.equal(
body.topology.errorProvider,
"",
"errorProvider must be empty when no provider's most recent request was a failure"
);
});
test("GET /api/provider-metrics returns sanitized 500 when metrics cannot be loaded", async () => {
const db = core.getDbInstance();
db.close();