Skip to content

Commit 2fb5fb6

Browse files
feat(enrichment): add empty-catch / error-swallow analyzer
Fixes #2014 — flags swallowed errors in added JS/TS/Python catch/except blocks. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent b5d2956 commit 2fb5fb6

9 files changed

Lines changed: 364 additions & 12 deletions

File tree

.env.example

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,25 +65,26 @@ GITTENSORY_REVIEW_ENRICHMENT=false
6565
# Current analyzer names:
6666
# dependency,lockfileDrift,secret,license,installScript,heavyDependency,hardcodedUrl,actionPin
6767
# eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature,iacMisconfig
68-
# nativeBuild,history,docCommentDrift,duplication,churnHotspot,blameLink,approvalIntegrity
69-
# ciCheckSignals,undocumentedExport,staleBranch,commitHygiene,pendingReviewRequests,testRatio
70-
# migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover
71-
# commitLint
68+
# nativeBuild,history,docCommentDrift,duplication,errorSwallow,churnHotspot,blameLink
69+
# approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
70+
# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
71+
# conflictMarker,debugLeftover,commitLint
7272
#
7373
# Profile defaults:
7474
# fast: dependency,lockfileDrift,secret,license,installScript,heavyDependency,hardcodedUrl
75-
# actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild,testRatio
76-
# migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover
75+
# actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild,errorSwallow
76+
# testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker
77+
# debugLeftover
7778
# balanced (default): dependency,lockfileDrift,secret,license,installScript,heavyDependency
7879
# hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat
79-
# commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot
80-
# blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
81-
# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
82-
# conflictMarker,debugLeftover,commitLint
80+
# commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,errorSwallow
81+
# churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch
82+
# commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology
83+
# todoMarker,magicNumber,conflictMarker,debugLeftover,commitLint
8384
# deep: dependency,lockfileDrift,secret,license,installScript,heavyDependency,hardcodedUrl
8485
# actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature
85-
# iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot,blameLink
86-
# approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
86+
# iacMisconfig,nativeBuild,history,docCommentDrift,duplication,errorSwallow,churnHotspot
87+
# blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
8788
# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
8889
# conflictMarker,debugLeftover,commitLint
8990
# END GENERATED REES ANALYZERS

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,28 @@ export const REES_ANALYZERS = [
559559
"Conservative: trivial/boilerplate lines are dropped and a long contiguous run is required, so incidental overlap is not flagged. Never returns code content.",
560560
},
561561
},
562+
{
563+
name: "errorSwallow",
564+
title: "Swallowed errors",
565+
category: "quality",
566+
cost: "local",
567+
defaultEnabled: true,
568+
profiles: ["fast", "balanced", "deep"],
569+
requires: ["files"],
570+
limits: {
571+
maxFindings: 25,
572+
maxLineChars: 2000,
573+
},
574+
docs: {
575+
summary:
576+
"Flags newly-added catch/except blocks that swallow errors without logging or rethrowing.",
577+
looksAt: "Added lines in changed JS/TS/Python source files, excluding tests.",
578+
reports: "File, line, and kind: empty-catch, unused-binding, or return-null.",
579+
network: "Pure local analyzer. No external network call.",
580+
notes:
581+
"Precision-first: catches that log, rethrow, or reference the binding are not flagged. Python `except: pass` is included.",
582+
},
583+
},
562584
{
563585
name: "churnHotspot",
564586
title: "Churn hotspots",

review-enrichment/analyzer-metadata.json

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,32 @@
637637
"notes": "Conservative: trivial/boilerplate lines are dropped and a long contiguous run is required, so incidental overlap is not flagged. Never returns code content."
638638
}
639639
},
640+
{
641+
"name": "errorSwallow",
642+
"title": "Swallowed errors",
643+
"category": "quality",
644+
"cost": "local",
645+
"defaultEnabled": true,
646+
"profiles": [
647+
"fast",
648+
"balanced",
649+
"deep"
650+
],
651+
"requires": [
652+
"files"
653+
],
654+
"limits": {
655+
"maxFindings": 25,
656+
"maxLineChars": 2000
657+
},
658+
"docs": {
659+
"summary": "Flags newly-added catch/except blocks that swallow errors without logging or rethrowing.",
660+
"looksAt": "Added lines in changed JS/TS/Python source files, excluding tests.",
661+
"reports": "File, line, and kind: empty-catch, unused-binding, or return-null.",
662+
"network": "Pure local analyzer. No external network call.",
663+
"notes": "Precision-first: catches that log, rethrow, or reference the binding are not flagged. Python `except: pass` is included."
664+
}
665+
},
640666
{
641667
"name": "churnHotspot",
642668
"title": "Churn hotspots",
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
// Empty-catch / error-swallow analyzer (#2014). Flags newly-added catch/except blocks that swallow the error
2+
// (empty body, unused binding, or a bare `return null`) — a top source of silent failures. Pure compute over
3+
// added diff lines, no network. Scoped to JS/TS/Python source files; Python `except: pass` is included.
4+
import type { EnrichRequest, ErrorSwallowFinding } from "../types.js";
5+
import { isTestPath } from "./test-ratio.js";
6+
7+
const MAX_FINDINGS = 25;
8+
const MAX_LINE_CHARS = 2000;
9+
10+
const SOURCE_EXTS = new Set(["ts", "tsx", "js", "jsx", "mjs", "cjs", "mts", "cts", "py"]);
11+
12+
const CATCH_OPEN_RE = /catch\s*\(\s*(\w+)?\s*\)\s*\{/;
13+
const SINGLE_CATCH_RE = /catch\s*\(\s*(\w+)?\s*\)\s*\{([\s\S]*?)\}/;
14+
const PY_EXCEPT_PASS_RE = /^\s*except(?:\s+[\w.]+\s*(?:as\s+(\w+))?)?\s*:\s*pass\s*(?:#.*)?$/;
15+
16+
function isScannablePath(path: string): boolean {
17+
const ext = /\.([^.]+)$/.exec(path)?.[1]?.toLowerCase();
18+
return Boolean(ext && SOURCE_EXTS.has(ext) && !isTestPath(path));
19+
}
20+
21+
function escapeRegExp(value: string): string {
22+
return value.replace(/[$.*+?^{}()|[\]\\]/g, "\\$&");
23+
}
24+
25+
function bodySwallowsError(body: string, binding: string | null): ErrorSwallowFinding["kind"] | null {
26+
const trimmed = body.trim();
27+
if (!trimmed) return "empty-catch";
28+
if (/^return\s+null\s*;?$/.test(trimmed)) return "return-null";
29+
if (!binding) return null;
30+
const bindingRe = new RegExp(`\\b${escapeRegExp(binding)}\\b`);
31+
if (/\bthrow\b/.test(trimmed)) return null;
32+
if (/\b(?:console|logger|log|winston|pino|bunyan)\s*[.(]/.test(trimmed)) return null;
33+
if (/\bprint\s*\(/.test(trimmed)) return null;
34+
if (!bindingRe.test(trimmed)) return "unused-binding";
35+
return null;
36+
}
37+
38+
/** Classify one source line for an error-swallow pattern, or null. Pure. */
39+
export function detectErrorSwallow(line: string): ErrorSwallowFinding["kind"] | null {
40+
const pyMatch = PY_EXCEPT_PASS_RE.exec(line);
41+
if (pyMatch) {
42+
return pyMatch[1] ? "unused-binding" : "empty-catch";
43+
}
44+
45+
const single = SINGLE_CATCH_RE.exec(line);
46+
if (single) {
47+
return bodySwallowsError(single[2] ?? "", single[1] ?? null);
48+
}
49+
50+
return null;
51+
}
52+
53+
type PendingCatch = {
54+
startLine: number;
55+
binding: string | null;
56+
body: string;
57+
depth: number;
58+
};
59+
60+
function updatePending(pending: PendingCatch, line: string): PendingCatch {
61+
let depth = pending.depth;
62+
for (const ch of line) {
63+
if (ch === "{") depth++;
64+
else if (ch === "}") depth--;
65+
}
66+
return { ...pending, body: `${pending.body}\n${line}`, depth };
67+
}
68+
69+
function flushPending(pending: PendingCatch): ErrorSwallowFinding["kind"] | null {
70+
const body = pending.body.replace(/^\s*\{/, "").replace(/\}\s*$/, "");
71+
return bodySwallowsError(body, pending.binding);
72+
}
73+
74+
type ScanLimits = {
75+
maxFindings?: number;
76+
signal?: AbortSignal;
77+
};
78+
79+
/** Scan one file patch's added lines for swallowed errors, line-cited via hunk headers. Pure. */
80+
export function scanPatchForErrorSwallow(
81+
path: string,
82+
patch: string,
83+
limits: ScanLimits = {},
84+
): ErrorSwallowFinding[] {
85+
const maxFindings = limits.maxFindings ?? MAX_FINDINGS;
86+
if (maxFindings <= 0 || !isScannablePath(path)) return [];
87+
const findings: ErrorSwallowFinding[] = [];
88+
let newLine = 0;
89+
let inHunk = false;
90+
let pending: PendingCatch | null = null;
91+
92+
const pushFinding = (line: number, kind: ErrorSwallowFinding["kind"]) => {
93+
findings.push({ file: path, line, kind });
94+
};
95+
96+
for (const line of patch.split("\n")) {
97+
if (limits.signal?.aborted) throw new Error("analyzer_aborted");
98+
const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line);
99+
if (hunk) {
100+
newLine = Number(hunk[1]);
101+
inHunk = true;
102+
pending = null;
103+
continue;
104+
}
105+
if (!inHunk) continue;
106+
107+
if (line.startsWith("+")) {
108+
const body = line.slice(1);
109+
if (body.length <= MAX_LINE_CHARS) {
110+
if (pending) {
111+
pending = updatePending(pending, body);
112+
if (pending.depth <= 0) {
113+
const kind = flushPending(pending);
114+
if (kind) pushFinding(pending.startLine, kind);
115+
pending = null;
116+
}
117+
} else {
118+
const kind = detectErrorSwallow(body);
119+
if (kind) {
120+
pushFinding(newLine, kind);
121+
if (findings.length >= maxFindings) return findings;
122+
} else {
123+
const open = CATCH_OPEN_RE.exec(body);
124+
if (open) {
125+
const braceIndex = body.indexOf("{", open.index ?? 0);
126+
if (braceIndex >= 0 && !body.slice(braceIndex + 1).includes("}")) {
127+
let depth = 0;
128+
for (const ch of body.slice(braceIndex)) {
129+
if (ch === "{") depth++;
130+
else if (ch === "}") depth--;
131+
}
132+
pending = {
133+
startLine: newLine,
134+
binding: open[1] ?? null,
135+
body: body.slice(braceIndex),
136+
depth,
137+
};
138+
}
139+
}
140+
}
141+
}
142+
}
143+
newLine++;
144+
} else if (!line.startsWith("-") && !line.startsWith("\\")) {
145+
pending = null;
146+
newLine++;
147+
} else {
148+
pending = null;
149+
}
150+
151+
if (findings.length >= maxFindings) return findings;
152+
}
153+
154+
return findings;
155+
}
156+
157+
/** Analyzer entrypoint: scan every changed scannable file's added lines for swallowed errors. */
158+
export async function scanErrorSwallow(
159+
req: EnrichRequest,
160+
signal?: AbortSignal,
161+
): Promise<ErrorSwallowFinding[]> {
162+
const findings: ErrorSwallowFinding[] = [];
163+
for (const file of req.files ?? []) {
164+
if (signal?.aborted) throw new Error("analyzer_aborted");
165+
if (!file.patch) continue;
166+
for (const finding of scanPatchForErrorSwallow(file.path, file.patch, {
167+
maxFindings: MAX_FINDINGS - findings.length,
168+
signal,
169+
})) {
170+
findings.push(finding);
171+
if (findings.length >= MAX_FINDINGS) return findings;
172+
}
173+
}
174+
return findings;
175+
}

review-enrichment/src/analyzers/registry.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { scanCommitSignature } from "./commit-signature.js";
1010
import { dependencyAnalyzer } from "./dependency/descriptor.js";
1111
import { scanDocCommentDrift } from "./doc-comment-drift.js";
1212
import { scanDuplication } from "./duplication-scan.js";
13+
import { scanErrorSwallow } from "./error-swallow.js";
1314
import { scanEol } from "./eol-check.js";
1415
import { scanHardcodedUrl } from "./hardcoded-url.js";
1516
import { scanHeavyDependencies } from "./heavy-dependency.js";
@@ -466,6 +467,35 @@ export const ANALYZER_DESCRIPTORS = [
466467
run: (req, { signal, analysis, diagnostics }) =>
467468
scanDuplication(req, fetch, { signal, analysis, diagnostics }),
468469
}),
470+
descriptor({
471+
name: "errorSwallow",
472+
title: "Swallowed errors",
473+
category: "quality",
474+
cost: "local",
475+
defaultEnabled: true,
476+
requires: ["files"],
477+
limits: { maxFindings: 25, maxLineChars: 2000 },
478+
docs: {
479+
summary:
480+
"Flags newly-added catch/except blocks that swallow errors without logging or rethrowing.",
481+
looksAt: "Added lines in changed JS/TS/Python source files, excluding tests.",
482+
reports: "File, line, and kind: empty-catch, unused-binding, or return-null.",
483+
network: "Pure local analyzer. No external network call.",
484+
notes:
485+
"Precision-first: catches that log, rethrow, or reference the binding are not flagged. Python `except: pass` is included.",
486+
},
487+
render: (findings, helpers) => {
488+
if (!findings.length) return [];
489+
const lines = ["### Swallowed errors (empty catch / unused binding / return null)"];
490+
for (const item of findings) {
491+
lines.push(
492+
`- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)}${helpers.safeCodeSpan(item.kind)}`,
493+
);
494+
}
495+
return lines;
496+
},
497+
run: (req, { signal }) => scanErrorSwallow(req, signal),
498+
}),
469499
descriptor({
470500
name: "churnHotspot",
471501
title: "Churn hotspots",

review-enrichment/src/render.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,7 @@ export function renderBrief(
481481
lines.push(...renderDescriptorSection("magicNumber", findings.magicNumber));
482482
lines.push(...renderDescriptorSection("conflictMarker", findings.conflictMarker));
483483
lines.push(...renderDescriptorSection("debugLeftover", findings.debugLeftover));
484+
lines.push(...renderDescriptorSection("errorSwallow", findings.errorSwallow));
484485
lines.push(...renderDescriptorSection("hardcodedUrl", findings.hardcodedUrl));
485486
lines.push(...renderDescriptorSection("commitLint", findings.commitLint));
486487

review-enrichment/src/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,14 @@ export interface HardcodedUrlFinding {
482482
host: string;
483483
}
484484

485+
/** A newly-added catch/except block that swallows an error without logging or rethrowing (#2014, part of #1499).
486+
* Reports location and kind only — never catch bodies or stack traces. */
487+
export interface ErrorSwallowFinding {
488+
file: string;
489+
line: number;
490+
kind: "empty-catch" | "unused-binding" | "return-null";
491+
}
492+
485493
/** A PR commit subject that does not conform to the Conventional Commits spec (#2021, part of #1499). Reports a
486494
* short SHA prefix, the subject, and the failing reason — never author/email. */
487495
export interface CommitLintFinding {
@@ -529,6 +537,7 @@ export interface BriefFindings {
529537
conflictMarker?: ConflictMarkerFinding[];
530538
debugLeftover?: DebugLeftoverFinding[];
531539
hardcodedUrl?: HardcodedUrlFinding[];
540+
errorSwallow?: ErrorSwallowFinding[];
532541
commitLint?: CommitLintFinding[];
533542
}
534543

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ const EXPECTED_ANALYZERS = [
3131
"history",
3232
"docCommentDrift",
3333
"duplication",
34+
"errorSwallow",
3435
"churnHotspot",
3536
"blameLink",
3637
"approvalIntegrity",

0 commit comments

Comments
 (0)