Skip to content

feat(errors): wire up no-silent-catch lint rule, drop ratchet baseline #1532

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BYK merged 1 commit into from
Sep 4, 2026
Merged

feat(errors): wire up no-silent-catch lint rule, drop ratchet baseline #1532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 13 additions & 9 deletions packages/cli/AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ CliError (base, exitCode=1)
- Pass `alternatives: []` when defaults are irrelevant (e.g., for missing Trace ID, Event ID)
- Use `" and "` in `resource` for plural grammar: `"Trace ID and span ID"` → "are required"

**CI enforcement:** `pnpm run check:errors` scans for `ContextError` with multiline commands, `CliError` with ad-hoc "Try:" strings, and silent `catch` blocks (ratchet baseline — new ones fail CI).
**CI enforcement:** `pnpm run check:errors` scans for `ContextError` with multiline commands and `CliError` with ad-hoc "Try:" strings. Silent `catch` blocks are enforced separately by the `no-silent-catch` Biome plugin (see below).

```typescript
// Usage examples
Expand Down Expand Up @@ -570,14 +570,18 @@ catch (error) {

Use `logger.withTag("command-name")` for tagged logging in command files.

**CI enforcement:** `pnpm run check:errors` includes a silent-catch scan that flags
`catch` blocks which are empty, comment-only, or return-only without surfacing the
error. It is enforced with a **ratchet baseline** (`script/silent-catch-baseline.json`)
recording the per-file count of the pre-existing backlog: a *new* silent catch (a file
exceeding its baseline, or one not in the baseline) fails CI, and removing silent
catches without lowering the baseline also fails — so the backlog can only shrink.
When you fix or intentionally add a silent catch, refresh the baseline with
`pnpm run check:errors -- --update` and commit it.
**CI enforcement:** the `no-silent-catch` Biome plugin
(`lint-rules/no-silent-catch.grit`, registered in `biome.jsonc`) flags `catch`
blocks — statement and `.catch()` form — that are empty, comment-only, or
return-only without surfacing the error. The pre-existing backlog is
grandfathered in place with inline
`// biome-ignore lint/plugin: <reason>` comments. Because `pnpm run lint` runs
with `--error-on-warnings`, an *orphaned* suppression (left behind when a
grandfathered catch is fixed) fails as `suppressions/unused` — so the backlog
can only shrink, the same ratchet the old JSON baseline provided, with no
separate script or baseline file to maintain. Fix a grandfathered catch by
adding logging/re-throwing and deleting its `biome-ignore` line; only add a new
suppression for a genuinely intentional silent catch, with a real reason.

### Auto-Recovery for Wrong Entity Types

