fix(sentry): suppress recoverable errors and fix grooming-error stringification#919
fix(sentry): suppress recoverable errors and fix grooming-error stringification#919pedramamini merged 1 commit intorcfrom
Conversation
…gification Cleans up four sources of high-volume Sentry noise observed on the rc channel: - git.ts scanWorktreeDirectory / watchWorktreeDirectory now skip captureException for ENOENT, which is expected when a configured worktree-parent path has been moved or deleted (Fixes MAESTRO-J5, MAESTRO-J6, MAESTRO-J7). - cue-db.ts safeRecordCueEvent / safeUpdateCueEventStatus early-return when the DB handle is null, instead of throwing through a captured catch. This race fires during shutdown and pre-init and is already non-fatal (Fixes MAESTRO-JX). - path-prober.ts getExpandedEnvWithShell skips captureException for the known recoverable shell PATH probe failures (timeout, non-zero shell exit). Callers already fall back to the base env (Fixes MAESTRO-J8). - context-groomer.ts onError now extracts .message from AgentError plain objects rather than relying on String(error), which was producing "Grooming error: [object Object]" in Sentry (Fixes MAESTRO-JB, MAESTRO-K7).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR refines error handling and observability across four modules by conditionally reporting errors to Sentry. Expected errors—such as shell timeouts, non-zero exit codes, missing files (ENOENT), and database initialization gaps—are now logged but excluded from Sentry reporting, reducing noise while maintaining observability. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR suppresses high-volume Sentry noise from four known-recoverable paths: ENOENT in worktree directory scanning/watching, null-DB early returns in Cue safe wrappers, expected shell PATH probe failures, and fixes Confidence Score: 4/5Safe to merge — all changes are scoped to error-reporting paths and leave runtime behavior unchanged. No P0 or P1 issues found. The string-based heuristic in path-prober.ts for classifying expected errors is slightly brittle if upstream error messages ever change, keeping this at 4 rather than 5. src/main/agents/path-prober.ts — the expected-error detection relies on substring matching against error message text. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Error caught] --> B{path-prober: timeout / non-zero exit?}
B -->|Yes| C[Skip Sentry, log debug]
B -->|No| D[captureException]
E[Error caught in git.ts] --> F{code === ENOENT?}
F -->|Yes| G[Log only, skip Sentry]
F -->|No| H[captureException]
I[safeRecordCueEvent / safeUpdateCueEventStatus called] --> J{db === null?}
J -->|Yes| K[log warn, return early — skip Sentry]
J -->|No| L[Try DB write]
L -->|throws| M[log warn + captureException]
N[agent-error event] --> O{error instanceof Error?}
O -->|Yes| P[use error.message]
O -->|No| Q{typeof error === 'object' and .message present?}
Q -->|Yes| R["String(error.message)"]
Q -->|No| S["String(error)"]
P & R & S --> T[reject with Grooming error message]
Reviews (1): Last reviewed commit: "fix(sentry): suppress recoverable errors..." | Re-trigger Greptile |
1 participant
Summary
Cleans up four sources of high-volume Sentry noise observed on the rc channel where the bug and fix were obvious:
git.ts):scanWorktreeDirectoryandwatchWorktreeDirectorynow skipcaptureExceptionwhen the parent path doesn't exist. ENOENT is expected when the user moves/deletes a worktree directory — the renderer already handles the failure gracefully. FixesMAESTRO-J5,MAESTRO-J6,MAESTRO-J7(J7 alone was at 273 events/escalating).cue-db.ts):safeRecordCueEvent/safeUpdateCueEventStatusnow early-return when the DB handle is null instead of throwing through a captured catch. Both wrappers are documented as non-fatal. FixesMAESTRO-JX.path-prober.ts):getExpandedEnvWithShellskips Sentry capture for the known recoverable cases (timeout / non-zero shell exit). Callers already fall back to the base expanded env. FixesMAESTRO-J8.context-groomer.ts):agent-errorevents emit anAgentErrorplain object, soString(error)was producing the literal text[object Object]in Sentry titles. Now extracts.messagewhen the value is object-shaped. FixesMAESTRO-JB,MAESTRO-K7.All four changes are scoped — they only suppress reporting / fix message formatting for known-recoverable paths; runtime behavior is unchanged. All issues above were tagged
channel: rcin Sentry and verified against the current code.Test plan
npm run lint(TypeScript multi-config) — cleannpm run lint:eslinton touched files — cleanvitest runforcue-db,cue-db-integration,cue-run-manager,context-groomer,gitipc handler,path-prober.shell-path— all pass (302 tests)Summary by CodeRabbit