-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Throw error for invalid currency conversion inputs - #1262
Conversation
The previous version silently returned 0 when centsPerCredit was invalid or credits/amountInCents was NaN/Infinity. This is dangerous in money-conversion paths because misconfigurations would silently produce wrong values rather than failing loudly. Changed to throw explicit errors with descriptive messages so invalid inputs are caught immediately during development and testing.
codebuff-team
commented
Sep 4, 2026
Good instinct — silently returning 0 for a misconfigured centsPerCredit is exactly the kind of bug that's expensive to trace back after the fact, and adding validation here is worth doing.
A few things to fix before this is portable:
-
No tests. You note this yourself in the PR body. For a change to shared money-math utilities (
convertCreditsToUsdCents,convertStripeGrantAmountToCredits), the maintainers will expect unit tests covering the new throw paths (zero, negative, NaN, Infinity) plus the existing happy path, since this is exactly the kind of function where regressions are costly. -
Behavioral change with unaudited blast radius. These functions are almost certainly called from billing/Stripe webhook code and possibly UI paths. Switching from "return 0" to "throw" means any caller that isn't wrapped in a try/catch will now crash instead of silently miscalculating. That might genuinely be the right tradeoff, but it needs to be verified against actual call sites (grep for both function names) — an uncaught exception in a webhook handler could turn a silent bug into a 500 error or a dropped payment, which is arguably worse. At minimum, the PR description should show you traced the callers and confirmed they handle this safely (or should also be updated to add error handling).
-
Minor:
!(centsPerCredit > 0)is a slightly indirect way to also reject NaN — consider!Number.isFinite(centsPerCredit) || centsPerCredit <= 0for symmetry with the other check and clearer intent.
Worth resubmitting with tests and a caller audit — the core idea is sound.
Overview
Fix currency conversion functions in
common/src/util/currency.tsto throw errors instead of silently returning 0 for invalid inputs.Bug Description
The previous version silently returned 0 when:
centsPerCreditwas 0 or negative (division by zero or negative rate)creditsoramountInCentswas NaN or InfinityThis is dangerous in money-conversion paths because misconfigurations would silently produce wrong values rather than failing loudly. A bad Stripe price setup or calculation error would result in users getting 0 credits/cents without any indication that something went wrong.
Fix
Changed to throw explicit errors with descriptive messages so invalid inputs are caught immediately during development and testing:
Testing
No existing tests for these functions, but the fix prevents silent failures in production.
Files Changed
common/src/util/currency.ts- Throw errors for invalid inputsScope
This change only touches
common/which is an approved contribution area per the Contributing Guide.