feat(rees): add error-swallow analyzer for empty catch blocks (#2014)

.env.example

@@ -68,25 +68,25 @@ GITTENSORY_REVIEW_ENRICHMENT=false
 # commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot
 # blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
 # pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
-# conflictMarker,debugLeftover,commitLint
+# conflictMarker,debugLeftover,errorSwallow,commitLint
 #
 # Profile defaults:
 # fast: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency
 # hardcodedUrl,actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild
 # testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker
-# debugLeftover
+# debugLeftover,errorSwallow
 # balanced (default): dependency,dependencyDiff,lockfileDrift,secret,license,installScript
 # heavyDependency,hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight
 # typosquat,commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication
 # churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch
 # commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology
-# todoMarker,magicNumber,conflictMarker,debugLeftover,commitLint
+# todoMarker,magicNumber,conflictMarker,debugLeftover,errorSwallow,commitLint
 # deep: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency
 # hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat
 # commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot
 # blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
 # pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
-# conflictMarker,debugLeftover,commitLint
+# conflictMarker,debugLeftover,errorSwallow,commitLint
 # END GENERATED REES ANALYZERS
 
 # Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep

apps/gittensory-ui/src/lib/rees-analyzers.ts

@@ -956,6 +956,28 @@ export const REES_ANALYZERS = [
         "Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching.",
     },
   },
+  {
+    name: "errorSwallow",
+    title: "Error swallowing",
+    category: "quality",
+    cost: "local",
+    defaultEnabled: true,
+    profiles: ["fast", "balanced", "deep"],
+    requires: ["files"],
+    limits: {
+      maxFindings: 25,
+      maxLineChars: 2000,
+    },
+    docs: {
+      summary:
+        "Flags newly added catch/except blocks that silently discard errors — empty bodies, unused bindings, or a lone return null.",
+      looksAt: "Added lines in changed JS/TS/Python source files (non-test).",
+      reports: "File, line, and kind: empty-catch, unused-binding, or return-null.",
+      network: "Pure local analyzer. No external network call.",
+      notes:
+        "Python `except: pass` is allowed. Catch blocks that log, rethrow, or reference the caught binding are not flagged.",
+    },
+  },
   {
     name: "commitLint",
     title: "Conventional-commit subjects",

review-enrichment/analyzer-metadata.json

@@ -1079,6 +1079,32 @@
         "notes": "Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching."
       }
     },
