Skip to content

feat(rees): add error-swallow analyzer for empty catch blocks (#2014) - #3516

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

feat(rees): add error-swallow analyzer for empty catch blocks (#2014)#3516
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:feat/enrichment-error-swallow-2014

Conversation

@RealDiligent

Copy link
Copy Markdown
Contributor

Summary

  • Add REES errorSwallow local analyzer detecting empty/unused catch blocks and return-null error swallow patterns in JS/TS and Python patches.
  • Registers analyzer in metadata, UI catalog, and enrichment prompt rendering.

Closes #2014
Part of #1499

Test plan

  • review-enrichment: npm run build && npm run metadata:check
  • node --test test/error-swallow.test.ts test/analyzer-registry.test.ts (10/10 pass)
  • Root npm run typecheck

…red#2014)

Detect JS/TS empty catches, unused bindings, and return-null swallow patterns
in added patch lines. Part of JSONbored#1499.

Co-authored-by: Cursor <cursoragent@cursor.com>
@RealDiligent
RealDiligent requested a review from JSONbored as a code owner July 5, 2026 15:17
@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:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 5, 2026
@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-07-05 15:19:54 UTC

10 files · 1 AI reviewer · 1 blocker · readiness 80/100 · CI pending · blocked

🛑 Suggested Action - Reject/Close

  • AI reviewers agree on a likely critical defect: review-enrichment/src/analyzers/error-swallow.ts:51 only matches JS/TS catches when the opening catch, body, and closing brace are on the same added line, so a normal added block like `catch (err) {` followed by `}` is completely missed
  • change the scanner to collect the added catch body across following added lines before calling `bodySwallowsError`, or narrow the analyzer docs/tests to same-line catches only. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
This adds the requested analyzer wiring and metadata consistently, and the issue link is present. The implementation is too line-local for the advertised analyzer: it misses normal multiline JS/TS catch blocks and can report wrong line numbers once a hunk has context lines before the added catch, so the user-facing findings are not reliable enough yet.

Blockers

  • review-enrichment/src/analyzers/error-swallow.ts:51 only matches JS/TS catches when the opening catch, body, and closing brace are on the same added line, so a normal added block like `catch (err) {` followed by `}` is completely missed; change the scanner to collect the added catch body across following added lines before calling `bodySwallowsError`, or narrow the analyzer docs/tests to same-line catches only.
  • review-enrichment/src/analyzers/error-swallow.ts:87 reports stale line numbers because `newLine` only advances on added lines; in a normal unified hunk with context before the added catch, the finding points at the hunk start instead of the real target line, so advance `newLine` for context lines as well and skip only deletion/file-header lines without incrementing.
Nits — 5 non-blocking
  • nit: review-enrichment/src/analyzers/error-swallow.ts:101 uses `lines.slice(index + 1).find(...)`, which can pick a later unrelated added Python line instead of the immediate except body; make it inspect the next patch line only, or explicitly require adjacency/indentation.
  • nit: review-enrichment/src/analyzers/error-swallow.ts:37 treats any body with no binding as `empty-catch`, so `catch { cleanup(); }` is reported as empty even though it has executable code; use a distinct kind or skip unbound non-empty bodies if the policy is only empty/unused/return-null.
  • nit: review-enrichment/test/error-swallow.test.ts should add real unified-hunk coverage with context lines before the added catch and multiline JS/TS catch bodies, because the current tests only exercise the easiest same-line path.
  • Add scanner tests for `@​@​ -10,3 +10,4 @​@​` hunks where context lines precede the added catch and assert the reported new-file line.
  • Add multiline JS/TS fixtures for empty catch, unused binding, logged binding, and rethrow so the analyzer covers the formatting developers normally write.

Why this is blocked

  • review-enrichment/src/analyzers/error-swallow.ts:51 only matches JS/TS catches when the opening catch, body, and closing brace are on the same added line, so a normal added block like `catch (err) {` followed by `}` is completely missed; change the scanner to collect the added catch body across following added lines before calling `bodySwallowsError`, or narrow the analyzer docs/tests to same-line catches only.
Signal Result Evidence
Code review ❌ 1 blocker 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: 163 registered-repo PR(s), 28 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor RealDiligent; Gittensor profile; 163 PR(s), 0 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: RealDiligent
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 163 PR(s), 0 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

@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Gittensory is closing this pull request on the maintainer's behalf (AI reviewers agree on a likely critical defect: review-enrichment/src/analyzers/error-swallow.ts:51 only matches JS/TS catches when the opening catch, body, and closing brace are on the same added line, so a normal added block like `catch (err) {` followed by `}` is completely missed; change the scanner to collect the added catch body across following added lines before calling `bodySwallowsError`, or narrow the analyzer docs/tests to same-line catches only.). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 5, 2026
@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 (9a125e9) to head (3bb4842).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3516   +/-   ##
=======================================
  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.

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

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant