From 8725175ccf435283f061dd7b6b058e17be9c7df7 Mon Sep 17 00:00:00 2001 From: Jeff <158072326+jeffrey701@users.noreply.github.com> Date: Sun, 26 Jul 2026 15:48:23 +0200 Subject: [PATCH] fix(orb): degrade getOrbGlobalStats to zeros on a DB error --- src/orb/outcomes.ts | 18 +++++++++++++----- test/integration/orb-outcomes.test.ts | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/orb/outcomes.ts b/src/orb/outcomes.ts index f91f569b45..befbec7119 100644 --- a/src/orb/outcomes.ts +++ b/src/orb/outcomes.ts @@ -63,8 +63,13 @@ export interface OrbGlobalStats { export async function getOrbGlobalStats(env: Env, opts: { excludeAccount?: string } = {}): Promise { // excludeAccount de-dups an account already counted by another source. "" = include all. const exclude = (opts.excludeAccount ?? "").toLowerCase(); - const row = await env.DB.prepare( - `SELECT + let row: { merged: number | null; closed: number | null; total: number | null } | null; + // #8879: guard ONLY the query. A D1 error degrades to zeros, mirroring computeFleetAnalytics's try/catch + // (src/orb/analytics.ts) so a failure on this join drops just the orb aggregate instead of 503-ing the entire + // /v1/public/stats payload (accuracyTrend/reuseRateTrend/reviewVolumeTrend/rulePrecision) via the route catch. + try { + row = await env.DB.prepare( + `SELECT SUM(CASE WHEN o.outcome = 'merged' THEN 1 ELSE 0 END) AS merged, SUM(CASE WHEN o.outcome = 'closed' THEN 1 ELSE 0 END) AS closed, COUNT(*) AS total @@ -75,9 +80,12 @@ export async function getOrbGlobalStats(env: Env, opts: { excludeAccount?: strin AND ae.event_type = 'github_app.pr_public_surface_published' WHERE (? = '' OR LOWER(COALESCE(i.account_login, '')) <> ?) AND ae.id IS NULL`, - ) - .bind(exclude, exclude) - .first<{ merged: number | null; closed: number | null; total: number | null }>(); + ) + .bind(exclude, exclude) + .first<{ merged: number | null; closed: number | null; total: number | null }>(); + } catch { + return { merged: 0, closed: 0, total: 0 }; + } /* v8 ignore next -- an aggregate query always returns exactly one row; this guards the nullable .first() type only */ if (!row) return { merged: 0, closed: 0, total: 0 }; return { merged: row.merged ?? 0, closed: row.closed ?? 0, total: row.total ?? 0 }; diff --git a/test/integration/orb-outcomes.test.ts b/test/integration/orb-outcomes.test.ts index 33b3a15fc4..1eaf2b6751 100644 --- a/test/integration/orb-outcomes.test.ts +++ b/test/integration/orb-outcomes.test.ts @@ -111,4 +111,22 @@ describe("getOrbGlobalStats", () => { await recordOrbPrOutcome(e, "pull_request", closedPr("acme/new", 2, null, 100)); // closed, no own-ledger counterpart → must still count expect(await getOrbGlobalStats(e)).toEqual({ merged: 0, closed: 1, total: 1 }); }); + + // #8879: a D1 error on the join must degrade to zeros (like computeFleetAnalytics), not throw out of the + // Promise.all in public-stats.ts and 503 the whole /v1/public/stats payload. + it("degrades to zeros on a DB error instead of throwing (#8879)", async () => { + const brokenDb = { + prepare: () => ({ bind: () => ({ first: () => Promise.reject(new Error("D1 exceeded its CPU time limit and was reset")) }) }), + } as unknown as Env["DB"]; + await expect(getOrbGlobalStats({ DB: brokenDb } as unknown as Env)).resolves.toEqual({ merged: 0, closed: 0, total: 0 }); + }); + + // Defensive .first() null guard: a driver anomaly returning no row degrades to zeros rather than throwing on + // a null field access (covers the `if (!row)` branch the try/catch reindent brings into the diff). + it("returns zeros when the query resolves without a row (#8879)", async () => { + const nullRowDb = { + prepare: () => ({ bind: () => ({ first: () => Promise.resolve(null) }) }), + } as unknown as Env["DB"]; + await expect(getOrbGlobalStats({ DB: nullRowDb } as unknown as Env)).resolves.toEqual({ merged: 0, closed: 0, total: 0 }); + }); });