+    {
+      "name": "errorSwallow",
+      "title": "Error swallowing",
+      "category": "quality",
+      "cost": "local",
+      "defaultEnabled": true,
+      "profiles": [
+        "fast",
+        "balanced",
+        "deep"
+      ],
+      "requires": [
+        "files"
+      ],
+      "limits": {
+        "maxFindings": 25,
+        "maxLineChars": 2000
+      },
+      "docs": {
+        "summary": "Flags newly added catch/except blocks that silently discard errors — empty bodies, unused bindings, or a lone return null.",
+        "looksAt": "Added lines in changed JS/TS/Python source files (non-test).",
+        "reports": "File, line, and kind: empty-catch, unused-binding, or return-null.",
+        "network": "Pure local analyzer. No external network call.",
+        "notes": "Python `except: pass` is allowed. Catch blocks that log, rethrow, or reference the caught binding are not flagged."
+      }
+    },
     {
       "name": "commitLint",
       "title": "Conventional-commit subjects",

review-enrichment/src/analyzers/error-swallow.ts

@@ -0,0 +1,138 @@
+// Error-swallow analyzer (#2014). Flags newly-added catch/except blocks that silently discard errors —
+// empty bodies, unused bindings, or a lone `return null` with no log/rethrow. Pure compute over added diff
+// lines; no network. JS/TS/Python only; Python `except: pass` is intentionally allowed.
+import type { EnrichRequest, ErrorSwallowFinding } from "../types.js";
+import { codeOnly } from "./secret-log.js";
+import { isTestPath } from "./test-ratio.js";
+
+const MAX_FINDINGS = 25;
+const MAX_LINE_CHARS = 2000;
+
+const SOURCE_EXTS = new Set(["ts", "tsx", "mts", "cts", "js", "jsx", "mjs", "cjs", "py"]);
+
+const JS_CATCH_RE = /\bcatch\s*(?:\(\s*([A-Za-z_$][\w$]*)\s*\))?\s*\{([\s\S]*)\}/;
+const PYTHON_EXCEPT_RE = /^\s*except\b(?:\s+([^:\n]+?))?(?:\s+as\s+([A-Za-z_]\w*))?\s*:\s*(.*)$/;
+
+type ScanLimits = {
+  maxFindings?: number;
+  signal?: AbortSignal;
+};
+
+function sourceExtOf(path: string): string | null {
+  const match = /\.([A-Za-z0-9]+)$/.exec(path);
+  return match ? match[1]!.toLowerCase() : null;
+}
+
+export function isErrorSwallowSourcePath(path: string): boolean {
+  const ext = sourceExtOf(path);
+  return Boolean(ext && SOURCE_EXTS.has(ext) && !isTestPath(path));
+}
+
+function bodySwallowsError(body: string, binding: string | null, isPython: boolean): ErrorSwallowFinding["kind"] | null {
+  const inner = codeOnly(body).trim();
+  if (!inner) return "empty-catch";
+  if (isPython && /^pass(?:\s+#.*)?$/.test(inner)) return null;
+
+  if (/^return\s+(?:null|None)\s*;?$/.test(inner) && !/\bthrow\b/.test(inner) && !mentionsLogOrBinding(inner, binding)) {
+    return "return-null";
+  }
+
+  if (/\bthrow\b/.test(inner)) return null;
+  if (mentionsLogOrBinding(inner, binding)) return null;
+
+  if (binding) return "unused-binding";
+  return "empty-catch";
+}
+
+function mentionsLogOrBinding(body: string, binding: string | null): boolean {
+  if (binding && new RegExp(`\\b${binding.replace(/[$]/g, "\\$")}\\b`).test(body)) return true;
+  return /\b(console\.|logger\.|log\.|print\s*\(|Sentry\.|captureException\b|reportError\b|\.error\s*\(|\.warn\s*\()/i.test(body);
+}
+
+/** Classify one added JS/TS catch on a single line, or null when clean / out of scope. Pure. */
+export function detectJsCatchSwallow(line: string): ErrorSwallowFinding["kind"] | null {
+  const code = codeOnly(line);
+  const match = JS_CATCH_RE.exec(code);
+  if (!match) return null;
+  return bodySwallowsError(match[2] ?? "", match[1] ?? null, false);
+}
+
+/** Classify one added Python except line (and optional same-line body), or null. Pure. */
+export function detectPythonExceptSwallow(line: string, nextAddedLine?: string | null): ErrorSwallowFinding["kind"] | null {
+  const match = PYTHON_EXCEPT_RE.exec(line);
+  if (!match) return null;
+  const binding = match[2] ?? null;
+  let body = (match[3] ?? "").trim();
+  if (!body && nextAddedLine) body = nextAddedLine.trim();
+  if (/^\s*pass\s*$/.test(body) || body === "pass") return null;
+  return bodySwallowsError(body, binding, true);
+}
+
+/** Scan one file patch's added lines for error-swallowing catch blocks, line-cited via hunk headers. Pure. */
+export function scanPatchForErrorSwallow(
+  path: string,
+  patch: string,
+  limits: ScanLimits = {},
+): ErrorSwallowFinding[] {
+  const maxFindings = limits.maxFindings ?? MAX_FINDINGS;
+  if (maxFindings <= 0 || !isErrorSwallowSourcePath(path)) return [];
+
+  const isPython = /\.pyi?$/i.test(path);
+  const findings: ErrorSwallowFinding[] = [];
+  const lines = patch.split("\n");
+  let newLine = 0;
+  let inHunk = false;
+
+  for (let index = 0; index < lines.length; index += 1) {
+    if (limits.signal?.aborted) throw new Error("analyzer_aborted");
+    const line = lines[index]!;
+    const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/.exec(line);
+    if (hunk) {
+      newLine = Number(hunk[1]);
+      inHunk = true;
+      continue;
+    }
+    if (!inHunk || !line.startsWith("+") || line.startsWith("+++")) continue;
+
+    const body = line.slice(1);
+    if (body.length > MAX_LINE_CHARS) {
+      newLine += 1;
+      continue;
+    }
+
+    let kind: ErrorSwallowFinding["kind"] | null = null;
+    if (isPython) {
+      const nextAdded =
+        lines.slice(index + 1).find((candidate) => candidate.startsWith("+") && !candidate.startsWith("+++"))?.slice(1) ??
+        null;
+      kind = detectPythonExceptSwallow(body, nextAdded);
+    } else {
+      kind = detectJsCatchSwallow(body);
+    }
+
+    if (kind) {
+      findings.push({ file: path, line: newLine, kind });
+      if (findings.length >= maxFindings) return findings;
+    }
+    newLine += 1;
+  }
+
+  return findings;
+}
+
+/** Analyzer entrypoint: scan every changed non-test source file's added lines for error swallowing. */
+export async function scanErrorSwallow(req: EnrichRequest, signal?: AbortSignal): Promise<ErrorSwallowFinding[]> {
+  const findings: ErrorSwallowFinding[] = [];
+  for (const file of req.files ?? []) {
+    if (signal?.aborted) throw new Error("analyzer_aborted");
+    if (!file.patch) continue;
+    for (const finding of scanPatchForErrorSwallow(file.path, file.patch, {
+      maxFindings: MAX_FINDINGS - findings.length,
+      signal,
+    })) {
+      findings.push(finding);
+      if (findings.length >= MAX_FINDINGS) return findings;
+    }
+  }
+  return findings;
+}

review-enrichment/src/analyzers/registry.ts

@@ -32,6 +32,7 @@ import { scanLooseRanges } from "./loose-range.js";
 import { scanMagicNumbers } from "./magic-number.js";
 import { scanConflictMarkers } from "./conflict-marker.js";
 import { scanDebugLeftover } from "./debug-leftover.js";
+import { scanErrorSwallow } from "./error-swallow.js";
 import { scanCommitLint } from "./commit-lint.js";
 import { scanTerminology } from "./terminology.js";
 import { scanTodoMarker } from "./todo-marker.js";
@@ -1013,6 +1014,35 @@ export const ANALYZER_DESCRIPTORS = [
     },
     run: (req, { signal }) => scanDebugLeftover(req, signal),
   }),
+  descriptor({
+    name: "errorSwallow",
+    title: "Error swallowing",
+    category: "quality",
+    cost: "local",
+    defaultEnabled: true,
+    requires: ["files"],
+    limits: { maxFindings: 25, maxLineChars: 2000 },
+    docs: {
+      summary:
+        "Flags newly added catch/except blocks that silently discard errors — empty bodies, unused bindings, or a lone return null.",
+      looksAt: "Added lines in changed JS/TS/Python source files (non-test).",
+      reports: "File, line, and kind: empty-catch, unused-binding, or return-null.",
+      network: "Pure local analyzer. No external network call.",
+      notes:
+        "Python `except: pass` is allowed. Catch blocks that log, rethrow, or reference the caught binding are not flagged.",
+    },
+    render: (findings, helpers) => {
+      if (!findings.length) return [];
+      const lines = ["### Error swallowing (silent catch/except added by this PR)"];
+      for (const item of findings) {
+        lines.push(
+          `- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} — ${helpers.safeCodeSpan(item.kind)}`,
+        );
+      }
+      return lines;
+    },
+    run: (req, { signal }) => scanErrorSwallow(req, signal),
+  }),
   descriptor({
     name: "commitLint",
     title: "Conventional-commit subjects",

review-enrichment/src/render.ts

@@ -482,6 +482,7 @@ export function renderBrief(
   lines.push(...renderDescriptorSection("magicNumber", findings.magicNumber));
   lines.push(...renderDescriptorSection("conflictMarker", findings.conflictMarker));
   lines.push(...renderDescriptorSection("debugLeftover", findings.debugLeftover));
+  lines.push(...renderDescriptorSection("errorSwallow", findings.errorSwallow));
   lines.push(...renderDescriptorSection("hardcodedUrl", findings.hardcodedUrl));
   lines.push(...renderDescriptorSection("commitLint", findings.commitLint));
 

review-enrichment/src/types.ts

@@ -483,6 +483,13 @@ export interface DebugLeftoverFinding {
   kind: "debugger" | "console" | "print";
 }
 
+/** A catch/except block a PR added that swallows an error without logging, rethrowing, or using the binding (#2014). */
+export interface ErrorSwallowFinding {
+  file: string;
+  line: number;
+  kind: "empty-catch" | "unused-binding" | "return-null";
+}
+
 /** An absolute HTTP(S) URL or raw IP:port endpoint hardcoded in non-test, non-config source (#2027, part of #1499).
  *  Reports location, kind, and a redacted/truncated host — never full paths or query strings. */
 export interface HardcodedUrlFinding {
@@ -539,6 +546,7 @@ export interface BriefFindings {
   magicNumber?: MagicNumberFinding[];
   conflictMarker?: ConflictMarkerFinding[];
   debugLeftover?: DebugLeftoverFinding[];
+  errorSwallow?: ErrorSwallowFinding[];
   hardcodedUrl?: HardcodedUrlFinding[];
   commitLint?: CommitLintFinding[];
 }

review-enrichment/test/analyzer-registry.test.ts

@@ -48,6 +48,7 @@ const EXPECTED_ANALYZERS = [
   "magicNumber",
   "conflictMarker",
   "debugLeftover",
+  "errorSwallow",
   "commitLint",
 ];
 

review-enrichment/test/error-swallow.test.ts

@@ -0,0 +1,78 @@
+// Units for the error-swallow analyzer (#2014). Own file so concurrent analyzer PRs don't collide.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import {
+  detectJsCatchSwallow,
+  detectPythonExceptSwallow,
+  scanErrorSwallow,
+  scanPatchForErrorSwallow,
+} from "../dist/analyzers/error-swallow.js";
+import { renderBrief } from "../dist/render.js";
+
+const patchOf = (lines: string[]) =>
+  `@@ -1,0 +1,${lines.length} @@\n${lines.map((l) => `+${l}`).join("\n")}`;
+
+test("detectJsCatchSwallow: flags empty and unused-binding catches", () => {
+  assert.equal(detectJsCatchSwallow("try {} catch (e) {}"), "empty-catch");
+  assert.equal(detectJsCatchSwallow("} catch {}"), "empty-catch");
+  assert.equal(detectJsCatchSwallow("catch (err) { return null; }"), "return-null");
+  assert.equal(detectJsCatchSwallow("catch (err) { doWork(); }"), "unused-binding");
+});
+
+test("detectJsCatchSwallow: does not flag catches that log, rethrow, or use the binding", () => {
+  assert.equal(detectJsCatchSwallow("catch (e) { console.error(e); }"), null);
+  assert.equal(detectJsCatchSwallow("catch (e) { logger.warn(e); }"), null);
+  assert.equal(detectJsCatchSwallow("catch (e) { throw e; }"), null);
+  assert.equal(detectJsCatchSwallow("catch (e) { return handle(e); }"), null);
+});
+
+test("detectPythonExceptSwallow: allows except pass and flags empty/unused bodies", () => {
+  assert.equal(detectPythonExceptSwallow("except Exception: pass"), null);
+  assert.equal(detectPythonExceptSwallow("except Exception as e: pass"), null);
+  assert.equal(detectPythonExceptSwallow("except Exception:"), "empty-catch");
+  assert.equal(detectPythonExceptSwallow("except Exception as e:", "return None"), "return-null");
+  assert.equal(detectPythonExceptSwallow("except Exception as e:", "cleanup()"), "unused-binding");
+});
+
+test("scanPatchForErrorSwallow: flags added lines with correct locations and respects caps", () => {
+  const findings = scanPatchForErrorSwallow(
+    "src/widget.ts",
+    patchOf([
+      "try { doWork(); } catch (e) {}",
+      "try { other(); } catch (err) { return null; }",
+    ]),
+  );
+  assert.deepEqual(findings, [
+    { file: "src/widget.ts", line: 1, kind: "empty-catch" },
+    { file: "src/widget.ts", line: 2, kind: "return-null" },
+  ]);
+  const many = Array.from({ length: 30 }, () => "catch (e) {}");
+  assert.equal(scanPatchForErrorSwallow("src/a.ts", patchOf(many), { maxFindings: 3 }).length, 3);
+});
+
+test("scanPatchForErrorSwallow: skips test files and clean input", () => {
+  assert.deepEqual(
+    scanPatchForErrorSwallow("src/widget.test.ts", patchOf(["catch (e) {}"])),
+    [],
+  );
+  assert.deepEqual(
+    scanPatchForErrorSwallow("src/widget.ts", patchOf(["catch (e) { console.error(e); }"])),
+    [],
+  );
+});
+
+test("scanErrorSwallow: aggregates across files and renders a value-safe brief", async () => {
+  const findings = await scanErrorSwallow({
+    files: [
+      { path: "src/a.ts", patch: patchOf(["catch (e) {}"]) },
+      { path: "lib/b.py", patch: patchOf(["except Exception:"]) },
+    ],
+  });
+  assert.equal(findings.length, 2);
+  const { promptSection } = renderBrief({
+    errorSwallow: findings,
+  });
+  assert.match(promptSection, /Error swallowing/);
+  assert.match(promptSection, /src\/a\.ts:1/);
+  assert.doesNotMatch(promptSection, /catch \(e\)/);
+});

src/review/enrichment-analyzer-names.ts

@@ -42,6 +42,7 @@ export const REES_ANALYZER_NAMES = [
   "magicNumber",
   "conflictMarker",
   "debugLeftover",
+  "errorSwallow",
   "commitLint",
 ] as const;