From d50e1a0e157c4c8ab384f082a8c1b13fb195d771 Mon Sep 17 00:00:00 2001 From: Benny Joo Date: Wed, 25 Jun 2025 16:18:01 -0400 Subject: [PATCH] feat: Leverage ErrorCode cases for correct error codes in server responses (#22023) * Leverage ErrorCode cases for correct error codes in server responses * better test * fix --- .../features/bookings/lib/handleNewBooking.ts | 23 +--- .../server/getServerErrorFromUnknown.test.ts | 107 +++++++++++++++++- .../lib/server/getServerErrorFromUnknown.ts | 30 ++++- 3 files changed, 136 insertions(+), 24 deletions(-) diff --git a/packages/features/bookings/lib/handleNewBooking.ts b/packages/features/bookings/lib/handleNewBooking.ts index 5823ebf76a..4934b6bcbe 100644 --- a/packages/features/bookings/lib/handleNewBooking.ts +++ b/packages/features/bookings/lib/handleNewBooking.ts @@ -52,7 +52,6 @@ import { getFirstDelegationConferencingCredentialAppLocation, } from "@calcom/lib/delegationCredential/server"; import { ErrorCode } from "@calcom/lib/errorCodes"; -import type { ErrorWithCode } from "@calcom/lib/errors"; import { getErrorFromUnknown } from "@calcom/lib/errors"; import { getEventName, updateHostInEventName } from "@calcom/lib/event"; import { extractBaseEmail } from "@calcom/lib/extract-base-email"; @@ -455,22 +454,12 @@ async function handler( await checkIfBookerEmailIsBlocked({ loggedInUserId: userId, bookerEmail }); if (!rawBookingData.rescheduleUid) { - try { - await checkActiveBookingsLimitForBooker({ - eventTypeId, - maxActiveBookingsPerBooker: eventType.maxActiveBookingsPerBooker, - bookerEmail, - offerToRescheduleLastBooking: eventType.maxActiveBookingPerBookerOfferReschedule, - }); - } catch (err) { - const error = err as ErrorWithCode; - - throw new HttpError({ - statusCode: 400, - message: error.message, - data: error.data, - }); - } + await checkActiveBookingsLimitForBooker({ + eventTypeId, + maxActiveBookingsPerBooker: eventType.maxActiveBookingsPerBooker, + bookerEmail, + offerToRescheduleLastBooking: eventType.maxActiveBookingPerBookerOfferReschedule, + }); } if (isEventTypeLoggingEnabled({ eventTypeId, usernameOrTeamName: reqBody.user })) { diff --git a/packages/lib/server/getServerErrorFromUnknown.test.ts b/packages/lib/server/getServerErrorFromUnknown.test.ts index d04edf37f0..31e920fed6 100644 --- a/packages/lib/server/getServerErrorFromUnknown.test.ts +++ b/packages/lib/server/getServerErrorFromUnknown.test.ts @@ -1,10 +1,42 @@ -import { expect, it, describe } from "vitest"; +import { describe, expect, test } from "vitest"; + +import { ErrorCode } from "@calcom/lib/errorCodes"; +import { ErrorWithCode } from "@calcom/lib/errors"; import { HttpError } from "../http-error"; import { getServerErrorFromUnknown } from "./getServerErrorFromUnknown"; +const test400Codes = [ + ErrorCode.RequestBodyWithouEnd, + ErrorCode.MissingPaymentCredential, + ErrorCode.MissingPaymentAppId, + ErrorCode.AvailabilityNotFoundInSchedule, + ErrorCode.CancelledBookingsCannotBeRescheduled, + ErrorCode.BookingTimeOutOfBounds, + ErrorCode.BookingNotAllowedByRestrictionSchedule, + ErrorCode.BookerLimitExceeded, + ErrorCode.BookerLimitExceededReschedule, +]; + +const test404Codes = [ + ErrorCode.EventTypeNotFound, + ErrorCode.BookingNotFound, + ErrorCode.RestrictionScheduleNotFound, +]; + +const test409Codes = [ + ErrorCode.NoAvailableUsersFound, + ErrorCode.HostsUnavailableForBooking, + ErrorCode.AlreadySignedUpForBooking, + ErrorCode.BookingSeatsFull, + ErrorCode.NotEnoughAvailableSeats, + ErrorCode.BookingConflict, + ErrorCode.PaymentCreationFailure, + ErrorCode.ChargeCardFailure, +]; + describe("getServerErrorFromUnknown", () => { - it("should handle a StripeInvalidRequestError", () => { + test("should handle a StripeInvalidRequestError", () => { const stripeError = { name: "StripeInvalidRequestError", type: "StripeInvalidRequestError", @@ -31,4 +63,75 @@ describe("getServerErrorFromUnknown", () => { expect(result.cause).toEqual(stripeError); expect(result.name).toBe("HttpError"); }); + + test("ErrorWithCode should preserve data property", () => { + const rescheduleData = { + rescheduleUid: "abc123", + startTime: new Date("2024-01-15T10:00:00Z"), + attendees: [{ name: "John Doe", email: "john@example.com" }], + }; + + const error = new ErrorWithCode( + ErrorCode.BookerLimitExceededReschedule, + "Maximum booking limit reached, please reschedule an existing booking", + rescheduleData + ); + + const result = getServerErrorFromUnknown(error); + + expect(result.statusCode).toBe(400); + expect(result.message).toBe("Maximum booking limit reached, please reschedule an existing booking"); + expect(result.data).toEqual(rescheduleData); + }); + + test("Plain Error with ErrorCode message should work the same as ErrorWithCode", () => { + const error = new Error(ErrorCode.NoAvailableUsersFound); + + const result = getServerErrorFromUnknown(error); + + expect(result.statusCode).toBe(409); + expect(result.message).toBe(ErrorCode.NoAvailableUsersFound); + }); + + test("Generic Error without ErrorCode should return 500", () => { + const error = new Error("Some unexpected server error"); + + const result = getServerErrorFromUnknown(error); + + expect(result.statusCode).toBe(500); + expect(result.message).toBe("Some unexpected server error"); + }); + + test("Server-side ErrorCode should return 500", () => { + const error = new ErrorWithCode(ErrorCode.CreatingOauthClientError, "OAuth client creation failed"); + + const result = getServerErrorFromUnknown(error); + + expect(result.statusCode).toBe(500); + expect(result.message).toBe("OAuth client creation failed"); + }); +}); + +test400Codes.forEach((errorCode) => { + test(`${errorCode} should return 400 Bad Request`, () => { + const error = new ErrorWithCode(errorCode, `Test message for ${errorCode}`); + const result = getServerErrorFromUnknown(error); + expect(result.statusCode).toBe(400); + }); +}); + +test404Codes.forEach((errorCode) => { + test(`${errorCode} should return 404 Not Found`, () => { + const error = new ErrorWithCode(errorCode, `Test message for ${errorCode}`); + const result = getServerErrorFromUnknown(error); + expect(result.statusCode).toBe(404); + }); +}); + +test409Codes.forEach((errorCode) => { + test(`${errorCode} should return 409 Conflict`, () => { + const error = new ErrorWithCode(errorCode, `Test message for ${errorCode}`); + const result = getServerErrorFromUnknown(error); + expect(result.statusCode).toBe(409); + }); }); diff --git a/packages/lib/server/getServerErrorFromUnknown.ts b/packages/lib/server/getServerErrorFromUnknown.ts index d33138c932..b7b469bb60 100644 --- a/packages/lib/server/getServerErrorFromUnknown.ts +++ b/packages/lib/server/getServerErrorFromUnknown.ts @@ -4,6 +4,7 @@ import { ZodError } from "zod"; import { stripeInvalidRequestErrorSchema } from "@calcom/app-store/_utils/stripe.types"; import { ErrorCode } from "@calcom/lib/errorCodes"; +import { ErrorWithCode } from "@calcom/lib/errors"; import { TRPCError } from "@trpc/server"; import { getHTTPStatusCodeFromError } from "@trpc/server/http"; @@ -60,6 +61,15 @@ export function getServerErrorFromUnknown(cause: unknown): HttpError { if (parsedStripeError.success) { return getHttpError({ statusCode: 400, cause: parsedStripeError.data }); } + if (cause instanceof ErrorWithCode) { + const statusCode = getStatusCode(cause); + return new HttpError({ + statusCode, + message: cause.message ?? "", + data: cause.data, + cause, + }); + } if (cause instanceof HttpError) { const redactedCause = redactError(cause); return { @@ -87,25 +97,35 @@ export function getServerErrorFromUnknown(cause: unknown): HttpError { }); } -function getStatusCode(cause: Error): number { - switch (cause.message) { +function getStatusCode(cause: Error | ErrorWithCode): number { + const errorCode = cause instanceof ErrorWithCode ? cause.code : cause.message; + + switch (errorCode) { + // 400 Bad Request case ErrorCode.RequestBodyWithouEnd: case ErrorCode.MissingPaymentCredential: case ErrorCode.MissingPaymentAppId: case ErrorCode.AvailabilityNotFoundInSchedule: - return 400; case ErrorCode.CancelledBookingsCannotBeRescheduled: + case ErrorCode.BookingTimeOutOfBounds: + case ErrorCode.BookingNotAllowedByRestrictionSchedule: + case ErrorCode.BookerLimitExceeded: + case ErrorCode.BookerLimitExceededReschedule: return 400; + // 409 Conflict case ErrorCode.NoAvailableUsersFound: case ErrorCode.HostsUnavailableForBooking: - case ErrorCode.PaymentCreationFailure: - case ErrorCode.ChargeCardFailure: case ErrorCode.AlreadySignedUpForBooking: case ErrorCode.BookingSeatsFull: case ErrorCode.NotEnoughAvailableSeats: + case ErrorCode.BookingConflict: + case ErrorCode.PaymentCreationFailure: + case ErrorCode.ChargeCardFailure: return 409; + // 404 Not Found case ErrorCode.EventTypeNotFound: case ErrorCode.BookingNotFound: + case ErrorCode.RestrictionScheduleNotFound: return 404; case ErrorCode.UnableToSubscribeToThePlatform: case ErrorCode.UpdatingOauthClientError: