From e51f93f9a06bd295096a8867d41e5a57955dffec Mon Sep 17 00:00:00 2001 From: Hemachandar <132386067+hmacr@users.noreply.github.com> Date: Wed, 26 Feb 2025 18:32:21 +0530 Subject: [PATCH] chore: Add API error handler middleware (#8572) --- server/onerror.ts | 37 +-------------- server/routes/api/index.ts | 2 + .../routes/api/middlewares/apiErrorHandler.ts | 47 +++++++++++++++++++ 3 files changed, 50 insertions(+), 36 deletions(-) create mode 100644 server/routes/api/middlewares/apiErrorHandler.ts diff --git a/server/onerror.ts b/server/onerror.ts index ad6848ee8e..4f1c585435 100644 --- a/server/onerror.ts +++ b/server/onerror.ts @@ -6,18 +6,8 @@ import Koa from "koa"; import escape from "lodash/escape"; import isNil from "lodash/isNil"; import snakeCase from "lodash/snakeCase"; -import { - ValidationError as SequelizeValidationError, - EmptyResultError as SequelizeEmptyResultError, -} from "sequelize"; import env from "@server/env"; -import { - AuthorizationError, - ClientClosedRequestError, - InternalError, - NotFoundError, - ValidationError, -} from "@server/errors"; +import { ClientClosedRequestError, InternalError } from "@server/errors"; import { requestErrorHandler } from "@server/logging/sentry"; let errorHtmlCache: Buffer | undefined; @@ -32,16 +22,6 @@ export default function onerror(app: Koa) { err = wrapInNativeError(err); - if (err instanceof SequelizeValidationError) { - if (err.errors && err.errors[0]) { - err = ValidationError( - `${err.errors[0].message} (${err.errors[0].path})` - ); - } else { - err = ValidationError(); - } - } - // Client aborted errors are a 500 by default, but 499 is more appropriate if (err instanceof formidable.errors.FormidableError) { if (err.internalCode === 1002) { @@ -49,21 +29,6 @@ export default function onerror(app: Koa) { } } - if ( - err.code === "ENOENT" || - err instanceof SequelizeEmptyResultError || - /Not found/i.test(err.message) - ) { - err = NotFoundError(); - } - - if ( - !(err instanceof AuthorizationError) && - /Authorization error/i.test(err.message) - ) { - err = AuthorizationError(); - } - // Push only unknown and 500 status errors to sentry if ( typeof err.status !== "number" || diff --git a/server/routes/api/index.ts b/server/routes/api/index.ts index 35b8de9684..2564237eac 100644 --- a/server/routes/api/index.ts +++ b/server/routes/api/index.ts @@ -22,6 +22,7 @@ import groupMemberships from "./groupMemberships"; import groups from "./groups"; import installation from "./installation"; import integrations from "./integrations"; +import apiErrorHandler from "./middlewares/apiErrorHandler"; import apiResponse from "./middlewares/apiResponse"; import apiTracer from "./middlewares/apiTracer"; import editor from "./middlewares/editor"; @@ -60,6 +61,7 @@ api.use(coalesceBody()); api.use(userAgent); api.use(apiTracer()); api.use(apiResponse()); +api.use(apiErrorHandler()); api.use(editor()); // Register plugin API routes before others to allow for overrides diff --git a/server/routes/api/middlewares/apiErrorHandler.ts b/server/routes/api/middlewares/apiErrorHandler.ts new file mode 100644 index 0000000000..23134f2122 --- /dev/null +++ b/server/routes/api/middlewares/apiErrorHandler.ts @@ -0,0 +1,47 @@ +import { Context, Next } from "koa"; +import { + ValidationError as SequelizeValidationError, + EmptyResultError as SequelizeEmptyResultError, +} from "sequelize"; +import { + AuthorizationError, + NotFoundError, + ValidationError, +} from "@server/errors"; + +export default function apiErrorHandler() { + return async function apiErrorHandlerMiddleware(ctx: Context, next: Next) { + try { + await next(); + } catch (err) { + let transformedErr = err; + + if ( + !(err instanceof AuthorizationError) && + /Authorization error/i.test(err.message) + ) { + transformedErr = AuthorizationError(); + } + + if (err instanceof SequelizeValidationError) { + if (err.errors && err.errors[0]) { + transformedErr = ValidationError( + `${err.errors[0].message} (${err.errors[0].path})` + ); + } else { + transformedErr = ValidationError(); + } + } + + if ( + err.code === "ENOENT" || + err instanceof SequelizeEmptyResultError || + /Not found/i.test(err.message) + ) { + transformedErr = NotFoundError(); + } + + throw transformedErr; + } + }; +}