Skip to content

Commit 6d3f385

Browse files
committed
fix(review): downgrade REES 413 response to warn, not error
A large diff/context exceeding REES's own request-size cap is an expected, already-handled degradation (review proceeds with no enrichment brief, same as any other http_error outcome) -- not a broken REES instance. Logging it at error paged/alerted on routine oversized payloads instead of genuine REES failures.
1 parent b9aa25f commit 6d3f385

2 files changed

Lines changed: 24 additions & 3 deletions

File tree

src/review/enrichment-wire.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -488,10 +488,14 @@ export async function buildReviewEnrichment(
488488
if (!response.ok) {
489489
const bodyPreview = await response.text().catch(() => "");
490490
// A non-2xx from REES (auth/5xx/bad-gateway) silently degraded the review to no-enrichment with no signal.
491-
// Surface it at ERROR level (same event as the catch below) so the Sentry forwarder catches a broken REES.
492-
console.error(
491+
// Surface it at ERROR level (same event as the catch below) so the Sentry forwarder catches a broken REES --
492+
// EXCEPT 413 (LOOPOVER-2J): a large diff/context exceeding REES's own request-size cap is an expected,
493+
// already-gracefully-handled degradation (the review proceeds with no enrichment brief, same as any other
494+
// http_error outcome below), not a broken REES instance -- WARN keeps it visible without paging on it.
495+
const level = response.status === 413 ? "warn" : "error";
496+
console[level](
493497
JSON.stringify({
494-
level: "error",
498+
level,
495499
event: "review_context_fetch_failed",
496500
contextType: "enrichment",
497501
ev: "enrichment_http_error",

test/unit/enrichment-wire.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,23 @@ describe("buildReviewEnrichment metrics recording (#5367)", () => {
709709
errSpy.mockRestore();
710710
});
711711

712+
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 () => {
713+
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
714+
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
715+
globalThis.fetch = vi.fn(
716+
async () => ({ ok: false, status: 413, statusText: "Payload Too Large", text: async () => '{"error":"request_too_large"}' }) as Response,
717+
) as unknown as typeof fetch;
718+
await buildReviewEnrichment(env({ REES_URL: "https://r" }), input);
719+
expect(errSpy).not.toHaveBeenCalled();
720+
expect(warnSpy).toHaveBeenCalledTimes(1);
721+
const logged = JSON.parse(warnSpy.mock.calls[0]![0] as string);
722+
expect(logged).toMatchObject({ level: "warn", event: "review_context_fetch_failed", ev: "enrichment_http_error", status: 413 });
723+
const metrics = await renderMetrics();
724+
expect(metrics).toContain('loopover_rees_enrich_requests_total{status="http_error"} 1'); // outcome recording is unaffected by the log-level change
725+
errSpy.mockRestore();
726+
warnSpy.mockRestore();
727+
});
728+
712729
it('records status="timeout" when the fetch rejects with a TimeoutError (AbortSignal.timeout)', async () => {
713730
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
714731
globalThis.fetch = vi.fn(async () => {

0 commit comments

Comments
 (0)