feat: improve TRPC error propagation #129
Conversation
WalkthroughCentralizes error handling with a new toTRPCError/ResourceNotFoundError and maps tRPC error codes to HTTP statuses via responseMeta in the tRPC route. Multiple routers now wrap logic in try/catch and delegate error translation. subscription-plan adds getUserActiveSubscriptions and a getAllPlans stub. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant TRPCRoute as "tRPC Route"
participant RouterProc as "Router Procedure"
participant ErrorMapper as "toTRPCError"
participant RespMeta as "responseMeta"
Client->>TRPCRoute: HTTP tRPC request (GET/POST)
TRPCRoute->>RouterProc: invoke procedure
alt success
RouterProc-->>TRPCRoute: result
TRPCRoute->>RespMeta: responseMeta({ result, errors: [] })
RespMeta-->>TRPCRoute: set status 200
TRPCRoute-->>Client: HTTP 200 + data
else error
RouterProc->>ErrorMapper: toTRPCError(error)
ErrorMapper-->>RouterProc: TRPCError(code, message)
RouterProc-->>TRPCRoute: returns TRPC Error
TRPCRoute->>RespMeta: responseMeta({ errors: [TRPCError] })
RespMeta-->>TRPCRoute: set status (map TRPC→HTTP)
TRPCRoute-->>Client: HTTP <mapped status> + error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
…1-easyinvoice---improve-trpc-error-propogation-we-_do_-propogate-but-with-generic-error-messages
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (23)
src/lib/errors.ts (3)
12-35: Map more HTTP statuses to precise tRPC codes (405, 408, 412, 413, 504).This improves fidelity and client UX by returning the most accurate tRPC code for common scenarios.
Apply this diff to extend mappings, including timeout handling for 408/504:
function mapStatusCodeToTRPCCode(statusCode: number): TRPC_ERROR_CODE_KEY { - if (statusCode >= 400 && statusCode < 500) { + // Explicit timeout handling + if (statusCode === 408 || statusCode === 504) { + return "TIMEOUT"; + } + + if (statusCode >= 400 && statusCode < 500) { switch (statusCode) { case 400: return "BAD_REQUEST"; + case 405: + return "METHOD_NOT_SUPPORTED"; case 401: return "UNAUTHORIZED"; case 403: return "FORBIDDEN"; case 404: return "NOT_FOUND"; + case 412: + return "PRECONDITION_FAILED"; case 409: return "CONFLICT"; + case 413: + return "PAYLOAD_TOO_LARGE"; case 422: return "UNPROCESSABLE_CONTENT"; case 429: return "TOO_MANY_REQUESTS"; default: return "BAD_REQUEST"; } } return "INTERNAL_SERVER_ERROR"; }
71-78: Consider reducing message leakage for unknown throwables (optional).If non-Error values get thrown, returning their message could leak internal details. In production, you may want a generic message while keeping the original as cause.
- const message = - error instanceof Error ? error.message : "Unknown error occurred"; + const message = + process.env.NODE_ENV === "production" + ? "Internal server error" + : error instanceof Error + ? error.message + : "Unknown error occurred";
37-78: Add unit tests to lock down toTRPCError behavior across inputs.This function is now a critical path. Suggest tests for:
- TRPCError passthrough
- AxiosError: 400/401/403/404/409/412/413/422/429, 408/504 (timeout), network error (no response)
- ResourceNotFoundError mapping
- Generic Error with status/statusCode and without
I can scaffold a jest/vitest test suite covering these cases if you want.
src/server/routers/invoice.ts (1)
135-143: Consistent error propagation: wrap all API/DB interactions in try/catch using toTRPCError.This block is now correct. For consistency across the router, consider updating sendPaymentIntent to also catch and translate Axios errors; otherwise, network/API failures will surface as generic INTERNAL_SERVER_ERROR without proper HTTP mapping.
Proposed change for sendPaymentIntent (outside this hunk):
// sendPaymentIntent .mutation(async ({ input }) => { const { paymentIntent, payload } = input; try { const response = await apiClient.post( `/v2/request/payment-intents/${paymentIntent}`, payload, ); return response.data; } catch (error) { throw toTRPCError(error); } }),src/server/routers/compliance.ts (2)
4-4: Centralized error translation imported—consider applying to remaining handlers for consistency.Most procedures now use toTRPCError, but submitComplianceInfo and the outer catch in getComplianceStatus still manually map and default to INTERNAL_SERVER_ERROR. For uniformity and future maintainability, consider switching those to toTRPCError (retaining the 404-special-case return in getComplianceStatus).
If helpful, I can provide a patch to refactor those blocks as well.
385-401: Avoid logging full upstream response bodies; redact or summarize to minimize PII leakage.The JSON-stringified Axios response body can contain PII. Keep context (status, code, message) but redact payloads.
Apply this diff to minimize sensitive logging while preserving diagnostics:
- console.error( - "Error allowing payment details:", - error instanceof AxiosError - ? JSON.stringify(error.response?.data) - : error instanceof Error - ? error.message - : error, - ); + console.error("Error allowing payment details:", { + status: error instanceof AxiosError ? error.response?.status : undefined, + code: error instanceof AxiosError ? error.code : undefined, + message: error instanceof AxiosError + ? (typeof error.response?.data === "string" + ? error.response?.data + : (error.response?.data as any)?.message) ?? error.message + : error instanceof Error + ? error.message + : String(error), + });src/server/routers/recurring-payment.ts (3)
122-127: Redundant status check: axios throws on non-2xx by default.The manual response.status check won’t execute for non-2xx unless validateStatus is customized. Rely on axios throwing and let toTRPCError map the failure.
- if (response.status !== 200) { - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: "Failed to update recurring payment", - }); - } + // No-op: axios throws for non-2xx; if we get here, it's successful.
188-194: Apply the same URL path fix and typing here.Mirror the leading slash and generic typing for consistent behavior.
- const response = await apiClient.patch( - `v2/payouts/recurring/${input.externalPaymentId}`, + const response = await apiClient.patch<UpdateRecurringPaymentResponse>( + `/v2/payouts/recurring/${input.externalPaymentId}`, { action: input.action, }, );
195-200: Remove redundant status check here as well.Same rationale as above; let axios + toTRPCError handle failures.
- if (response.status !== 200) { - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: "Failed to update recurring payment", - }); - } + // No-op: axios throws for non-2xx; reaching here implies success.src/server/routers/subscription-plan.ts (3)
56-69: Optional: detect no-op deletes and return NOT_FOUND.Currently, deactivating a non-existent or non-owned plan silently succeeds. Consider checking affected rows and throwing NOT_FOUND to improve API ergonomics.
Example (outside this hunk):
const res = await db.update(...).where(...).returning({ id: subscriptionPlanTable.id }); if (res.length === 0) throw new TRPCError({ code: "NOT_FOUND", message: "Plan not found" });
146-211: LGTM: getAllPayments composes SubscriptionPayment DTOs and maps errors.Minor nit: paymentNumber is derived from local array index; if payments are not guaranteed sorted, you might want to sort by date first for deterministic numbering.
257-282: Filter out cancelled entries for “active subscriptions.”Given the name, returning cancelled entries may surprise clients. Filter status to exclude "cancelled" (or include only "active") for clarity.
- const userSubscriptions = await db.query.recurringPaymentTable.findMany({ - where: and( - eq(recurringPaymentTable.userId, user.id), - isNotNull(recurringPaymentTable.subscriptionId), - ), + const userSubscriptions = await db.query.recurringPaymentTable.findMany({ + where: and( + eq(recurringPaymentTable.userId, user.id), + isNotNull(recurringPaymentTable.subscriptionId), + not(eq(recurringPaymentTable.status, "cancelled")), + ), orderBy: desc(recurringPaymentTable.createdAt), with: { subscription: { columns: { id: true, label: true, trialDays: true, }, }, }, });src/server/routers/payment.ts (5)
15-20: Authorization checks are redundant under protectedProcedureprotectedProcedure already enforces auth. These explicit user checks are duplicate and add noise.
Apply these diffs to remove the redundant checks:
@@ - if (!user) { - throw new TRPCError({ - code: "UNAUTHORIZED", - message: "You must be logged in to pay", - }); - }@@ - if (!user) { - throw new TRPCError({ - code: "UNAUTHORIZED", - message: "You must be logged in to pay", - }); - }Also applies to: 53-58
40-45: Simplify catch: toTRPCError already passes through TRPCErrortoTRPCError returns the original TRPCError instance. The explicit instanceof check is unnecessary.
- } catch (error) { - if (error instanceof TRPCError) { - throw error; - } - throw toTRPCError(error); - } + } catch (error) { + throw toTRPCError(error); + }
76-81: Same catch simplification applies hereMirror the simplified error handling in batchPay.
- } catch (error) { - if (error instanceof TRPCError) { - throw error; - } - throw toTRPCError(error); - } + } catch (error) { + throw toTRPCError(error); + }
22-24: Trim env vars to avoid sending empty fee fieldsEmpty strings from envs evaluate truthy and would incorrectly include fee fields (in pay). Trim to avoid this edge case.
- const feePercentage = process.env.FEE_PERCENTAGE_FOR_PAYMENT; - const feeAddress = process.env.FEE_ADDRESS_FOR_PAYMENT; + const feePercentage = process.env.FEE_PERCENTAGE_FOR_PAYMENT?.trim(); + const feeAddress = process.env.FEE_ADDRESS_FOR_PAYMENT?.trim();
60-73: Align fee field handling with pay; omit fee keys unless both envs are presentCurrently, batchPay always includes feePercentage/feeAddress keys (potentially undefined), which is inconsistent with pay. Conditionally include them only when both envs exist.
- const response = await apiClient.post("v2/payouts/batch", { - requests: input.payouts - ? input.payouts.map((payout) => ({ - amount: payout.amount.toString(), - payee: payout.payee, - invoiceCurrency: payout.invoiceCurrency, - paymentCurrency: payout.paymentCurrency, - feePercentage: process.env.FEE_PERCENTAGE_FOR_PAYMENT, - feeAddress: process.env.FEE_ADDRESS_FOR_PAYMENT, - })) - : undefined, - requestIds: input.requestIds, - payer: input.payer, - }); + const feePercentage = process.env.FEE_PERCENTAGE_FOR_PAYMENT?.trim(); + const feeAddress = process.env.FEE_ADDRESS_FOR_PAYMENT?.trim(); + + const response = await apiClient.post("v2/payouts/batch", { + requests: input.payouts + ? input.payouts.map((payout) => ({ + amount: payout.amount.toString(), + payee: payout.payee, + invoiceCurrency: payout.invoiceCurrency, + paymentCurrency: payout.paymentCurrency, + ...(feePercentage && feeAddress + ? { feePercentage, feeAddress } + : {}), + })) + : undefined, + requestIds: input.requestIds, + payer: input.payer, + });src/app/api/trpc/[trpc]/route.ts (2)
25-43: Robust translator for both string and numeric error codesHandling both TRPC_ERROR_CODES_BY_KEY and raw numeric codes ensures compatibility across adapters and versions. Defaulting to 500 is reasonable.
Optional follow-up: consider extracting getHttpStatusFromTrpcError into a small shared util if you anticipate using it outside this route.
70-78: Batch semantics: using the first error to set status is acceptable, but note trade-offsFor batched calls, this sets a single HTTP status based on the first error. This matches typical tRPC patterns, but the "worst" error might be later in the list.
No action required; just documenting the trade-off.
src/server/routers/invoice-me.ts (4)
19-24: Redundant auth checks under protectedProcedureprotectedProcedure already guarantees an authenticated user in ctx. These explicit checks are not needed and add duplication.
@@ - if (!user) { - throw new TRPCError({ - code: "UNAUTHORIZED", - message: "You must be logged in to create an invoice me link", - }); - }@@ - if (!user) { - throw new TRPCError({ - code: "UNAUTHORIZED", - message: "You must be logged in to delete an invoice me link", - }); - }Also applies to: 60-66
26-31: Return created link ID to the clientAs written, create returns undefined. Returning the new ID improves UX (e.g., redirect/copy link) and avoids an immediate follow-up query.
- await db.insert(invoiceMeTable).values({ - id: ulid(), - label: input.label, - userId: user.id, - }); + const newId = ulid(); + await db.insert(invoiceMeTable).values({ + id: newId, + label: input.label, + userId: user.id, + }); + return { id: newId };If the client already expects no return value, ignore this. Otherwise, consider adding this now to avoid an extra read.
84-93: Public endpoint exposes user email; verify PII requirementsgetById is a publicProcedure (which aligns with prior decisions for shareable endpoints), but it returns the user’s email. If the link is shared publicly, this might be an unnecessary PII exposure.
user: { columns: { name: true, - email: true, id: true, }, },Confirm whether the UI needs the email. If not, removing it reduces exposure surface.
68-76: Optionally report NOT_FOUND when delete affects no rowsVerified that
.returning()is already in use across the codebase (e.g., insrc/server/routers/recurring-payment.ts,src/server/routers/invoice.ts,src/app/api/webhook/route.ts), so the DB driver supports it. You can enhance the delete path insrc/server/routers/invoice-me.tsto throw aNOT_FOUNDerror when nothing is removed:• File: src/server/routers/invoice-me.ts
Lines: 68–76- await db - .delete(invoiceMeTable) - .where( - and( - eq(invoiceMeTable.id, input), - eq(invoiceMeTable.userId, user.id), - ), - ); + const deleted = await db + .delete(invoiceMeTable) + .where( + and( + eq(invoiceMeTable.id, input), + eq(invoiceMeTable.userId, user.id), + ), + ) + .returning({ id: invoiceMeTable.id }); + if (deleted.length === 0) { + throw new TRPCError({ code: "NOT_FOUND", message: "Invoice link not found" }); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
src/app/api/trpc/[trpc]/route.ts(2 hunks)src/lib/errors.ts(1 hunks)src/server/routers/compliance.ts(6 hunks)src/server/routers/invoice-me.ts(3 hunks)src/server/routers/invoice.ts(3 hunks)src/server/routers/payment.ts(3 hunks)src/server/routers/recurring-payment.ts(5 hunks)src/server/routers/subscription-plan.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-02-14T12:48:42.125Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#8
File: src/app/invoice-me/page.tsx:21-21
Timestamp: 2025-02-14T12:48:42.125Z
Learning: In the easy-invoice codebase, error handling for TRPC queries is not required at the component level as confirmed by the maintainer.
Applied to files:
src/server/routers/invoice.tssrc/server/routers/invoice-me.tssrc/server/routers/payment.ts
📚 Learning: 2025-02-12T12:40:14.742Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/server/routers/invoice.ts:88-109
Timestamp: 2025-02-12T12:40:14.742Z
Learning: The payRequest endpoint in src/server/routers/invoice.ts is intentionally kept public (using publicProcedure) to allow invoice sharing and payment by anyone with the payment reference, similar to how payment links work in other payment systems.
Applied to files:
src/server/routers/invoice-me.tssrc/server/routers/payment.ts
🧬 Code Graph Analysis (6)
src/server/routers/recurring-payment.ts (3)
src/server/db/schema.ts (2)
recurringPaymentTable(218-257)subscriptionPlanTable(283-300)src/lib/errors.ts (1)
toTRPCError(37-78)src/lib/axios.ts (1)
apiClient(3-8)
src/server/routers/invoice.ts (1)
src/lib/errors.ts (1)
toTRPCError(37-78)
src/server/routers/invoice-me.ts (3)
src/server/db/schema.ts (1)
invoiceMeTable(272-281)src/lib/errors.ts (1)
toTRPCError(37-78)src/server/trpc.ts (2)
protectedProcedure(39-39)publicProcedure(12-12)
src/server/routers/payment.ts (4)
src/lib/axios.ts (1)
apiClient(3-8)src/lib/errors.ts (1)
toTRPCError(37-78)src/server/trpc.ts (1)
protectedProcedure(39-39)src/lib/schemas/payment.ts (1)
batchPaymentApiSchema(55-67)
src/server/routers/compliance.ts (1)
src/lib/errors.ts (1)
toTRPCError(37-78)
src/server/routers/subscription-plan.ts (4)
src/server/db/schema.ts (2)
subscriptionPlanTable(283-300)recurringPaymentTable(218-257)src/lib/errors.ts (1)
toTRPCError(37-78)src/server/trpc.ts (1)
protectedProcedure(39-39)src/lib/types/index.ts (1)
SubscriptionPayment(22-35)
🔇 Additional comments (20)
src/lib/errors.ts (1)
57-69: LGTM on custom status propagation for generic Errors.Gracefully handling Error instances with status/statusCode covers common server-side patterns.
src/server/routers/invoice.ts (2)
2-2: Good move: centralizing error translation via toTRPCError.This aligns the router with the new error handling approach.
186-193: LGTM on createFromInvoiceMe error mapping.Standardized catch-to-TRPC behavior looks good.
src/server/routers/compliance.ts (4)
131-134: LGTM: updateAgreementStatus now standardizes error mapping.Error propagation is now uniform and will pick up AxiosError status mapping.
470-473: LGTM: getPaymentDetails now delegates to toTRPCError.Good consistency with the centralized error handling approach.
533-534: LGTM: getPaymentDetailsById catch path standardized.Rethrowing TRPCError and otherwise delegating to toTRPCError is solid.
565-567: LGTM: getUserByEmail adopts toTRPCError for non-TRPCError failures.This brings the procedure in line with the new error handling strategy.
src/server/routers/recurring-payment.ts (2)
29-44: LGTM: centralized error handling on listing.Wrapping the DB query with toTRPCError ensures consistent error propagation.
52-82: LGTM: createRecurringPayment now properly normalizes failures.The try/catch with toTRPCError is correct.
src/server/routers/subscription-plan.ts (6)
1-1: LGTM: importing toTRPCError to standardize error propagation.Good step toward uniform error handling across these procedures.
16-31: LGTM: create wrapped with toTRPCError.Insert path is clear and errors are normalized.
36-49: LGTM: getAll now returns errors consistently via toTRPCError.Looks good.
76-105: LGTM: getById preserves NOT_FOUND semantics and normalizes other failures.Good.
141-145: LGTM: getAllSubscribers error handling standardized.Consistent with the rest of the router.
217-256: LGTM: owner check and NOT_FOUND behavior are correct; errors centralized.No issues spotted.
src/server/routers/payment.ts (2)
102-114: LGTM: Consistent error translation and axios handlingThe try/catch pattern with toTRPCError aligns with the centralized error handling objective and leverages axios to surface non-2xx responses correctly.
60-74: Confirm external API payload property for batch payoutsThe internal schema and client-side validation use
payouts(andrequestIds), but the outgoing call sends arequestsarray. I didn’t find any other API client calls usingrequestsfor this endpoint. Please verify against the v2/payouts/batch contract that the endpoint expects arequestsfield; if it actually expectspayouts, update the payload accordingly to prevent 4xx errors.Locations to check:
- src/server/routers/payment.ts: lines 60–74
src/app/api/trpc/[trpc]/route.ts (1)
8-23: Good, explicit TRPC→HTTP mappingThe mapping covers the standard tRPC error keys, including UNPROCESSABLE_CONTENT and CLIENT_CLOSED_REQUEST. This makes responses predictable for clients.
src/server/routers/invoice-me.ts (2)
45-53: LGTM: Query and error translation are consistentThe query is scoped to the authenticated user and wrapped in the centralized error handler.
97-102: LGTM: Proper NOT_FOUND signal for missing linksCorrect use of TRPCError with NOT_FOUND here; it will map to 404 via the new responseMeta.
aimensahnoun
deleted the
111-easyinvoice---improve-trpc-error-propogation-we-_do_-propogate-but-with-generic-error-messages
branch
Problem:
TRPC throwing wrong errors
Solution:
Update the code to throw correct TRPC errors, in all of routers.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor