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
8 changes: 4 additions & 4 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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,sizeSmell,commitLint
# conflictMarker,debugLeftover,sizeSmell,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,sizeSmell
# debugLeftover,sizeSmell,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,sizeSmell,commitLint
# todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,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,sizeSmell,commitLint
# conflictMarker,debugLeftover,sizeSmell,errorSwallow,commitLint
# END GENERATED REES ANALYZERS

# Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep
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 @@ -981,6 +981,28 @@ export const REES_ANALYZERS = [
"File length is estimated from hunk headers (the visible patch), not a full checkout. Function detection is structural (`function` / arrow-with-brace) and counts added body lines until brace balance returns to zero.",
},
},
{
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",
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 @@ -1107,6 +1107,32 @@
"notes": "File length is estimated from hunk headers (the visible patch), not a full checkout. Function detection is structural (`function` / arrow-with-brace) and counts added body lines until brace balance returns to zero."
}
},
{
"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",
Expand Down
196 changes: 196 additions & 0 deletions review-enrichment/src/analyzers/error-swallow.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
// 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 MAX_MULTILINE_ADDED_LINES = 50;

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 isAddedPatchLine(line: string): boolean {
return line.startsWith("+") && !line.startsWith("+++");
}

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 null;
}

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);
}

function classifyJsCatchText(text: string): ErrorSwallowFinding["kind"] | null {
const match = JS_CATCH_RE.exec(codeOnly(text));
if (!match) return null;
return bodySwallowsError(match[2] ?? "", match[1] ?? null, false);
}

function braceBlockComplete(text: string): boolean {
const code = codeOnly(text);
let depth = 0;
let seenOpen = false;
for (const ch of code) {
if (ch === "{") {
depth += 1;
seenOpen = true;
} else if (ch === "}") {
depth -= 1;
}
}
return seenOpen && depth <= 0;
}

/** Collect consecutive added lines until a `{`/`}` block closes. Pure. */
function collectAddedBraceBlock(lines: string[], startIndex: number): { text: string; lineCount: number } | null {
let text = "";
let lineCount = 0;
for (let i = startIndex; i < lines.length && lineCount < MAX_MULTILINE_ADDED_LINES; i += 1) {
const line = lines[i]!;
if (!isAddedPatchLine(line)) break;
text += (lineCount > 0 ? "\n" : "") + line.slice(1);
lineCount += 1;
if (braceBlockComplete(text)) break;
}
return lineCount > 0 ? { text, lineCount } : null;
}

/** Classify one added JS/TS catch (single- or multi-line text), or null when clean / out of scope. Pure. */
export function detectJsCatchSwallow(line: string): ErrorSwallowFinding["kind"] | null {
if (!/\bcatch\b/.test(line)) return null;
return classifyJsCatchText(line);
}

/** Classify one added Python except line (and optional immediate next added body line), 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);
}

function pythonExceptUsesNextAddedLine(line: string): boolean {
const match = PYTHON_EXCEPT_RE.exec(line);
return Boolean(match && !(match[3] ?? "").trim());
}

/** 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) continue;

if (isAddedPatchLine(line)) {
const body = line.slice(1);
if (body.length > MAX_LINE_CHARS) {
newLine += 1;
continue;
}

let kind: ErrorSwallowFinding["kind"] | null = null;
let skipAddedLines = 0;

if (isPython) {
const nextLine = lines[index + 1];
const nextAdded = nextLine && isAddedPatchLine(nextLine) ? nextLine.slice(1) : null;
kind = detectPythonExceptSwallow(body, nextAdded);
if (pythonExceptUsesNextAddedLine(body) && nextAdded) skipAddedLines = 1;
} else if (/\bcatch\b/.test(body)) {
kind = detectJsCatchSwallow(body);
if (!kind) {
const block = collectAddedBraceBlock(lines, index);
if (block) {
kind = classifyJsCatchText(block.text);
skipAddedLines = block.lineCount - 1;
}
}
}

if (kind) {
findings.push({ file: path, line: newLine, kind });
if (findings.length >= maxFindings) return findings;
}

newLine += 1 + skipAddedLines;
index += skipAddedLines;
} else if (!line.startsWith("-") && !line.startsWith("\\")) {
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;
}
30 changes: 30 additions & 0 deletions review-enrichment/src/analyzers/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { scanMagicNumbers } from "./magic-number.js";
import { scanConflictMarkers } from "./conflict-marker.js";
import { scanDebugLeftover } from "./debug-leftover.js";
import { scanSizeSmell } from "./size-smell.js";
import { scanErrorSwallow } from "./error-swallow.js";
import { scanCommitLint } from "./commit-lint.js";
import { scanTerminology } from "./terminology.js";
import { scanTodoMarker } from "./todo-marker.js";
Expand Down Expand Up @@ -1046,6 +1047,35 @@ export const ANALYZER_DESCRIPTORS = [
},
run: (req, { signal }) => scanSizeSmell(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",
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 @@ -483,6 +483,7 @@ export function renderBrief(
lines.push(...renderDescriptorSection("conflictMarker", findings.conflictMarker));
lines.push(...renderDescriptorSection("debugLeftover", findings.debugLeftover));
lines.push(...renderDescriptorSection("sizeSmell", findings.sizeSmell));
lines.push(...renderDescriptorSection("errorSwallow", findings.errorSwallow));
lines.push(...renderDescriptorSection("hardcodedUrl", findings.hardcodedUrl));
lines.push(...renderDescriptorSection("commitLint", findings.commitLint));

Expand Down
8 changes: 8 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,13 @@ export interface SizeSmellFinding {
name?: string;
}

/** 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 {
Expand Down Expand Up @@ -551,6 +558,7 @@ export interface BriefFindings {
conflictMarker?: ConflictMarkerFinding[];
debugLeftover?: DebugLeftoverFinding[];
sizeSmell?: SizeSmellFinding[];
errorSwallow?: ErrorSwallowFinding[];
hardcodedUrl?: HardcodedUrlFinding[];
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 @@ -49,6 +49,7 @@ const EXPECTED_ANALYZERS = [
"conflictMarker",
"debugLeftover",
"sizeSmell",
"errorSwallow",
"commitLint",
];

Expand Down
Loading
Loading