Expand Down
10 changes: 10 additions & 0 deletions packages/cli/biome.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@
}
},
"overrides": [
{
// Silent-catch enforcement is scoped to production code under src/**.
// The grandfathered backlog is pinned with inline `// biome-ignore
// lint/plugin` comments; `pnpm run lint` runs with `--error-on-warnings`
// so an orphaned suppression fails as `suppressions/unused`, keeping the
// backlog shrink-only. Replaces the old script/silent-catch-baseline.json
// ratchet (see #1531).
"includes": ["src/**/*.ts"],
"plugins": ["./lint-rules/no-silent-catch.grit"]
Comment thread
BYK marked this conversation as resolved.
},
{
// The React-hook lint rules infer "this is a hook" from the
// `use*` naming convention. We have a couple of test helpers
Expand Down
17 changes: 9 additions & 8 deletions packages/cli/lint-rules/no-silent-catch.grit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ language js
// covers syntactically empty `catch {}`; this also covers comment-only and
// return-only bodies.
//
// This replaces the hand-rolled detector in script/check-error-patterns.ts.
// The pre-existing backlog is grandfathered with inline
// `// biome-ignore lint/plugin: <reason>` comments. Removing a grandfathered
// catch orphans its suppression, which Biome reports as `suppressions/unused`;
// the lint step runs with `--error-on-warnings`, so the backlog can only shrink
// (the same ratchet the old baseline JSON provided).
// This replaces the hand-rolled silent-catch detector and the
// script/silent-catch-baseline.json ratchet (see #1531). The pre-existing
// backlog is grandfathered with inline `// biome-ignore lint/plugin: <reason>`
// comments. Removing a grandfathered catch orphans its suppression, which Biome
// reports as `suppressions/unused`; `pnpm run lint` runs with
// `--error-on-warnings`, so the backlog can only shrink (the same ratchet the
// old baseline JSON provided).
//
// Registered via an override scoped to `src/**` in biome.jsonc (the old script
// only scanned `src/**/*.ts`).
// Registered via an override scoped to `src/**/*.ts` in biome.jsonc (matching
// the old script's scan scope).
or {
`try { $t } catch { }`,
`try { $t } catch { return; }`,
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"build:all": "pnpm run generate:schema && pnpm run generate:docs && pnpm run generate:sdk && pnpm tsx script/build.ts",
"bundle": "pnpm run generate:schema && pnpm run generate:docs && pnpm run generate:sdk && pnpm tsx script/bundle.ts",
"typecheck": "pnpm run generate:docs && pnpm run generate:sdk && tsc --noEmit",
"lint": "biome check --no-errors-on-unmatched --max-diagnostics=none ./",
"lint": "biome check --no-errors-on-unmatched --error-on-warnings --max-diagnostics=none ./",
"lint:fix": "biome check --write --no-errors-on-unmatched --max-diagnostics=none ./",
"test": "pnpm run test:unit",
"test:unit": "pnpm run generate:docs && pnpm run generate:sdk && vitest run test/lib test/commands test/types test/script --coverage",
Expand Down
258 changes: 14 additions & 244 deletions packages/cli/script/check-error-patterns.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,48 +10,28 @@
* 2. `new CliError(... "Try:" ...)` — ad-hoc "Try:" strings
* → Should use ResolutionError with structured hint/suggestions
*
* 3. Silent catch blocks — `catch { ... }` whose body has no logging, no
* re-throw, and at most a bare `return`. Errors must be surfaced via
* `log.debug`/`log.warn` (or re-thrown) per AGENTS.md. Biome's
* `noEmptyBlockStatements` only catches syntactically empty `catch {}`;
* this catches comment-only and return-only blocks too.
*
* Silent catches are enforced with a **ratchet baseline**
* (`silent-catch-baseline.json`): the repo has a pre-existing backlog of
* best-effort catches (UI teardown, cleanup paths, etc.). The baseline records
* the known per-file count so that:
* - a *new* silent catch (a file exceeding its baseline, or a file absent from
* the baseline) fails CI, and
* - removing silent catches without lowering the baseline also fails CI, so
* the backlog can only shrink.
* Run with `--update` to regenerate the baseline after intentionally changing
* the set of silent catches.
* Silent catch blocks used to be checked here via a ratchet baseline. That
* check now lives in the Biome plugin `lint-rules/no-silent-catch.grit`, whose
* grandfathered backlog is pinned inline with `// biome-ignore lint/plugin`
* comments (an unused suppression is itself reported, so the backlog can only
* shrink). See #1531.
*
* Usage:
* tsx script/check-error-patterns.ts # check (fails CI on drift)
* tsx script/check-error-patterns.ts --update # rewrite the baseline
* tsx script/check-error-patterns.ts # check (fails CI on any violation)
*
* Exit codes:
* 0 - No anti-patterns found and silent-catch baseline is in sync
* 1 - Anti-patterns detected or silent-catch baseline drifted
* 0 - No anti-patterns found
* 1 - Anti-patterns detected
*/

import { readFile, writeFile } from "node:fs/promises";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
import { readFile } from "node:fs/promises";
import { glob } from "tinyglobby";

export type Violation = { file: string; line: number; message: string };

/** Per-file count of grandfathered silent catch blocks. */
export type SilentCatchBaseline = Record<string, number>;

const CONTEXT_ERROR_RE = /new ContextError\(/g;
const TRY_PATTERN_RE = /["'`]Try:/;

const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url));
export const BASELINE_PATH = join(SCRIPT_DIR, "silent-catch-baseline.json");

/** Characters that open a nesting level in JavaScript source. */
function isOpener(ch: string): boolean {
return ch === "(" || ch === "[" || ch === "{";
Expand Down Expand Up @@ -288,191 +268,22 @@ export function findAdHocTryPatterns(
return found;
}

/** Matches the start of a catch block in both statement and promise form. */
const CATCH_RE =
/\bcatch\s*(?:\(\s*(\w+)[^)]*\)\s*)?\{|\.catch\(\s*(?:\(\s*(\w+)[^)]*\)|(\w+))\s*=>\s*\{/g;

/** Tokens inside a catch body that prove the error is surfaced (not silenced). */
const SURFACING_RE =
/\b(?:log|logger|console)\s*\.|[^.]\bthrow\b|captureException|reportError/;

/** A catch body consisting solely of a single `return ...;` statement. */
const RETURN_ONLY_RE = /^return\b[^;]*;?$/;

/**
* Return the source of a balanced `{...}` block given the index of its opening
* brace, skipping strings so braces inside literals don't break depth tracking.
*/
function readBlock(content: string, openBraceIdx: number): string {
let depth = 0;
let i = openBraceIdx;
while (i < content.length) {
const { next, ch } = advanceToken(content, i);
if (ch === "{") {
depth += 1;
} else if (ch === "}") {
depth -= 1;
if (depth === 0) {
return content.slice(openBraceIdx + 1, i);
}
}
i = next;
}
return content.slice(openBraceIdx + 1);
}

/**
* Strip line and block comments from a snippet so comment-only catch bodies are
* treated as empty.
*/
function stripComments(snippet: string): string {
return snippet.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/[^\n]*/g, "");
}

/**
* Detect silent catch blocks: catch bodies that, after removing comments, are
* empty or contain only a bare `return;`/`return <value>;` with no logging or
* re-throw. These hide errors and violate the AGENTS.md no-silent-catch rule.
*/
export function findSilentCatches(
content: string,
filePath: string
): Violation[] {
const found: Violation[] = [];
let match = CATCH_RE.exec(content);
while (match !== null) {
const openBraceIdx = match.index + match[0].length - 1;
const errorParam = match[1] ?? match[2] ?? match[3];
const body = readBlock(content, openBraceIdx);
const code = stripComments(body).trim();
// A body that references the caught error identifier (forwarding it to a
// handler, attaching it, etc.) is not "silent" even if it lacks an explicit
// log/throw — avoids false positives like `return handleFetchError(error)`.
const usesError =
errorParam !== undefined && new RegExp(`\\b${errorParam}\\b`).test(code);
const returnOnly = RETURN_ONLY_RE.test(code);
const silent =
!(SURFACING_RE.test(code) || usesError) &&
(code.length === 0 || returnOnly);
if (silent) {
const line = content.slice(0, match.index).split("\n").length;
found.push({
file: filePath,
line,
message:
"Silent catch block. Add log.debug()/log.warn() or re-throw — errors must not vanish (AGENTS.md).",
});
}
match = CATCH_RE.exec(content);
}
return found;
}

export type ScanResult = {
/** Hard violations — always fail CI. */
violations: Violation[];
/** Every silent catch found, across all scanned files. */
silentCatches: Violation[];
};

/** Scan the given files and collect violations and silent catches. */
export async function scanFiles(files: string[]): Promise<ScanResult> {
/** Scan the given files and collect violations. */
export async function scanFiles(files: string[]): Promise<Violation[]> {
const violations: Violation[] = [];
const silentCatches: Violation[] = [];
for (const filePath of files) {
const content = await readFile(filePath, "utf-8");
violations.push(...findContextErrorNewlines(content, filePath));
violations.push(...findAdHocTryPatterns(content, filePath));
silentCatches.push(...findSilentCatches(content, filePath));
}
return { violations, silentCatches };
}

/** Group silent catches into a per-file count map. */
export function countByFile(silentCatches: Violation[]): SilentCatchBaseline {
const counts: SilentCatchBaseline = {};
for (const v of silentCatches) {
counts[v.file] = (counts[v.file] ?? 0) + 1;
}
return counts;
}

export type BaselineDrift = {
/** Files with more silent catches than the baseline allows (or new files). */
regressions: { file: string; baseline: number; actual: number }[];
/** Files with fewer silent catches than the baseline records. */
improvements: { file: string; baseline: number; actual: number }[];
};

/**
* Compare the current per-file silent-catch counts against the committed
* baseline. A regression (new silent catch) always fails CI. An improvement
* (silent catch removed without updating the baseline) also fails so the
* baseline stays honest and can only ratchet down.
*/
export function compareToBaseline(
actual: SilentCatchBaseline,
baseline: SilentCatchBaseline
): BaselineDrift {
const regressions: BaselineDrift["regressions"] = [];
const improvements: BaselineDrift["improvements"] = [];
const files = new Set([...Object.keys(actual), ...Object.keys(baseline)]);
for (const file of files) {
const a = actual[file] ?? 0;
const b = baseline[file] ?? 0;
if (a > b) {
regressions.push({ file, baseline: b, actual: a });
} else if (a < b) {
improvements.push({ file, baseline: b, actual: a });
}
}
regressions.sort((x, y) => x.file.localeCompare(y.file));
improvements.sort((x, y) => x.file.localeCompare(y.file));
return { regressions, improvements };
}

/** Load the committed baseline, treating a missing file as an empty baseline. */
async function loadBaseline(): Promise<SilentCatchBaseline> {
try {
return JSON.parse(await readFile(BASELINE_PATH, "utf-8"));
} catch (error) {
if ((error as NodeJS.ErrnoException).code === "ENOENT") {
return {};
}
throw error;
}
}

/** Serialize the baseline with stable key ordering and a trailing newline. */
function serializeBaseline(counts: SilentCatchBaseline): string {
const sorted: SilentCatchBaseline = {};
for (const key of Object.keys(counts).sort()) {
sorted[key] = counts[key] as number;
}
return `${JSON.stringify(sorted, null, 2)}\n`;
return violations;
}

async function main(): Promise<void> {
const update = process.argv.includes("--update");
const files = await glob("src/**/*.ts");
const { violations, silentCatches } = await scanFiles(files);
const actual = countByFile(silentCatches);

if (update) {
await writeFile(BASELINE_PATH, serializeBaseline(actual));
const total = silentCatches.length;
console.log(
`✓ Wrote silent-catch baseline: ${total} catch(es) across ${Object.keys(actual).length} file(s).`
);
}

const baseline = update ? actual : await loadBaseline();
const { regressions, improvements } = compareToBaseline(actual, baseline);

let failed = false;
const violations = await scanFiles(files);

if (violations.length > 0) {
failed = true;
console.error(
`✗ Found ${violations.length} error class anti-pattern(s):\n`
);
Expand All @@ -486,51 +297,10 @@ async function main(): Promise<void> {
console.error(
"See ContextError JSDoc in src/lib/errors.ts for usage guidance.\n"
);
}

if (regressions.length > 0) {
failed = true;
const added = regressions.reduce((n, r) => n + (r.actual - r.baseline), 0);
console.error(
`✗ ${added} new silent catch block(s) beyond the baseline:\n`
);
for (const r of regressions) {
console.error(` ${r.file}: ${r.baseline} → ${r.actual}`);
}
console.error(
"\nEvery catch must re-throw, log.debug()/log.warn(), or return a fallback " +
"with a log.debug() explaining the suppression (AGENTS.md)."
);
console.error(
"If a silent catch is truly intentional, run `pnpm run check:errors -- --update`.\n"
);
}

if (improvements.length > 0) {
failed = true;
const removed = improvements.reduce(
(n, r) => n + (r.baseline - r.actual),
0
);
console.error(
`✗ ${removed} silent catch block(s) removed but the baseline is stale:\n`
);
for (const r of improvements) {
console.error(` ${r.file}: ${r.baseline} → ${r.actual}`);
}
console.error(
"\nNice — the backlog shrank. Lock it in with `pnpm run check:errors -- --update`.\n"
);
}

if (failed) {
process.exit(1);
}

const total = silentCatches.length;
console.log(
`✓ No error class anti-patterns found (silent-catch baseline: ${total} grandfathered).`
);
console.log("✓ No error class anti-patterns found.");
process.exit(0);
}

Expand Down
Loading
Loading