feat(billing): sell DOS Plus/Pro via shared dos.me billing - #38
Conversation
Crove is first-party DOS, so checkout must grant the same user_plans row used by app.dos.ai. Co-authored-by: Cursor <[email protected]>
There was a problem hiding this comment.
Code Review
This pull request introduces a shared billing integration with DOS, adding a client and service to synchronize organization subscriptions, handle checkouts, and manage portals, while updating the backend controllers and frontend components to support DOS Plus and Pro plans. The review feedback highlights critical performance and stability concerns: synchronously calling syncOrg on high-frequency endpoints like /user/self creates a major bottleneck, external HTTP requests lack timeouts which could block NestJS threads, missing date validation on current_period_end could cause database crashes, and error handling in rethrowDosMe may discard descriptive error messages.
| async syncOrg( | ||
| user: User, | ||
| organizationId: string | ||
| ): Promise<CrovePlanMapping> { | ||
| const dosUserId = this.dosUserId(user); | ||
| if (!dosUserId) { | ||
| this.logger.warn( | ||
| `DOS shared billing is on but user ${user.id} has no DOS UUID providerId` | ||
| ); | ||
| return mapDosPlanToCrove('free'); | ||
| } | ||
|
|
||
| const entitlement = await this.client.getEntitlement(dosUserId); |
There was a problem hiding this comment.
Synchronously calling syncOrg on every /user/self request (which occurs on almost every page load) introduces a major performance bottleneck and scalability issue. Since syncOrg performs an external HTTP request to api.dos.me and a database write (upsert), this will cause significant latency and can easily overload both the external API and the database. Consider caching the entitlement mapping in Redis with a short TTL (e.g., 5 minutes) or using a webhook-based sync mechanism to avoid synchronous external calls on high-frequency endpoints.
| const cancelAt = entitlement.current_period_end | ||
| ? new Date(entitlement.current_period_end) | ||
| : null; |
There was a problem hiding this comment.
If entitlement.current_period_end is an invalid date string, new Date() will return an Invalid Date object. Attempting to save an Invalid Date to a PostgreSQL timestamp column via Prisma will throw a database error and crash the request. Since syncOrg is called during user authentication/self-retrieval, this would completely block the user from accessing the application. Please validate that the parsed date is valid before saving it.
let cancelAt: Date | null = null;
if (entitlement.current_period_end) {
const parsedDate = new Date(entitlement.current_period_end);
if (!isNaN(parsedDate.getTime())) {
cancelAt = parsedDate;
} else {
this.logger.error('Invalid current_period_end date format: ' + entitlement.current_period_end);
}
}| const response = await fetch(url, { | ||
| ...init, | ||
| headers: { | ||
| Accept: 'application/json', | ||
| 'Content-Type': 'application/json', | ||
| 'X-API-Key': this.apiKey(), | ||
| ...(init.headers || {}), | ||
| }, | ||
| }); |
There was a problem hiding this comment.
The external HTTP request to api.dos.me is performed without any timeout. If the external billing API is slow or unresponsive, this request will hang indefinitely, blocking the NestJS request thread. Since this is called synchronously during user session retrieval (getSelf), it can easily lead to a complete denial of service for the user. Please add a reasonable timeout (e.g., 5 seconds) using AbortSignal.timeout().
| const response = await fetch(url, { | |
| ...init, | |
| headers: { | |
| Accept: 'application/json', | |
| 'Content-Type': 'application/json', | |
| 'X-API-Key': this.apiKey(), | |
| ...(init.headers || {}), | |
| }, | |
| }); | |
| const response = await fetch(url, { | |
| ...init, | |
| signal: AbortSignal.timeout(5000), | |
| headers: { | |
| Accept: 'application/json', | |
| 'Content-Type': 'application/json', | |
| 'X-API-Key': this.apiKey(), | |
| ...(init.headers || {}), | |
| }, | |
| }); |
| private rethrowDosMe(err: unknown): never { | ||
| const status = (err as { status?: number })?.status; | ||
| const message = err instanceof Error ? err.message : 'DOS billing failed'; | ||
| throw new HttpException(message, status && status >= 400 ? status : 502); | ||
| } |
There was a problem hiding this comment.
In rethrowDosMe, if err is not an instance of Error but is an object with a message property (which is common for custom error objects or API responses), err instanceof Error will evaluate to false, and the actual error message will be lost and replaced with the generic 'DOS billing failed'. Consider checking for err?.message as a fallback to preserve the original error message.
| private rethrowDosMe(err: unknown): never { | |
| const status = (err as { status?: number })?.status; | |
| const message = err instanceof Error ? err.message : 'DOS billing failed'; | |
| throw new HttpException(message, status && status >= 400 ? status : 502); | |
| } | |
| private rethrowDosMe(err: unknown): never { | |
| const status = (err as { status?: number })?.status; | |
| const message = err instanceof Error ? err.message : (err as any)?.message || 'DOS billing failed'; | |
| throw new HttpException(message, status && status >= 400 ? status : 502); | |
| } |
Resolve conflicts keeping both intents: DOS shared billing gates run first and fall back to the PaymentService multi-provider flow (replacing the legacy StripeService calls the branch was written against). api.module keeps StripeController (dev) + DOS billing providers (branch).
Summary
POST /internal/billing/checkouton api.dos.me) and entitlements are read from the sameuser_plansrow used by app.dos.ai (GET /internal/users/{id}/plan).DOS_SHARED_BILLING=true— merging this changes no behavior until the env flag +DOS_ME_INTERNAL_API_KEYare provisioned on a deployment.dos-plan.map.spec.tsunit tests.Deployment notes
DOS_SHARED_BILLING=true,DOS_ME_API_URL=https://api.dos.me,DOS_ME_INTERNAL_API_KEY=<from DOS-Me, min 32 chars>(not yet provisioned on Beta/Prod).Merge conflicts vs dev: none (verified via merge-tree).