Repository navigation
docs: document errorlog footgun and logging util usage (#1286) - #1352
Conversation
document correct usage of shared errorLog/warnLog/infoLog utils, the footgun where errorlog() without an error object degrades to info-level message capture in sentry (losing stack context), and add tracking link to #1286 in adr.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Warning Review limit reached
More reviews will be available in 20 minutes and 41 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 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 |
|
@cubic-dev-ai review |
@LucasSantana-Dev I have started the AI code review. It will take a few minutes to complete. |
|
Failed to generate code suggestions for PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@cubic-dev-ai review — findings addressed |
@LucasSantana-Dev I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 2 files
Auto-approved: Documentation-only changes for logging best practices and a tracking link; no logic, config, or production code modified.
Re-trigger cubic
|
) ## What First concrete delta for **#1286 Track B3** (silent-catch sweep on external/IO call paths): `FeatureToggleService.getDbGlobalOverride` no longer swallows DB errors to `null` without a trace. ```ts } catch (error) { warnLog({ message: 'Failed to read global feature toggle override; falling back to config default', error, data: { name }, }) return null } ``` ## Why The previous `catch { return null }` made a **DB outage indistinguishable from "no override set"** — the caller (`getGlobalToggleStatus`) treats `null` as "fall back to the env/config default", so a persistent database failure silently degraded every global feature toggle to its config value with zero observability. The fail-open-to-config behavior is **correct and intentionally preserved** — this only adds a `warnLog` so the failure is visible. `warn` (not `error`) because the path is gracefully handled. ## Scope note This PR is the *only* genuine finding from a read-only sweep of the four external-API families called out in #1286's next-increment (Spotify, Last.fm, Discord, Prisma). The rest of those paths already log via `errorLog` / `warnLog` / `logAndSwallow` / `logAndWarn` before any swallow — the codebase's existing discipline (#1296/#1302/#1352/#1358) already covers them. The broader "promote empty-catch lint warn→error" item (~2026-06-26) and the Track C requestId issue remain as tracked in #1286. ## Verification - `tsc --noEmit` clean; shared build clean. - Extended the existing `FeatureToggleService.spec.ts` "db throws" test to assert the warning is emitted (`warnLog` called once with the error + `{ name }`). - Shared suite green: **829/829** (61 suites). Refs #1286. @cubic-dev-ai please review. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Log database errors when reading global feature toggle overrides so outages are visible while still falling back to config defaults. Addresses #1286 (Track B3) by surfacing failures on an external/IO path. - **Bug Fixes** - `FeatureToggleService.getDbGlobalOverride` now calls `warnLog({ message, error, data: { name } })` on DB errors, then returns `null`. - Extended unit test to assert the warning is emitted when the DB read throws. <sup>Written for commit 3ffc0b4. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/1411?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->



Item B6 of #1286 (logging-hardening umbrella; does not close it).
docs/logging.md: shared logging utils usage; the footgun —errorLog({ message })withouterrorroutes tocaptureMessageinstead ofcaptureException, silently dropping the exception object and stack; correct/incorrect examples; ADR pointer2026-06-01-logging-observability-hardening.md: tracking link to docs: umbrella tracking issue for logging-hardening Tracks A/B/C status #1286Docs-only diff (2 .md files, +95).
Summary by cubic
Add docs for shared logging utils and clarify a key footgun: calling
errorLog({ message })withouterrortriggers SentrycaptureMessageat error level (notcaptureException), dropping the exception and stack. Includes correct/incorrect examples, when to usewarnLog, structured context tips, and updates the logging ADR with a tracking link to #1286.Written for commit 7023bf5. Summary will update on new commits.