From 0623887ff8ae909a5e06e717b74348cf3664bf92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Omar=20L=C3=B3pez?= Date: Wed, 23 Apr 2025 09:47:29 -0300 Subject: [PATCH] refactor: Returns 400 error for ZodErrors (#20831) --- .../trpc/server/createNextApiHandler.test.ts | 172 +++++++++++++ packages/trpc/server/createNextApiHandler.ts | 8 +- packages/trpc/server/errorFormatter.test.ts | 234 ++++++++++++++++++ packages/trpc/server/errorFormatter.ts | 39 +++ .../middlewares/captureErrorsMiddleware.ts | 19 -- packages/trpc/server/onErrorHandler.ts | 23 ++ .../trpc/server/procedures/authedProcedure.ts | 7 +- .../trpc/server/procedures/publicProcedure.ts | 3 +- packages/trpc/server/trpc.ts | 2 + 9 files changed, 476 insertions(+), 31 deletions(-) create mode 100644 packages/trpc/server/createNextApiHandler.test.ts create mode 100644 packages/trpc/server/errorFormatter.test.ts create mode 100644 packages/trpc/server/errorFormatter.ts delete mode 100644 packages/trpc/server/middlewares/captureErrorsMiddleware.ts create mode 100644 packages/trpc/server/onErrorHandler.ts diff --git a/packages/trpc/server/createNextApiHandler.test.ts b/packages/trpc/server/createNextApiHandler.test.ts new file mode 100644 index 0000000000..7055e41ca2 --- /dev/null +++ b/packages/trpc/server/createNextApiHandler.test.ts @@ -0,0 +1,172 @@ +import { captureException } from "@sentry/nextjs"; +import type { NextApiRequest } from "next"; +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; +import type { ZodIssue } from "zod"; +import { ZodError } from "zod"; + +import { TRPCError } from "@trpc/server"; +import type { AnyRouter } from "@trpc/server"; + +import { createNextApiHandler } from "./createNextApiHandler"; +import { errorFormatter } from "./errorFormatter"; +import { onErrorHandler } from "./onErrorHandler"; + +vi.mock("@sentry/nextjs", () => ({ + captureException: vi.fn(), +})); + +describe("createNextApiHandler", () => { + const mockRouter = { + createCaller: vi.fn(), + _def: { + procedures: {}, + middleware: [], + contextOptions: {}, + }, + } as unknown as AnyRouter; + + const mockReq = { + method: "POST", + url: "/api/trpc/test", + } as NextApiRequest; + + let consoleSpy: ReturnType; + + beforeEach(() => { + consoleSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + vi.clearAllMocks(); + }); + + afterEach(() => { + consoleSpy.mockRestore(); + }); + + describe("error formatting", () => { + it("should format ZodError as BAD_REQUEST", () => { + const zodError = new ZodError([ + { + path: ["name"], + message: "Required", + code: "invalid_type", + expected: "string", + received: "undefined", + } as ZodIssue, + ]); + + const error = new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Internal Server Error", + cause: zodError, + }); + + const result = errorFormatter({ + shape: { + message: "Internal Server Error", + code: 500, + data: { + code: "INTERNAL_SERVER_ERROR", + httpStatus: 500, + path: "test", + }, + }, + error, + type: "query", + path: "test", + input: undefined, + ctx: undefined, + }); + + expect(result).toEqual({ + message: "Invalid input", + code: 400, + data: { + code: "BAD_REQUEST", + httpStatus: 400, + path: "test", + zodError: { + formErrors: [], + fieldErrors: { + name: ["Required"], + }, + }, + }, + }); + }); + + it("should not modify non-ZodError errors", () => { + const baseError = new Error("Database connection failed"); + const error = new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Internal Server Error", + cause: baseError, + }); + + const shape = { + message: "Internal Server Error", + code: 500, + data: { + code: "INTERNAL_SERVER_ERROR", + httpStatus: 500, + path: "test", + }, + }; + + const result = errorFormatter({ + shape, + error, + type: "query", + path: "test", + input: undefined, + ctx: undefined, + }); + + expect(result).toEqual(shape); + }); + }); + + describe("error handling", () => { + it("should handle TRPC errors correctly", () => { + const handler = createNextApiHandler(mockRouter); + const error = new TRPCError({ code: "BAD_REQUEST", message: "Invalid input" }); + + onErrorHandler({ + error, + req: mockReq, + }); + + expect(captureException).not.toHaveBeenCalled(); + expect(consoleSpy).not.toHaveBeenCalled(); + }); + + it("should handle internal server errors correctly", () => { + const handler = createNextApiHandler(mockRouter); + const baseError = new Error("Database connection failed"); + const error = new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Internal Server Error", + cause: baseError, + }); + + onErrorHandler({ + error, + req: mockReq, + }); + + expect(consoleSpy).toHaveBeenCalledWith("Something went wrong", error); + expect(captureException).toHaveBeenCalledWith(error); + }); + + it("should not log client errors", () => { + const handler = createNextApiHandler(mockRouter); + const error = new TRPCError({ code: "BAD_REQUEST", message: "Invalid input" }); + + onErrorHandler({ + error, + req: mockReq, + }); + + expect(consoleSpy).not.toHaveBeenCalled(); + expect(captureException).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/trpc/server/createNextApiHandler.ts b/packages/trpc/server/createNextApiHandler.ts index 064d1f388a..70915b67ad 100644 --- a/packages/trpc/server/createNextApiHandler.ts +++ b/packages/trpc/server/createNextApiHandler.ts @@ -4,6 +4,7 @@ import type { AnyRouter } from "@trpc/server"; import { createNextApiHandler as _createNextApiHandler } from "@trpc/server/adapters/next"; import { createContext as createTrpcContext } from "./createContext"; +import { onErrorHandler } from "./onErrorHandler"; /** * Creates an API handler executed by Next.js. @@ -20,12 +21,7 @@ export function createNextApiHandler(router: AnyRouter, isPublic = false, namesp /** * @link https://trpc.io/docs/error-handling */ - onError({ error }) { - if (error.code === "INTERNAL_SERVER_ERROR") { - // send to bug reporting - console.error("Something went wrong", error); - } - }, + onError: onErrorHandler, /** * Enable query batching */ diff --git a/packages/trpc/server/errorFormatter.test.ts b/packages/trpc/server/errorFormatter.test.ts new file mode 100644 index 0000000000..335619491c --- /dev/null +++ b/packages/trpc/server/errorFormatter.test.ts @@ -0,0 +1,234 @@ +import { describe, expect, it } from "vitest"; +import { z } from "zod"; + +import { TRPCError } from "@trpc/server"; + +import { errorFormatter } from "./errorFormatter"; + +describe("errorFormatter", () => { + const baseShape = { + message: "Internal Server Error", + code: 500, + data: { + code: "INTERNAL_SERVER_ERROR", + httpStatus: 500, + path: "test", + }, + }; + + it("should format ZodError as BAD_REQUEST", () => { + const zodError = new z.ZodError([ + { + path: ["name"], + message: "Required", + code: "invalid_type", + expected: "string", + received: "undefined", + }, + ]); + + const error = new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Internal Server Error", + cause: zodError, + }); + + const result = errorFormatter({ + shape: baseShape, + error, + type: "query", + path: "test", + input: undefined, + ctx: undefined, + }); + + expect(result).toEqual({ + message: "Invalid input", + code: 400, + data: { + code: "BAD_REQUEST", + httpStatus: 400, + path: "test", + zodError: { + formErrors: [], + fieldErrors: { + name: ["Required"], + }, + }, + }, + }); + }); + + it("should handle nested ZodError", () => { + const schema = z.object({ + user: z.object({ + name: z.string(), + age: z.number().min(18), + }), + }); + + const zodError = new z.ZodError([ + { + path: ["user", "age"], + message: "Number must be greater than or equal to 18", + code: "too_small", + minimum: 18, + type: "number", + inclusive: true, + }, + ]); + + const error = new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Internal Server Error", + cause: zodError, + }); + + const result = errorFormatter({ + shape: baseShape, + error, + type: "query", + path: "test", + input: undefined, + ctx: undefined, + }); + + expect(result).toEqual({ + message: "Invalid input", + code: 400, + data: { + code: "BAD_REQUEST", + httpStatus: 400, + path: "test", + zodError: { + formErrors: [], + fieldErrors: { + user: ["Number must be greater than or equal to 18"], + }, + }, + }, + }); + }); + + it("should handle multiple ZodError issues", () => { + const zodError = new z.ZodError([ + { + path: ["name"], + message: "Required", + code: "invalid_type", + expected: "string", + received: "undefined", + }, + { + path: ["age"], + message: "Number must be greater than or equal to 18", + code: "too_small", + minimum: 18, + type: "number", + inclusive: true, + }, + ]); + + const error = new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Internal Server Error", + cause: zodError, + }); + + const result = errorFormatter({ + shape: baseShape, + error, + type: "query", + path: "test", + input: undefined, + ctx: undefined, + }); + + expect(result).toEqual({ + message: "Invalid input", + code: 400, + data: { + code: "BAD_REQUEST", + httpStatus: 400, + path: "test", + zodError: { + formErrors: [], + fieldErrors: { + name: ["Required"], + age: ["Number must be greater than or equal to 18"], + }, + }, + }, + }); + }); + + it("should not modify TRPCError", () => { + const error = new TRPCError({ + code: "BAD_REQUEST", + message: "Invalid input", + }); + + const result = errorFormatter({ + shape: baseShape, + error, + type: "query", + path: "test", + input: undefined, + ctx: undefined, + }); + + expect(result).toEqual(baseShape); + }); + + it("should not modify generic Error", () => { + const baseError = new Error("Database connection failed"); + const error = new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Internal Server Error", + cause: baseError, + }); + + const result = errorFormatter({ + shape: baseShape, + error, + type: "query", + path: "test", + input: undefined, + ctx: undefined, + }); + + expect(result).toEqual(baseShape); + }); + + it("should handle empty ZodError", () => { + const zodError = new z.ZodError([]); + const error = new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Internal Server Error", + cause: zodError, + }); + + const result = errorFormatter({ + shape: baseShape, + error, + type: "query", + path: "test", + input: undefined, + ctx: undefined, + }); + + expect(result).toEqual({ + message: "Invalid input", + code: 400, + data: { + code: "BAD_REQUEST", + httpStatus: 400, + path: "test", + zodError: { + formErrors: [], + fieldErrors: {}, + }, + }, + }); + }); +}); diff --git a/packages/trpc/server/errorFormatter.ts b/packages/trpc/server/errorFormatter.ts new file mode 100644 index 0000000000..76818470fb --- /dev/null +++ b/packages/trpc/server/errorFormatter.ts @@ -0,0 +1,39 @@ +import { z } from "zod"; + +import type { TRPCError } from "@trpc/server"; + +type ErrorShape = { + message: string; + code: number; + data: { + code: string; + httpStatus: number; + path?: string; + [key: string]: unknown; + }; +}; + +type ErrorFormatterOptions = { + shape: ErrorShape; + error: TRPCError; + type: "query" | "mutation" | "subscription" | "unknown"; + path: string | undefined; + input: unknown; + ctx: unknown; +}; + +export function errorFormatter({ shape, error }: ErrorFormatterOptions): ErrorShape { + if (error.cause instanceof z.ZodError) { + return { + message: "Invalid input", + code: 400, + data: { + code: "BAD_REQUEST", + httpStatus: 400, + path: shape.data.path, + zodError: error.cause.flatten(), + }, + }; + } + return shape; +} diff --git a/packages/trpc/server/middlewares/captureErrorsMiddleware.ts b/packages/trpc/server/middlewares/captureErrorsMiddleware.ts deleted file mode 100644 index 0e7eeb4c80..0000000000 --- a/packages/trpc/server/middlewares/captureErrorsMiddleware.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { captureException } from "@sentry/nextjs"; - -import { redactError } from "@calcom/lib/redactError"; - -import { middleware } from "../trpc"; - -const captureErrorsMiddleware = middleware(async ({ next }) => { - try { - const result = await next(); - if (result.ok) return result; - if (!result.error.cause) return result; - throw result.error.cause; - } catch (error) { - captureException(error); - throw redactError(error); - } -}); - -export default captureErrorsMiddleware; diff --git a/packages/trpc/server/onErrorHandler.ts b/packages/trpc/server/onErrorHandler.ts new file mode 100644 index 0000000000..5045a4f461 --- /dev/null +++ b/packages/trpc/server/onErrorHandler.ts @@ -0,0 +1,23 @@ +import { captureException } from "@sentry/nextjs"; +import type { NextApiRequest, NextApiResponse } from "next"; + +import { getServerErrorFromUnknown } from "@calcom/lib/server/getServerErrorFromUnknown"; + +import type { TRPCError } from "@trpc/server"; + +type OnErrorOptions = { + error: TRPCError; + req?: NextApiRequest; + res?: NextApiResponse; +}; + +export function onErrorHandler({ error }: OnErrorOptions) { + // Convert any error to a HttpError using our centralized error handling + const httpError = getServerErrorFromUnknown(error); + + // Log errors that aren't client errors (400s) + if (httpError.statusCode >= 500) { + captureException(error); + console.error("Something went wrong", error); + } +} diff --git a/packages/trpc/server/procedures/authedProcedure.ts b/packages/trpc/server/procedures/authedProcedure.ts index 7489d535d3..78845080e6 100644 --- a/packages/trpc/server/procedures/authedProcedure.ts +++ b/packages/trpc/server/procedures/authedProcedure.ts @@ -1,4 +1,3 @@ -import captureErrorsMiddleware from "../middlewares/captureErrorsMiddleware"; import perfMiddleware from "../middlewares/perfMiddleware"; import { isAdminMiddleware, isAuthed, isOrgAdminMiddleware } from "../middlewares/sessionMiddleware"; import { procedure } from "../trpc"; @@ -24,10 +23,10 @@ const isRateLimitedByUserIdMiddleware = ({ intervalInMs, limit }: IRateLimitOpti return next({ ctx: { user: ctx.user, session: ctx.session } }); }); */ -const authedProcedure = procedure.use(captureErrorsMiddleware).use(perfMiddleware).use(isAuthed); +const authedProcedure = procedure.use(perfMiddleware).use(isAuthed); /*export const authedRateLimitedProcedure = ({ intervalInMs, limit }: IRateLimitOptions) => authedProcedure.use(isRateLimitedByUserIdMiddleware({ intervalInMs, limit }));*/ -export const authedAdminProcedure = publicProcedure.use(captureErrorsMiddleware).use(isAdminMiddleware); -export const authedOrgAdminProcedure = publicProcedure.use(captureErrorsMiddleware).use(isOrgAdminMiddleware); +export const authedAdminProcedure = publicProcedure.use(isAdminMiddleware); +export const authedOrgAdminProcedure = publicProcedure.use(isOrgAdminMiddleware); export default authedProcedure; diff --git a/packages/trpc/server/procedures/publicProcedure.ts b/packages/trpc/server/procedures/publicProcedure.ts index 5d4c313c91..1b27a25d47 100644 --- a/packages/trpc/server/procedures/publicProcedure.ts +++ b/packages/trpc/server/procedures/publicProcedure.ts @@ -1,7 +1,6 @@ -import captureErrorsMiddleware from "../middlewares/captureErrorsMiddleware"; import perfMiddleware from "../middlewares/perfMiddleware"; import { tRPCContext } from "../trpc"; -const publicProcedure = tRPCContext.procedure.use(captureErrorsMiddleware).use(perfMiddleware); +const publicProcedure = tRPCContext.procedure.use(perfMiddleware); export default publicProcedure; diff --git a/packages/trpc/server/trpc.ts b/packages/trpc/server/trpc.ts index 0e92a36b8f..53b01ab163 100644 --- a/packages/trpc/server/trpc.ts +++ b/packages/trpc/server/trpc.ts @@ -3,9 +3,11 @@ import superjson from "superjson"; import { initTRPC } from "@trpc/server"; import type { createContextInner } from "./createContext"; +import { errorFormatter } from "./errorFormatter"; export const tRPCContext = initTRPC.context().create({ transformer: superjson, + errorFormatter, }); export const router = tRPCContext.router;