Throw error for invalid currency conversion inputs

[pavankumar-vh]

Overview

Fix currency conversion functions in common/src/util/currency.ts to throw errors instead of silently returning 0 for invalid inputs.

Bug Description

The previous version silently returned 0 when:

• centsPerCredit was 0 or negative (division by zero or negative rate)
• credits or amountInCents was NaN or Infinity

This 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:

if (!(centsPerCredit > 0)) {
  throw new Error(
    `convertCreditsToUsdCents: centsPerCredit must be positive, got ${centsPerCredit}`,
  )
}
if (!Number.isFinite(credits)) {
  throw new Error(
    `convertCreditsToUsdCents: credits must be finite, got ${credits}`,
  )
}

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 inputs

Scope

This change only touches common/ which is an approved contribution area per the Contributing Guide.

[codebuff-team]

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:

1. 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.

2. 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).

3. Minor: !(centsPerCredit > 0) is a slightly indirect way to also reject NaN — consider !Number.isFinite(centsPerCredit) || centsPerCredit <= 0 for symmetry with the other check and clearer intent.

Worth resubmitting with tests and a caller audit — the core idea is sound.