From b2b7c5f95a209c2011816cbc45f8419d5fb1d572 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 27 Jun 2026 20:11:55 -0700 Subject: [PATCH] fix(review): surface a non-2xx REES enrichment response to Sentry buildReviewEnrichment returned undefined on a non-200 from /v1/enrich with no log, so a broken or auth-failing REES backend silently degraded the review to no-enrichment with no signal. Log it at error level (the same review_context_fetch_failed event as the network-error catch) so the Sentry forwarder captures a broken REES backend. --- src/review/enrichment-wire.ts | 15 ++++++++++++++- test/unit/enrichment-wire.test.ts | 15 +++++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/review/enrichment-wire.ts b/src/review/enrichment-wire.ts index ab5a66f668..4ec00e5624 100644 --- a/src/review/enrichment-wire.ts +++ b/src/review/enrichment-wire.ts @@ -92,7 +92,20 @@ export async function buildReviewEnrichment( }), signal: AbortSignal.timeout(timeoutMs), }); - if (!response.ok) return undefined; + if (!response.ok) { + // A non-2xx from REES (auth/5xx/bad-gateway) silently degraded the review to no-enrichment with no signal. + // Surface it at ERROR level (same event as the catch below) so the Sentry forwarder catches a broken REES. + console.error( + JSON.stringify({ + level: "error", + event: "review_context_fetch_failed", + repository: input.repoFullName, + contextType: "enrichment", + message: `REES /v1/enrich returned ${response.status}`, + }), + ); + return undefined; + } const brief = (await response.json()) as { promptSection?: string; systemSuffix?: string; diff --git a/test/unit/enrichment-wire.test.ts b/test/unit/enrichment-wire.test.ts index 361bf1170b..65e7680428 100644 --- a/test/unit/enrichment-wire.test.ts +++ b/test/unit/enrichment-wire.test.ts @@ -90,13 +90,24 @@ describe("buildReviewEnrichment", () => { expect(await buildReviewEnrichment(env({}), input)).toBeUndefined(); }); - it("undefined on a non-200 response", async () => { + it("undefined on a non-200 response, and surfaces it at ERROR for Sentry (was a silent skip)", async () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); globalThis.fetch = vi.fn( - async () => ({ ok: false, json: async () => ({}) }) as Response, + async () => + ({ ok: false, status: 502, json: async () => ({}) }) as Response, ) as unknown as typeof fetch; expect( await buildReviewEnrichment(env({ REES_URL: "https://r" }), input), ).toBeUndefined(); + // A non-2xx REES response now logs at error level (was a silent skip) so a broken backend is visible in Sentry. + expect( + errSpy.mock.calls.some( + (c) => + String(c[0]).includes("review_context_fetch_failed") && + String(c[0]).includes("502"), + ), + ).toBe(true); + errSpy.mockRestore(); }); it("undefined on a fetch error (network/timeout) and surfaces it at ERROR for Sentry (#5)", async () => {