feat: add goleak, guard InitRollbar on empty token#29
Conversation
- Add goleak.VerifyTestMain to catch goroutine leaks in tests - Guard InitRollbar with early return when token is empty, preventing rollbarInited=true with no token configured
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds goleak-based goroutine leak detection to tests, adds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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.
Pull request overview
Adds test-time goroutine leak detection and tightens Rollbar initialization behavior in the notifier package to avoid marking Rollbar as initialized when no token is configured.
Changes:
- Add an early return in
InitRollbar()when the token is empty. - Add a package-level
TestMainusinggoleak.VerifyTestMain(currently for the rooterrorspackage only). - Add
go.uber.org/goleakas a module dependency.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| notifier/notifier.go | Guards Rollbar initialization to avoid enabling Rollbar behavior when token is empty. |
| goleak_test.go | Introduces TestMain to run goleak verification for the errors package tests. |
| go.mod | Adds go.uber.org/goleak dependency for leak detection in tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@notifier/notifier.go`:
- Around line 136-138: When InitRollbar(token, env) receives an empty token it
currently returns early without resetting notifier state; update InitRollbar to
explicitly set rollbarInited = false (and clear any Rollbar client/state if
present) before returning when token == "" so prior successful inits are fully
disabled and the notifier state reflects that Rollbar is off.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 20a78d8d-4225-419a-bb0e-1bc79a8dc451
📒 Files selected for processing (3)
go.modgoleak_test.gonotifier/notifier.go
Address review: set rollbarInited=false before early return so
re-calling InitRollbar("", env) properly disables Rollbar after
a previous init.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
goleak.VerifyTestMainto catch goroutine leaks in testsInitRollbar()with early return when token is empty, preventingrollbarInited=truewith no token configuredTest plan
make testpasses with goleak enabledmake lintcleanSummary by CodeRabbit
Bug Fixes
Chores