fix(desktop): log the real error in the app error boundary#1427
Conversation
componentDidCatch swallowed the error entirely, so a generic 'Something went wrong' crash left no trace in the console or for the observability stack, making it undiagnosable. Log the error and component stack for the generic mode (the expected waiting/chunk states stay quiet). No UI change.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesAppErrorBoundary conditional error logging
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 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 |
|
Note Your trial team has used its Gitar budget, so automatic reviews are paused. Upgrade now to unlock full capacity. Comment "Gitar review" to trigger a review manually. Code Review ✅ ApprovedUpdates OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Important Your trial ends in 3 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more. Was this helpful? React with 👍 / 👎 | Gitar |
| // is diagnosable from the console and reachable by the observability stack. | ||
| // Skip the "waiting"/"chunk" modes -- those are expected, handled states. | ||
| if (this.state.mode === "generic") { | ||
| console.error("[AppErrorBoundary]", error, info?.componentStack ?? ""); |
There was a problem hiding this comment.
WARNING: Defensive info?.componentStack ?? "" is misleading
ErrorInfo from react types componentStack as a required non-optional string (see @types/react ErrorInfo). React never invokes componentDidCatch with a nullish info or with componentStack missing. The optional-chaining + nullish coalesce suggest these are realistic failure modes when they aren't, and they obscure the real contract from future readers. Drop the noise:
| console.error("[AppErrorBoundary]", error, info?.componentStack ?? ""); | |
| console.error("[AppErrorBoundary]", error, info.componentStack); |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
The change correctly fixes the root cause: a The test additions are well-scoped: one positive case proves the real error reaches the console with the right prefix, and one negative case proves the handled One small wart on the new logging line, captured inline. Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Reviewed by minimax-m3 · Input: 33.4K · Output: 2.8K · Cached: 136.1K |
…dger failures (#1431) Folds two gitar findings on the consent-flow backend: - #1430 Edge-Case (must-fix): request_app_consent did not de-duplicate input capabilities, so a repeated cap became a colliding consent option whose value the #1428 dedup suffixer rewrote to a non-capability (e.g. 'app.net (2)'). Now de-duped while preserving order. - #1429 Quality: _apply_app_grant swallowed all app_grants ledger errors silently; now logs a warning with the app + decision id so a broken grant write is diagnosable (same lesson as #1427).
Problem
The top-level AppErrorBoundary's
componentDidCatch()took no arguments and logged nothing. When an unexpected error hit the generic "Something went wrong" fallback, the actual error and component stack were discarded, so the crash left no console trace and nothing for the observability stack to pick up. (Surfaced while debugging a live 'Something went wrong' crash on the Pi that was undiagnosable for exactly this reason.)Fix
Log
error+info.componentStackincomponentDidCatchfor thegenericmode only. The expected, handled states (waitingfor backend reconnect,chunkfor stale-deploy reload) stay quiet to avoid noise. No UI/behavior change.Tests
Added two cases: a generic crash logs the real error + stack; the waiting state does not log. Both AppErrorBoundary suites green (13 passed).
Summary by CodeRabbit