Repository navigation
chore(eslint): warn on empty catch blocks (#1286 track b3) - #1358
Conversation
|
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.
|
Warning Review limit reached
More reviews will be available in 35 minutes and 3 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. ✨ 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 |
|
Failed to generate code suggestions for PR |
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 |
@LucasSantana-Dev I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Fixed — test-config.js was a leftover scratch file from validating the rule, removed. The PR is now the intended one-line eslint.config.js change. |
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.
) ## 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 1 of #1286 (Track B3 soft-launch; does not close the umbrella).
Adds
"no-empty": ["warn", { allowEmptyCatch: false }]to the shared rules block — one line, no config restructuring. Warnings don't fail CI.Scope notes:
no-empty: offand a--max-warnings 0gate, so any warning would break its CI — documented exclusion.Summary by cubic
Warn on empty catch blocks to prevent swallowed errors, as part of Linear 1286 (Track B3 soft-launch). Adds ESLint
no-emptyas a warning with{ allowEmptyCatch: false }; warnings don't fail CI.eslint.config.js.packages/backend;packages/frontendexcluded;packages/botandpackages/sharedstill unlinted.test-config.jsscratch file.Written for commit d2a53b8. Summary will update on new commits.