diff --git a/src/review/enrichment-wire.ts b/src/review/enrichment-wire.ts index 8210596cb7..ab5a66f668 100644 --- a/src/review/enrichment-wire.ts +++ b/src/review/enrichment-wire.ts @@ -108,7 +108,11 @@ export async function buildReviewEnrichment( ? ENRICHMENT_SYSTEM_SUFFIX : "", }; - } catch { + } catch (error) { + // Surface the failure (#5 review observability): the REES enrichment call can fail (timeout / network / parse) + // and the review then silently proceeds without the brief. ERROR level so the central Sentry forwarder captures + // a broken/slow REES backend instead of it degrading invisibly. + console.error(JSON.stringify({ level: "error", event: "review_context_fetch_failed", repository: input.repoFullName, contextType: "enrichment", message: String(error).slice(0, 200) })); return undefined; // timeout / network / parse ⇒ fail-safe; review proceeds without the brief } } diff --git a/src/review/inline-comments.ts b/src/review/inline-comments.ts index 7c1579db52..7ddd0d6a8b 100644 --- a/src/review/inline-comments.ts +++ b/src/review/inline-comments.ts @@ -118,7 +118,9 @@ export async function postInlineReviewComments( await createPullRequestReviewComments(env, args.installationId, args.repoFullName, args.pullNumber, args.commitId, comments, args.mode); return { posted: comments.length }; } catch (error) { - console.warn(JSON.stringify({ level: "warn", event: "inline_comments_post_failed", repository: args.repoFullName, pullNumber: args.pullNumber, count: comments.length, error: errorMessage(error) })); + // ERROR level (#5 review observability) so the central Sentry forwarder captures a failing inline-comment post + // (auth/permission/422) — it degrades silently (gate unaffected) and was otherwise invisible at warn. + console.error(JSON.stringify({ level: "error", event: "inline_comments_post_failed", repository: args.repoFullName, pullNumber: args.pullNumber, count: comments.length, error: errorMessage(error) })); return { posted: 0 }; } } diff --git a/src/review/rag.ts b/src/review/rag.ts index 26420af97f..afd34b3130 100644 --- a/src/review/rag.ts +++ b/src/review/rag.ts @@ -407,7 +407,10 @@ export async function retrieveContext( ); return out; } catch (error) { - console.log(JSON.stringify({ ev: "rag_retrieve_error", message: String(error).slice(0, 200) })); + // ERROR level (#5 review observability): emit so the central Sentry forwarder captures a broken RAG backend + // (qdrant/embedder down) — retrieval degrades the review to diff-only, and this was previously a no-`level` + // console.log invisible to Sentry. Keeps the `ev` tag for log continuity. + console.error(JSON.stringify({ level: "error", event: "review_context_fetch_failed", contextType: "rag", ev: "rag_retrieve_error", message: String(error).slice(0, 200) })); return ""; } } diff --git a/test/unit/enrichment-wire.test.ts b/test/unit/enrichment-wire.test.ts index f9583ea08a..361bf1170b 100644 --- a/test/unit/enrichment-wire.test.ts +++ b/test/unit/enrichment-wire.test.ts @@ -99,6 +99,17 @@ describe("buildReviewEnrichment", () => { ).toBeUndefined(); }); + it("undefined on a fetch error (network/timeout) and surfaces it at ERROR for Sentry (#5)", async () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + globalThis.fetch = vi.fn(async () => { + throw new Error("network down"); + }) as unknown as typeof fetch; + expect(await buildReviewEnrichment(env({ REES_URL: "https://r" }), input)).toBeUndefined(); + // A broken/slow REES backend now surfaces at level:error (central Sentry forwarder) instead of degrading silently. + expect(errSpy.mock.calls.some((c) => String(c[0]).includes("review_context_fetch_failed") && String(c[0]).includes('"contextType":"enrichment"'))).toBe(true); + errSpy.mockRestore(); + }); + it("undefined on an empty promptSection (no findings)", async () => { globalThis.fetch = vi.fn( async () => diff --git a/test/unit/inline-comments.test.ts b/test/unit/inline-comments.test.ts index 37228b1767..45d3ef4e93 100644 --- a/test/unit/inline-comments.test.ts +++ b/test/unit/inline-comments.test.ts @@ -118,13 +118,17 @@ describe("postInlineReviewComments (#inline-comments, fail-safe)", () => { expect(calls[0]?.body).toMatchObject({ event: "COMMENT", commit_id: "headsha", comments: [{ path: "src/a.ts", line: 2, side: "RIGHT", body: "**Nit:** guard this" }] }); }); - it("swallows an API error (the gate is never affected) and reports 0 posted", async () => { + it("swallows an API error (the gate is never affected), reports 0 posted, and surfaces it at ERROR for Sentry (#5)", async () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { const url = input.toString(); if (url.includes("/access_tokens")) return Response.json({ token: "t" }); return new Response("boom", { status: 500 }); // /reviews → non-2xx → octokit throws → caught }); expect(await postInlineReviewComments(envWithKey(), { ...base, commitId: "headsha", findings })).toEqual({ posted: 0 }); + // The failure is now emitted at level:error so the central Sentry forwarder captures it (was an invisible warn). + expect(errSpy.mock.calls.some((c) => String(c[0]).includes("inline_comments_post_failed") && String(c[0]).includes('"level":"error"'))).toBe(true); + errSpy.mockRestore(); }); }); diff --git a/test/unit/rag-wiring.test.ts b/test/unit/rag-wiring.test.ts index 2bcd09a449..5968a2068a 100644 --- a/test/unit/rag-wiring.test.ts +++ b/test/unit/rag-wiring.test.ts @@ -156,10 +156,14 @@ describe("buildReviewRagContext: retrieval wiring + fail-safe", () => { expect(vec.query).not.toHaveBeenCalled(); }); - it("fail-safe: a THROWING vector query degrades to empty context (never throws)", async () => { + it("fail-safe: a THROWING vector query degrades to empty context (never throws) + surfaces it at ERROR for Sentry (#5)", async () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); const vec = { ...vectorizeStub(), query: vi.fn(async () => { throw new Error("vectorize down"); }) }; const env = createTestEnv({ DB: ragDbStub(), VECTORIZE: vec as unknown as Vectorize, AI: aiStub() as unknown as Ai }); await expect(buildReviewRagContext(env, { repoFullName: "acme/rag-throw-1", files: changedFiles })).resolves.toBe(""); + // A broken RAG backend now surfaces at level:error (central Sentry forwarder) instead of degrading invisibly. + expect(errSpy.mock.calls.some((c) => String(c[0]).includes("review_context_fetch_failed") && String(c[0]).includes('"contextType":"rag"'))).toBe(true); + errSpy.mockRestore(); }); it("fail-safe: no changed files → empty context, no adapter use", async () => {