Skip to content
Closed
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
25 changes: 13 additions & 12 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -65,25 +65,26 @@ GITTENSORY_REVIEW_ENRICHMENT=false
# Current analyzer names:
# dependency,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
# nativeBuild,history,docCommentDrift,duplication,errorSwallow,churnHotspot,blameLink
# approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
# conflictMarker,debugLeftover,commitLint
#
# Profile defaults:
# fast: dependency,lockfileDrift,secret,license,installScript,heavyDependency,hardcodedUrl
# actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild,testRatio
# migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover
# actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild,errorSwallow
# testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker
# debugLeftover
# balanced (default): dependency,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
# commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,errorSwallow
# churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch
# commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology
# todoMarker,magicNumber,conflictMarker,debugLeftover,commitLint
# deep: dependency,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
# iacMisconfig,nativeBuild,history,docCommentDrift,duplication,errorSwallow,churnHotspot
# blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
# conflictMarker,debugLeftover,commitLint
# END GENERATED REES ANALYZERS
Expand Down
22 changes: 22 additions & 0 deletions apps/gittensory-ui/src/lib/rees-analyzers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,28 @@ export const REES_ANALYZERS = [
"Conservative: trivial/boilerplate lines are dropped and a long contiguous run is required, so incidental overlap is not flagged. Never returns code content.",
},
},
{
name: "errorSwallow",
title: "Swallowed errors",
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 swallow errors without logging or rethrowing.",
looksAt: "Added lines in changed JS/TS/Python source files, excluding tests.",
reports: "File, line, and kind: empty-catch, unused-binding, or return-null.",
network: "Pure local analyzer. No external network call.",
notes:
"Precision-first: catches that log, rethrow, or reference the binding are not flagged. Python `except: pass` is included.",
},
},
{
name: "churnHotspot",
title: "Churn hotspots",
Expand Down
26 changes: 26 additions & 0 deletions review-enrichment/analyzer-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,32 @@
"notes": "Conservative: trivial/boilerplate lines are dropped and a long contiguous run is required, so incidental overlap is not flagged. Never returns code content."
}
},
{
"name": "errorSwallow",
"title": "Swallowed errors",
"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 swallow errors without logging or rethrowing.",
"looksAt": "Added lines in changed JS/TS/Python source files, excluding tests.",
"reports": "File, line, and kind: empty-catch, unused-binding, or return-null.",
"network": "Pure local analyzer. No external network call.",
"notes": "Precision-first: catches that log, rethrow, or reference the binding are not flagged. Python `except: pass` is included."
}
},
{
"name": "churnHotspot",
"title": "Churn hotspots",
Expand Down
175 changes: 175 additions & 0 deletions review-enrichment/src/analyzers/error-swallow.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
// Empty-catch / error-swallow analyzer (#2014). Flags newly-added catch/except blocks that swallow the error
// (empty body, unused binding, or a bare `return null`) — a top source of silent failures. Pure compute over
// added diff lines, no network. Scoped to JS/TS/Python source files; Python `except: pass` is included.
import type { EnrichRequest, ErrorSwallowFinding } from "../types.js";
import { isTestPath } from "./test-ratio.js";

const MAX_FINDINGS = 25;
const MAX_LINE_CHARS = 2000;

const SOURCE_EXTS = new Set(["ts", "tsx", "js", "jsx", "mjs", "cjs", "mts", "cts", "py"]);

const CATCH_OPEN_RE = /catch\s*\(\s*(\w+)?\s*\)\s*\{/;
const SINGLE_CATCH_RE = /catch\s*\(\s*(\w+)?\s*\)\s*\{([\s\S]*?)\}/;
const PY_EXCEPT_PASS_RE = /^\s*except(?:\s+[\w.]+\s*(?:as\s+(\w+))?)?\s*:\s*pass\s*(?:#.*)?$/;

function isScannablePath(path: string): boolean {
const ext = /\.([^.]+)$/.exec(path)?.[1]?.toLowerCase();
return Boolean(ext && SOURCE_EXTS.has(ext) && !isTestPath(path));
}

function escapeRegExp(value: string): string {
return value.replace(/[$.*+?^{}()|[\]\\]/g, "\\$&");
}

function bodySwallowsError(body: string, binding: string | null): ErrorSwallowFinding["kind"] | null {
const trimmed = body.trim();
if (!trimmed) return "empty-catch";
if (/^return\s+null\s*;?$/.test(trimmed)) return "return-null";
if (!binding) return null;
const bindingRe = new RegExp(`\\b${escapeRegExp(binding)}\\b`);
if (/\bthrow\b/.test(trimmed)) return null;
if (/\b(?:console|logger|log|winston|pino|bunyan)\s*[.(]/.test(trimmed)) return null;
if (/\bprint\s*\(/.test(trimmed)) return null;
if (!bindingRe.test(trimmed)) return "unused-binding";
return null;
}

/** Classify one source line for an error-swallow pattern, or null. Pure. */
export function detectErrorSwallow(line: string): ErrorSwallowFinding["kind"] | null {
const pyMatch = PY_EXCEPT_PASS_RE.exec(line);
if (pyMatch) {
return pyMatch[1] ? "unused-binding" : "empty-catch";
}

const single = SINGLE_CATCH_RE.exec(line);
if (single) {
return bodySwallowsError(single[2] ?? "", single[1] ?? null);
}

return null;
}

type PendingCatch = {
startLine: number;
binding: string | null;
body: string;
depth: number;
};

function updatePending(pending: PendingCatch, line: string): PendingCatch {
let depth = pending.depth;
for (const ch of line) {
if (ch === "{") depth++;
else if (ch === "}") depth--;
}
return { ...pending, body: `${pending.body}\n${line}`, depth };
}

function flushPending(pending: PendingCatch): ErrorSwallowFinding["kind"] | null {
const body = pending.body.replace(/^\s*\{/, "").replace(/\}\s*$/, "");
return bodySwallowsError(body, pending.binding);
}

type ScanLimits = {
maxFindings?: number;
signal?: AbortSignal;
};

/** Scan one file patch's added lines for swallowed errors, 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 || !isScannablePath(path)) return [];
const findings: ErrorSwallowFinding[] = [];
let newLine = 0;
let inHunk = false;
let pending: PendingCatch | null = null;

const pushFinding = (line: number, kind: ErrorSwallowFinding["kind"]) => {
findings.push({ file: path, line, kind });
};

for (const line of patch.split("\n")) {
if (limits.signal?.aborted) throw new Error("analyzer_aborted");
const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line);
if (hunk) {
newLine = Number(hunk[1]);
inHunk = true;
pending = null;
continue;
}
if (!inHunk) continue;

if (line.startsWith("+")) {
const body = line.slice(1);
if (body.length <= MAX_LINE_CHARS) {
if (pending) {
pending = updatePending(pending, body);
if (pending.depth <= 0) {
const kind = flushPending(pending);
if (kind) pushFinding(pending.startLine, kind);
pending = null;
}
} else {
const kind = detectErrorSwallow(body);
if (kind) {
pushFinding(newLine, kind);
if (findings.length >= maxFindings) return findings;
} else {
const open = CATCH_OPEN_RE.exec(body);
if (open) {
const braceIndex = body.indexOf("{", open.index ?? 0);
if (braceIndex >= 0 && !body.slice(braceIndex + 1).includes("}")) {
let depth = 0;
for (const ch of body.slice(braceIndex)) {
if (ch === "{") depth++;
else if (ch === "}") depth--;
}
pending = {
startLine: newLine,
binding: open[1] ?? null,
body: body.slice(braceIndex),
depth,
};
}
}
}
}
}
newLine++;
} else if (!line.startsWith("-") && !line.startsWith("\\")) {
pending = null;
newLine++;
} else {
pending = null;
}

if (findings.length >= maxFindings) return findings;
}

return findings;
}

/** Analyzer entrypoint: scan every changed scannable file's added lines for swallowed errors. */
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;
}
30 changes: 30 additions & 0 deletions review-enrichment/src/analyzers/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { scanCommitSignature } from "./commit-signature.js";
import { dependencyAnalyzer } from "./dependency/descriptor.js";
import { scanDocCommentDrift } from "./doc-comment-drift.js";
import { scanDuplication } from "./duplication-scan.js";
import { scanErrorSwallow } from "./error-swallow.js";
import { scanEol } from "./eol-check.js";
import { scanHardcodedUrl } from "./hardcoded-url.js";
import { scanHeavyDependencies } from "./heavy-dependency.js";
Expand Down Expand Up @@ -466,6 +467,35 @@ export const ANALYZER_DESCRIPTORS = [
run: (req, { signal, analysis, diagnostics }) =>
scanDuplication(req, fetch, { signal, analysis, diagnostics }),
}),
descriptor({
name: "errorSwallow",
title: "Swallowed errors",
category: "quality",
cost: "local",
defaultEnabled: true,
requires: ["files"],
limits: { maxFindings: 25, maxLineChars: 2000 },
docs: {
summary:
"Flags newly-added catch/except blocks that swallow errors without logging or rethrowing.",
looksAt: "Added lines in changed JS/TS/Python source files, excluding tests.",
reports: "File, line, and kind: empty-catch, unused-binding, or return-null.",
network: "Pure local analyzer. No external network call.",
notes:
"Precision-first: catches that log, rethrow, or reference the binding are not flagged. Python `except: pass` is included.",
},
render: (findings, helpers) => {
if (!findings.length) return [];
const lines = ["### Swallowed errors (empty catch / unused binding / return null)"];
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: "churnHotspot",
title: "Churn hotspots",
Expand Down
1 change: 1 addition & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,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));

Expand Down
9 changes: 9 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,14 @@ export interface HardcodedUrlFinding {
host: string;
}

/** A newly-added catch/except block that swallows an error without logging or rethrowing (#2014, part of #1499).
* Reports location and kind only — never catch bodies or stack traces. */
export interface ErrorSwallowFinding {
file: string;
line: number;
kind: "empty-catch" | "unused-binding" | "return-null";
}

/** A PR commit subject that does not conform to the Conventional Commits spec (#2021, part of #1499). Reports a
* short SHA prefix, the subject, and the failing reason — never author/email. */
export interface CommitLintFinding {
Expand Down Expand Up @@ -529,6 +537,7 @@ export interface BriefFindings {
conflictMarker?: ConflictMarkerFinding[];
debugLeftover?: DebugLeftoverFinding[];
hardcodedUrl?: HardcodedUrlFinding[];
errorSwallow?: ErrorSwallowFinding[];
commitLint?: CommitLintFinding[];
}

Expand Down
1 change: 1 addition & 0 deletions review-enrichment/test/analyzer-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const EXPECTED_ANALYZERS = [
"history",
"docCommentDrift",
"duplication",
"errorSwallow",
"churnHotspot",
"blameLink",
"approvalIntegrity",
Expand Down
Loading
Loading