Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions src/orb/outcomes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,13 @@ export interface OrbGlobalStats {
export async function getOrbGlobalStats(env: Env, opts: { excludeAccount?: string } = {}): Promise<OrbGlobalStats> {
// 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
Expand All @@ -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 };
Expand Down
18 changes: 18 additions & 0 deletions test/integration/orb-outcomes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});
});