proposal: log telemetry initialization failures#662
proposal: log telemetry initialization failures#662dev-punia-altimate wants to merge 1 commit intomainfrom
Conversation
Category: error-swallowing Severity: medium Repo: altimate-code Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
📝 WalkthroughWalkthroughTelemetry initialization error handling was modified to log warnings instead of silently failing. The catch block now emits a warning message with the error details while maintaining non-fatal error behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/opencode/src/index.ts (1)
285-289: Consider logging shutdown failures for consistency.The PR improves diagnostics by logging telemetry init failures, but shutdown failures remain silently swallowed. While the comment explains the intent, logging a warning here would maintain consistency and aid in diagnosing telemetry issues without affecting the shutdown flow.
♻️ Suggested change
try { await Telemetry.shutdown() - } catch { + } catch (e) { + Log.Default.warn("Telemetry shutdown failed", { error: String(e) }) // Telemetry failure must never prevent shutdown }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/index.ts` around lines 285 - 289, The catch block swallowing errors from Telemetry.shutdown() should capture the thrown error and log a non-fatal warning for diagnostics; change the empty catch to catch (err) and emit a warning (e.g., console.warn('Telemetry.shutdown failed:', err)) so shutdown still continues but failures are recorded; reference Telemetry.shutdown() and ensure the error is not rethrown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/opencode/src/index.ts`:
- Around line 285-289: The catch block swallowing errors from
Telemetry.shutdown() should capture the thrown error and log a non-fatal warning
for diagnostics; change the empty catch to catch (err) and emit a warning (e.g.,
console.warn('Telemetry.shutdown failed:', err)) so shutdown still continues but
failures are recorded; reference Telemetry.shutdown() and ensure the error is
not rethrown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f662074b-7ab2-4295-94f6-a2f5bf10f6ed
📒 Files selected for processing (1)
packages/opencode/src/index.ts
Proposal
Repo: altimate-code
Category: error-swallowing
Severity: medium
Files:
packages/opencode/src/index.tsWhat I Found
At CLI startup (line 126),
Telemetry.init().catch(() => {})silently swallows all telemetry initialization failures. This means:Logutility is already imported and used throughout the fileFix
Replaced
.catch(() => {})with.catch((e) => Log.Default.warn("Telemetry initialization failed", { error: String(e) }))using the existingLogutility that's already imported in this file.Auto-generated by QA Autopilot Proposal Monitor. Open for 30-day human review.
Summary by cubic
Adds a warning log when telemetry initialization fails at CLI startup. Prevents silent loss of events and makes telemetry outages visible.
Telemetry.init().catch(() => {})withTelemetry.init().catch((e) => Log.Default.warn("Telemetry initialization failed", { error: String(e) })).Written for commit 2459d8c. Summary will update on new commits.
Summary by CodeRabbit