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
10 changes: 7 additions & 3 deletions src/review/enrichment-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -488,10 +488,14 @@ export async function buildReviewEnrichment(
if (!response.ok) {
const bodyPreview = await response.text().catch(() => "");
// 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(
// Surface it at ERROR level (same event as the catch below) so the Sentry forwarder catches a broken REES --
// EXCEPT 413 (LOOPOVER-2J): a large diff/context exceeding REES's own request-size cap is an expected,
// already-gracefully-handled degradation (the review proceeds with no enrichment brief, same as any other
// http_error outcome below), not a broken REES instance -- WARN keeps it visible without paging on it.
const level = response.status === 413 ? "warn" : "error";
console[level](
JSON.stringify({
level: "error",
level,
event: "review_context_fetch_failed",
contextType: "enrichment",
ev: "enrichment_http_error",
Expand Down
17 changes: 17 additions & 0 deletions test/unit/enrichment-wire.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,23 @@ describe("buildReviewEnrichment metrics recording (#5367)", () => {
errSpy.mockRestore();
});

it("REGRESSION (LOOPOVER-2J): a 413 (payload too large) logs at warn, not error -- an expected, already-gracefully-degraded outcome, not a broken REES instance", async () => {
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
globalThis.fetch = vi.fn(
async () => ({ ok: false, status: 413, statusText: "Payload Too Large", text: async () => '{"error":"request_too_large"}' }) as Response,
) as unknown as typeof fetch;
await buildReviewEnrichment(env({ REES_URL: "https://r" }), input);
expect(errSpy).not.toHaveBeenCalled();
expect(warnSpy).toHaveBeenCalledTimes(1);
const logged = JSON.parse(warnSpy.mock.calls[0]![0] as string);
expect(logged).toMatchObject({ level: "warn", event: "review_context_fetch_failed", ev: "enrichment_http_error", status: 413 });
const metrics = await renderMetrics();
expect(metrics).toContain('loopover_rees_enrich_requests_total{status="http_error"} 1'); // outcome recording is unaffected by the log-level change
errSpy.mockRestore();
warnSpy.mockRestore();
});

it('records status="timeout" when the fetch rejects with a TimeoutError (AbortSignal.timeout)', async () => {
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
globalThis.fetch = vi.fn(async () => {
Expand Down