Skip to content

feat(enrichment): add error-swallow catch analyzer - #3361

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

feat(enrichment): add error-swallow catch analyzer#3361
jaso0n0818 wants to merge 1 commit into
JSONbored:mainfrom
jaso0n0818:feat/error-swallow-analyzer-2014

Conversation

@jaso0n0818

Copy link
Copy Markdown
Contributor

Closes #2014

What

A new local REES analyzer, errorSwallow, that flags newly-added catch blocks that swallow the error — an empty body, a body that just returns null/undefined, or a body that neither rethrows, logs, nor references the caught binding. A top source of silent failures the headless reviewer often misses. Pure compute over added diff lines, no network.

Detection (single-line, precision-first)

  • Scoped to JS/TS (a catch block) and Python (except …: pass). Non-source and other extensions are skipped.
  • Single-line by design (the catch/except and its body on one added line — the compact form the pattern targets, following actions-pin.ts): a body spread across multiple lines is not tracked — the safe, false-negative direction, with no cross-line state.
  • Three kinds, matching the issue's definition exactly:
    • empty-catch — an empty body (a comment-only body counts, since it swallows too).
    • return-null — the body just return null/return undefined.
    • unused-binding — a body that has a binding but neither rethrows (throw), logs (console.*/logger.*/log(/…), nor references the caught binding. A bindingless catch { … } is never an unused-binding.
  • String/comment blanking first (via secret-log.ts's codeOnly plus same-line comment strip), so a catch {} inside a string literal or a // } catch (e) {} note is not matched. Added lines only, line-cited via hunk headers, with the shared \ No newline line-counter fix; findings capped (maxFindings: 25) per file and globally.

Registration

Registered as a local descriptor (category quality, cost local, requires ["files"]) with an inline render(), following the redos/actions-pin descriptor shape. All wiring updated: types.ts (ErrorSwallowFinding + errorSwallow? key), render.ts, analyzer-registry.test.ts, root src/review/enrichment-analyzer-names.ts (canonical REES_ANALYZER_NAMES), root test/unit/enrichment-wire.test.ts, and the generated analyzer-metadata.json / rees-analyzers.ts / .env.example via node scripts/generate-analyzer-metadata.mjs.

Tests

review-enrichment/test/error-swallow.test.ts (12 tests) covers: empty catch flagged (with/without binding, comment-only body), return-null, unused-binding (ignored binding), a catch that rethrows/logs/references the binding NOT flagged, a bindingless catch with real work not flagged, catch inside a string/comment not matched, Python except: pass flagged vs a handling except not, non-JS/Py files skipped, added-line scanning with exact locations, added-lines-only with line-number accuracy across mixed hunks, the per-file cap + maxFindings: 0, the entrypoint's global cap across files, and the no-files case. Analyzer metadata is regenerated and committed.

@jaso0n0818
jaso0n0818 requested a review from JSONbored as a code owner July 5, 2026 04:56
@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 05:26:41 UTC

11 files · 1 AI reviewer · 1 blocker · readiness 62/100 · CI green · clean

🛑 Suggested Action - Reject/Close

  • AI reviewers agree on a likely critical defect: review-enrichment/src/analyzers/error-swallow.ts:21 only matches `except ...: pass` when `pass` is the physical end of the line, so `except ValueError: pass # ignore` is a reachable single-line swallow that the Python branch fails to report despite the analyzer contract saying comments are blanked first. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
The new analyzer is wired through the registry, render path, metadata, UI docs, and canonical analyzer-name list, and the JS single-line cases are covered with focused tests. I would not block the registration wiring, but the Python detector does not honor the advertised comment blanking behavior, so a common `except ...: pass # reason` swallow slips through even though it is still a single-line `pass` body. That is a real analyzer correctness miss in the newly-added path, not just a precision tradeoff.

Blockers

  • review-enrichment/src/analyzers/error-swallow.ts:21 only matches `except ...: pass` when `pass` is the physical end of the line, so `except ValueError: pass # ignore` is a reachable single-line swallow that the Python branch fails to report despite the analyzer contract saying comments are blanked first.
Nits — 5 non-blocking
  • nit: review-enrichment/src/analyzers/error-swallow.ts:18 uses `[^}]*` for the catch body, so `catch (e) { cleanup({ ok: true }); }` is parsed only up to the object literal and may produce avoidable false positives/negatives; document that nested braces are intentionally out of scope or add a focused test.
  • nit: review-enrichment/test/error-swallow.test.ts does not cover a Python `except ...: pass` line with a trailing comment, which is the missing branch in the implementation.
  • Change the Python branch in `detectErrorSwallow` to strip same-line `#` comments before applying `PY_EXCEPT_PASS_RE`, and add a regression test for `except ValueError: pass # ignored`.
  • Add one JS regression test for a catch body containing an object literal so the single-line brace limitation is explicit rather than accidental.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.

Why this is blocked

  • review-enrichment/src/analyzers/error-swallow.ts:21 only matches `except ...: pass` when `pass` is the physical end of the line, so `except ValueError: pass # ignore` is a reachable single-line swallow that the Python branch fails to report despite the analyzer contract saying comments are blanked first.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ Linked #2014
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High 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: 444 registered-repo PR(s), 267 merged, 7 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jaso0n0818; Gittensor profile; 444 PR(s), 7 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: jaso0n0818
  • 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: 444 PR(s), 7 issue(s).
  • Related work: Titles/paths share 5 meaningful terms. (issue #1514, issue #1477)
  • Related work: Titles/paths share 5 meaningful terms. (issue #1514, issue #2025)
  • Related work: Titles/paths share 7 meaningful terms. (issue #2017, issue #2025)
  • Additional title-only matches omitted; title-only overlap does not block.
Contributor next steps
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Await review-lane availability.
  • Refresh registry data or choose a registered active repo.
  • Check active issues and PRs before submitting.
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.88%. Comparing base (d638678) to head (aad1d2c).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3361   +/-   ##
=======================================
  Coverage   93.88%   93.88%           
=======================================
  Files         281      281           
  Lines       30566    30566           
  Branches    11134    11134           
=======================================
  Hits        28698    28698           
  Misses       1211     1211           
  Partials      657      657           
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.

@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:21 only matches `except ...: pass` when `pass` is the physical end of the line, so `except ValueError: pass # ignore` is a reachable single-line swallow that the Python branch fails to report despite the analyzer contract saying comments are blanked first.). 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.

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