Skip to content

feat(enrichment): empty-catch / error-swallow analyzer - #3532

Closed
bohdansolovie wants to merge 1 commit into
JSONbored:mainfrom
bohdansolovie:feat/enrichment-error-swallow-2014-v4
Closed

feat(enrichment): empty-catch / error-swallow analyzer#3532
bohdansolovie wants to merge 1 commit into
JSONbored:mainfrom
bohdansolovie:feat/enrichment-error-swallow-2014-v4

Conversation

@bohdansolovie

Copy link
Copy Markdown
Contributor

Summary

Fixes #2014

  • Add ErrorSwallowFinding and a local errorSwallow analyzer that flags newly-added catch/except blocks that swallow errors: empty body, unused binding, or bare return null
  • Brace-balanced catch-body extraction for single-line and multi-line added hunks; JS/TS optional-binding catch {} and Python except: pass supported
  • Binding references use identifier-boundary lookarounds (including $ in valid JS identifiers), not \b word boundaries
  • Register the analyzer in the REES registry with render/docs/metadata; sync src/review/enrichment-analyzer-names.ts and generated .env.example / UI metadata
  • Add review-enrichment/test/error-swallow.test.ts covering empty/return-null/unused-binding cases, log/rethrow/$-binding/nested-brace non-flags, multiline catches, cap, and public-safe brief rendering

Test plan

  • cd review-enrichment && npm run build && npm run metadata && node --test test/error-swallow.test.ts test/analyzer-registry.test.ts
  • CI validate-code

Made with Cursor

Fixes JSONbored#2014

Co-authored-by: Cursor <cursoragent@cursor.com>
@bohdansolovie
bohdansolovie requested a review from JSONbored as a code owner July 5, 2026 15:59
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 5, 2026
@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-05 16:17:53 UTC

10 files · 1 AI reviewer · no blockers · readiness 80/100 · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
Adds a correct, well-structured `errorSwallow` analyzer covering the full wiring surface: new `ErrorSwallowFinding` type, registry descriptor, render hook, `enrichment-analyzer-names.ts`, `analyzer-metadata.json`, UI metadata, and `.env.example`. The brace-balance state machine for multi-line catches is logically sound — the multi-line test correctly traces `startLine` through the patch hunk counter and `flushPending` correctly strips the opening `{` and closing `}` before calling `bodySwallowsError`. Identifier-boundary lookarounds with `[A-Za-z0-9_$]` instead of `\b` correctly handle `$`-prefixed bindings. All checks pass green and the test suite covers single-line, multi-line, Python, cap, cross-file, and public-safe-brief cases.

Nits — 5 non-blocking
  • In `updatePending` and `braceBalanceFrom`, brace counting is purely character-level and doesn't skip string or template-literal contents — a `}` inside a string literal (e.g., `const msg = "closing: }";`) will prematurely decrement `depth` to 0, causing `flushPending` to evaluate an incomplete body, which can produce a false-positive `unused-binding` result for a catch that actually uses or rethrows the error; worth documenting as a known limitation alongside the existing `notes` field in the registry descriptor.
  • In `scanPatchForErrorSwallow`, after `flushPending` produces a finding via `pushFinding(pending.startLine, kind)`, there is no immediate cap check — the `if (findings.length >= maxFindings) return findings` guard appears only at the bottom of the outer loop, one iteration later; the single-line path does an inline `return findings` after the cap check (`error-swallow.ts:~160`), so the multi-line flush path should mirror it to stay consistent and avoid emitting up to `maxFindings + 1` results.
  • Magic numbers `25` and `2000` are inlined in `registry.ts:1087` and `apps/gittensory-ui/src/lib/rees-analyzers.ts:1016-1017`; `MAX_FINDINGS` and `MAX_LINE_CHARS` are private to `error-swallow.ts` and not exported, so any future limit adjustment requires edits in three places.
  • Neither `detectErrorSwallow` nor `parseCompleteCatchLine` skips comment lines — a diff line like `// catch (e) {}` matches `CATCH_OPEN_RE` and would be flagged as `empty-catch`; low practical frequency but worth a single test case and a guard (`line.trimStart().startsWith('//')`).
  • The `AbortSignal` abort path in `scanPatchForErrorSwallow` (the `throw new Error('analyzer_aborted')` branch) has no test coverage — other analyzers in this repo (e.g. `floating-promise`) include an abort-signal test; add one.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #2014
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 318 registered-repo PR(s), 186 merged, 9 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bohdansolovie; Gittensor profile; 318 PR(s), 9 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: bohdansolovie
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, C++, JavaScript
  • Official Gittensor activity: 318 PR(s), 9 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Await review-lane availability.
  • Refresh registry data or choose a registered active repo.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.09%. Comparing base (2c547b4) to head (e8f9973).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3532   +/-   ##
=======================================
  Coverage   93.09%   93.09%           
=======================================
  Files         301      301           
  Lines       31450    31450           
  Branches    11483    11483           
=======================================
  Hits        29279    29279           
  Misses       1517     1517           
  Partials      654      654           
Files with missing lines Coverage Δ
src/review/enrichment-analyzer-names.ts 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge conflicts:

This branch has conflicts that must be resolved
Use the [web editor](https://github.com/JSONbored/gittensory/pull/3532/conflicts) or the command line to resolve conflicts before continuing.

.env.example
apps/gittensory-ui/src/lib/rees-analyzers.ts
review-enrichment/analyzer-metadata.json
review-enrichment/src/analyzers/registry.ts
review-enrichment/src/render.ts
review-enrichment/src/types.ts
review-enrichment/test/analyzer-registry.test.ts
src/review/enrichment-analyzer-names.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

feat(enrichment): empty-catch / error-swallow analyzer

2 participants