Skip to content

Make swallowed errors visible: log or justify every silent catch#195

Merged
willwashburn merged 1 commit into
mainfrom
audit/error-visibility
Jun 10, 2026
Merged

Make swallowed errors visible: log or justify every silent catch#195
willwashburn merged 1 commit into
mainfrom
audit/error-visibility

Conversation

@willwashburn

Copy link
Copy Markdown
Member

Summary

Audit follow-up making every silently-swallowed error in src/main and src/renderer non-test code visible. For each site I did exactly one of:

  • (a) Log — a categorized, context-bearing message in the existing bracket-prefix style ([broker], [cloud-agent], [burn]), console.warn for expected/recoverable, with identifying context (projectId, agent name, op) and never any secret/token; or
  • (b) Justify — a one-line comment explaining why silence is correct (the pattern from DiffPane.tsx handleGenerate, where the store surfaces the error).

Logging only — no control-flow, retry, or error-propagation changes. High-frequency catches (PTY / event / poll loops) are kept silent with a justification per AGENTS.md, and the polled burn.getFingerprint log is time-throttled (60s) so telemetry stays low-noise.

Verify: npm test → 115 pass · npx vitest run → 263 pass (19 files). All green. (The repo's tsconfig.node.json has 171 pre-existing tsc errors on clean main, in files this PR does not touch; my changes add none.)

Sites → action

src/main/broker.ts

Site Action
attachCloudSandbox previous cloud-session shutdown (~1827, audit priority) Logged [broker] — failed shutdown can leak the previous cloud sandbox
shutdown() per-session shutdown (~4073) Logged [broker] — leaked sandbox on teardown
detachCloudSandbox shutdown (~4125) Logged [broker] — leaked detached sandbox
void handleSpawnedChildLineage fire-and-forget (~2391, audit priority) Logged on rejection [broker] — lost parent/child attribution
sibling listAgents() during spawn dedupe (~2722) Logged [broker] — missing names risk a relay name collision
workspaceKeyForProject start-gate .catch (~1771) Justified — gate only; owner surfaces start failure
attachCloudSandbox in-flight-attach gate (~1813) Justified — gate only
getOrAwaitSessionsForProject revive/start/attach gates (~1995–2003) Justified — gates; !sessions.length turns a real failure into a throw
locateSessionForAgent 250ms listAgents() probe (~2981) Justified — tight poll; registered:false surfaces outcome; logging would flood
waitForAgentIdle getStatus() poll (~3250) Justified — tight poll; logging would flood
getPending().catch(()=>0) pending count ×2 (~3338, ~3483) Justified — non-critical UI hint; snapshot path surfaces real failures
getInboundDeliveryMode().catch per-agent enrichment (~3936) Justified — best-effort badge on every poll
parsePortFromUrl / readBrokerConnectionFileInfo parse catches (~1013, ~1075) Justified — return meaningful sentinels that propagate
broker-compat shim process.kill catch (~414) Justified — has fallback exit(1); nothing to log inside the shim

src/main/cloud-agent.ts

Site Action
deleteBox().catch(()=>undefined) on attach failure (~891) Logged [cloud-agent] — leaked sandbox
deleteBox() on attach cancel (~922) Logged [cloud-agent]
deleteBox() on warm cancel ×2 (~1105, ~1132) Logged [cloud-agent]
normalizeHttpsGitRemote URL parse catch (~428) Justifiednull is the documented "not a usable remote" sentinel
void emitMountStatus per mount event (~1273) Justified — internally swallows mount.status(), emit is sync; never rejects
void maybePullAfterRun timer (~1412) Justified — wraps git work in try/catch that emits errors

(Note: deleteBox itself throws on failure but does not log; the .catch(()=>undefined) callers were the only visibility for box-leak failures.)

src/main/burn.ts

Site Action
getFingerprint catch (~823) Logged [burn], time-throttled 60s (polled ~15s by burn views)
listProjectAgentSummaries batch-summary fallback (~956) Logged [burn] — fast path consistently failing stays visible
void ingestRecent() ×2 (warmUp, refreshLedger) (~1099, ~1113) JustifiedingestRecent self-catches and once-logs via warnedUnavailable

src/renderer

Site Action
terminal-runtime-registry.ts xterm addon/term dispose+refresh // ignore ×5 Justified — disposing/refreshing an already-disposed/detached term throws benignly
use-terminal.ts visibility-change fit/refresh // ignore Justified — self-corrects on next fit
git-store.ts per-root git.status/summary fallbacks Justified — one bad root can't blank the multi-root sweep; main logs the git error
issues-store.ts readRemoteFile().catch(()=>null) Justified — best-effort preview enrichment
ProjectSidebar.tsx authRecoveryState() probes ×2 Justified — best-effort banner; event listener sets it when needed
ProjectSettings.tsx mounted/remote Slack channel fallbacks ×2 Justified — secondary/last-resort fallbacks; primary error already logged
CloudAgentPicker.tsx void cancelPrewarm().catch Justified — superseded prewarm; box self-expires; main logs real failures
BrokerDetailsPage.tsx listEvents().catch(()=>undefined) Justified — best-effort backfill; page renders from listDetails
DiffPane.tsx handleGenerate, BurnProjectPage.tsx, ConversationsPanel.tsx, ChatView.tsx, DiffViewer.tsx Already addressed — pre-existing justifications or meaningful fallbacks (no change)

Deferred: src/main/integrations.ts

Not touched (edited in parallel on another branch). Candidate silent swallows for a follow-up:

  • removeProjectIntegrationsLink(...).catch(() => undefined) (~1211) — silent link-cleanup failure.
  • handle.refreshToken().catch(() => undefined) (~1522) — silent token-refresh failure; worth a [integrations] warn since it feeds auth-recovery UX. (Same pattern in relay-workspace.ts:79.)
  • response.json().catch(() => null) (~1439) — benign parse fallback; justify.
  • Bare void fire-and-forgets with no .catch: syncLocalMounts (~916, ~1188), hydrateProjectCloudIntegrations (~981), pollConnect (~1837), safeInjectSystemMessage (~2171) — verify each self-handles; add a rejection log where it doesn't. (syncAgentState ~1957 and syncLocalIntegrationState ~1978 already .catch-and-emit.)

🤖 Generated with Claude Code

Audit follow-up: every silently-swallowed error in src/main and
src/renderer non-test code now either emits a categorized,
context-bearing log (bracket-prefix style per AGENTS.md) or carries a
one-line comment justifying why silence is correct. No control-flow,
retry, or error-propagation behavior was changed — logging only.

High-frequency catches (PTY/event/poll loops) are kept silent with a
justification, and the polled burn fingerprint log is time-throttled so
the telemetry stays low-noise.

src/main/integrations.ts is intentionally untouched (edited in parallel
on another branch); its findings are deferred to a follow-up in the PR.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@codeant-ai

codeant-ai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@willwashburn, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 31 minutes and 39 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 40aeedc8-9b26-4e34-b193-1cc41e26b76e

📥 Commits

Reviewing files that changed from the base of the PR and between 599b033 and f2b01ef.

📒 Files selected for processing (11)
  • src/main/broker.ts
  • src/main/burn.ts
  • src/main/cloud-agent.ts
  • src/renderer/src/components/agents/CloudAgentPicker.tsx
  • src/renderer/src/components/broker/BrokerDetailsPage.tsx
  • src/renderer/src/components/settings/ProjectSettings.tsx
  • src/renderer/src/components/sidebar/ProjectSidebar.tsx
  • src/renderer/src/hooks/use-terminal.ts
  • src/renderer/src/lib/terminal-runtime-registry.ts
  • src/renderer/src/stores/git-store.ts
  • src/renderer/src/stores/issues-store.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch audit/error-visibility

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@willwashburn willwashburn merged commit afc548b into main Jun 10, 2026
4 checks passed
@willwashburn willwashburn deleted the audit/error-visibility branch June 10, 2026 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant