refactor(subscription): update subscription status endpoint to proxy requests to Recoup API#1729
Conversation
…requests to Recoup API - Enhanced the GET /api/subscription/status endpoint to proxy requests to the Recoup API, allowing for streamlined access to subscription status. - Improved error handling and response formatting using NextResponse. - Updated the fetch logic in the useProStatus hook to include authorization headers and handle access tokens via Privy authentication. - Ensured that the query is only executed when the user is authenticated and has a valid account ID.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe GET /api/subscription/status route was removed and the pro-status flow was changed to require a runtime bearer token. The hook now uses Privy to obtain an access token and calls the proxied subscription status endpoint with Authorization and accountId. ChangesSubscription Status Proxy + Hook Auth
Sequence DiagramsequenceDiagram
actor User
participant React as React Component
participant Hook as useProStatus
participant Privy as usePrivy
participant Route as GET /api/subscription/status
participant ExtAPI as External API
User->>React: Mounts component (authenticated)
React->>Hook: useProStatus()
Hook->>Privy: getAccessToken()
Privy-->>Hook: accessToken
alt authenticated && account_id present
Hook->>Route: fetch(/api/subscription/status?accountId=..., { Authorization: Bearer token })
Route->>ExtAPI: fetch(/api/subscriptions/status, { Authorization: Bearer token }, cache:no-store)
ExtAPI-->>Route: status + body
Route-->>Hook: status code + body
Hook-->>React: parsed ProStatusResponse
React->>User: Render subscription state
else Missing auth or account_id
Hook-->>React: disabled query or error
React->>User: Prompt sign-in
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
hooks/useProStatus.ts (1)
4-4: ⚡ Quick winMove
ProStatusResponseout of the route module.Line 4 makes this client hook depend on
app/api/subscription/status/route.ts, which is a server entrypoint importingnext/server. Even if this is currently erased as a type-only use, it is a fragile client/server boundary. Put the contract in a sharedtypesmodule and import it withimport type.♻️ Proposed change
-import { ProStatusResponse } from "@/app/api/subscription/status/route"; +import type { ProStatusResponse } from "@/types/subscription";// types/subscription.ts export interface ProStatusResponse { isPro: boolean; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useProStatus.ts` at line 4, The hook useProStatus.ts currently imports ProStatusResponse from the server route module (app/api/subscription/status/route) which breaks the client/server boundary; move the ProStatusResponse type into a shared types module (e.g., types/subscription.ts exporting interface ProStatusResponse { isPro: boolean }) and change the import in useProStatus.ts to a type-only import (import type { ProStatusResponse } from "@/types/subscription"). Update any other files referencing ProStatusResponse to import from the new shared types module instead of the route module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/subscription/status/route.ts`:
- Around line 34-47: The proxy fetch to the Recoup API using
upstreamUrl/upstreamHeaders and building a NextResponse from upstream may throw
on network/transport errors; wrap the fetch + read (the upstream = await
fetch(...) and body = await upstream.text()) in a try/catch, and on any
transport failure return a controlled 502 NextResponse that uses the same JSON
error shape your app expects (e.g., { error: "upstream_unavailable", message:
"Recoup API unavailable" }) and set Content-Type to application/json, preserving
other successful behavior when the fetch succeeds; use NextResponse and
upstream.status only for successful upstream responses and log the caught error
for diagnostics.
---
Nitpick comments:
In `@hooks/useProStatus.ts`:
- Line 4: The hook useProStatus.ts currently imports ProStatusResponse from the
server route module (app/api/subscription/status/route) which breaks the
client/server boundary; move the ProStatusResponse type into a shared types
module (e.g., types/subscription.ts exporting interface ProStatusResponse {
isPro: boolean }) and change the import in useProStatus.ts to a type-only import
(import type { ProStatusResponse } from "@/types/subscription"). Update any
other files referencing ProStatusResponse to import from the new shared types
module instead of the route module.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a55527e2-fbfa-4a37-b2dd-ec81f7095b85
📒 Files selected for processing (2)
app/api/subscription/status/route.tshooks/useProStatus.ts
There was a problem hiding this comment.
2 issues found across 2 files
Confidence score: 3/5
- There is a concrete reliability risk in
app/api/subscription/status/route.ts: the upstreamfetchis not wrapped intry/catch, so network-level failures can escape as generic 500s instead of a controlled JSON error response. app/api/subscription/status/route.tsmay also throw when proxying 204/304 responses if a body is still passed toNextResponse, which can convert otherwise valid no-body upstream statuses into 500s.- This lands at moderate merge risk because the top issue is medium severity (6/10) with high confidence and can directly affect API behavior seen by clients.
- Pay close attention to
app/api/subscription/status/route.ts- add transport-error handling and no-body status handling to avoid unintended 500 responses.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/api/subscription/status/route.ts">
<violation number="1" location="app/api/subscription/status/route.ts:34">
P2: Wrap the upstream `fetch` call in a `try/catch` to handle network-level failures (e.g., DNS resolution, connection refused). Without it, transport errors will surface as a generic 500 instead of a controlled JSON error response consistent with the rest of this handler.</violation>
<violation number="2" location="app/api/subscription/status/route.ts:44">
P2: Handle no-body statuses before constructing `NextResponse`; passing even an empty string body with 204/304 will throw and turn the proxy response into a 500.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…oint - Added try-catch block around the fetch call to handle upstream fetch failures gracefully. - Updated the response to return a 502 Bad Gateway status with an error message when the fetch fails.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/api/subscription/status/route.ts">
<violation number="1" location="app/api/subscription/status/route.ts:43">
P2: Capture and log the caught error object instead of using a bare `catch` so upstream failures are diagnosable.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…scription status endpoint - Updated error handling to log the specific error message when upstream fetch fails. - Adjusted response logic to return null for 204 and 304 status codes, improving response consistency.
- Deleted the /api/subscription/status endpoint as it is no longer needed. - Updated the useProStatus hook to directly fetch from the Recoup API, improving code clarity and reducing redundancy.
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: This is a high-impact change that modifies the core subscription status logic, moving it from a local API route to an external service and changing the authentication mechanism.
The Recoup API endpoint moved from GET /api/subscriptions/status?accountId=
to GET /api/accounts/{id}/subscription as part of recoupable/api#506 and
recoupable/docs#183. Hitting the old path now returns 404, which would leave
all paying users with isSubscribed=false. ProStatusResponse stays narrow
({ isPro }); the wider response shape from the api side is ignored by
structural typing.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Refactors subscription status from internal API to external Recoup API with Privy auth. Changes core business logic and data source; high risk of breaking pro status checks. Requires human review for
Summary by cubic
Removed the deprecated
/api/subscription/statusendpoint and pointeduseProStatusto Recoup’s account-scoped path to prevent false negatives after the API change.app/api/subscription/status/route.ts; client now calls${NEW_API_BASE_URL}/api/accounts/{id}/subscription.Authorization: Bearer <accessToken>from@privy-io/react-auth; throws if missing and runs only whenauthenticatedandaccount_idexist.{ isPro }, ignoring extra API fields.Written for commit 394219e. Summary will update on new commits.
Summary by CodeRabbit