fix(limits): handle missing billing limit fields from staging API#306
fix(limits): handle missing billing limit fields from staging API#306ben-fornefeld merged 1 commit intomainfrom
Conversation
The billing API serializes BillingLimit with `omitempty` on its `*int64` pointer fields, so unset limits are absent from the JSON response rather than emitted as explicit nulls. The post-refactor limits page guards with strict `value !== null` checks and then calls `formatNumber(value)` (which calls `toLocaleString`), so an `undefined` field slipped through and crashed the page with "Cannot read properties of undefined". Coerce both fields at the repository boundary so the rest of the app can keep relying on the `number | null` contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview Instead of returning the raw Reviewed by Cursor Bugbot for commit b8393b5. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Pull request overview
This PR hardens the billing limits repository so the dashboard limits page can rely on a strict number | null contract even when the billing API omits unset fields (instead of sending explicit null), preventing a client-side crash when formatting missing values.
Changes:
- Coerce missing
limit_amount_gteandalert_amount_gtefields from the billing API response tonullingetLimits().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — small, type-correct boundary fix that aligns the API response with the existing BillingLimit interface.
Extended reasoning...
Overview
Single-file change to src/core/modules/billing/repository.server.ts: getLimits() now parses the response as Partial<BillingLimit> and coerces missing limit_amount_gte/alert_amount_gte fields to null via ?? null. The BillingLimit interface already declares these as number | null, so this just makes the runtime payload match the typed contract.
Security risks
None. No auth, crypto, or permission paths are touched. The data flow is unchanged — only nullability normalization at the repository boundary. No new untrusted input is parsed or trusted.
Level of scrutiny
Low. This is a defensive 4-line fix for a documented toLocaleString crash introduced when #296 tightened the limits page to assume strict number | null. The Go backend uses *int64 with omitempty, so omitted fields surface as undefined in JS — coercing to null is the right and minimal fix.
Other factors
The downstream consumers (usage-limit-form, usage-alert-form, remove-usage-limit-dialog) all type the cache as BillingLimit | undefined with number | null fields, so this fix removes a real type/runtime mismatch without changing any consumer behavior. No bugs surfaced from the bug hunting system.
Summary
limit_amount_gte/alert_amount_gtefields from the billing API tonullat the repository boundary so the rest of the limits page can rely on the typednumber | nullcontract.Cannot read properties of undefined (reading 'toLocaleString')crash on/dashboard/[teamSlug]/limitsfor any team that has never set a usage limit or alert.Why this happened
The billing API's
BillingLimitstruct uses*int64withjson:",omitempty", so unset limits are omitted from the JSON response entirely, not serialized as explicitnull. The recent limits page refactor (#296) replaced the previousoriginalValue?.toLocaleString()/?? ''pattern with strictvalue !== nullchecks followed byformatNumber(value)— which letsundefinedslip through and throws insidetoLocaleString.I verified two weeks of
e2b-dev/belthistory (#654 sqlc migration, #690 SupabaseDB cleanup, #693 oapi-codegen upgrade); none of them changed theBillingLimitJSON shape. The API has always omitted unset fields — the dashboard refactor is what removed the tolerance forundefined.