From 7229861a8cf1d5b12b93140fd2095b4a11147bea Mon Sep 17 00:00:00 2001 From: maxbronnikov10 Date: Fri, 2 May 2025 05:24:37 +0300 Subject: [PATCH] refactor(core,express,fastify): HTTP adapter error mapping --- packages/core/adapters/http-adapter.ts | 4 ++ packages/core/router/routes-resolver.ts | 27 +------- .../core/test/router/routes-resolver.spec.ts | 64 +------------------ .../adapters/express-adapter.ts | 13 ++++ .../test/adapters/express-adapter.spec.ts | 27 ++++++++ .../adapters/fastify-adapter.ts | 22 +++++++ .../test/adapters/fastify-adapter.spec.ts | 48 ++++++++++++++ 7 files changed, 116 insertions(+), 89 deletions(-) create mode 100644 packages/platform-fastify/test/adapters/fastify-adapter.spec.ts diff --git a/packages/core/adapters/http-adapter.ts b/packages/core/adapters/http-adapter.ts index 39d1bb6c6d5..bb5a6af045f 100644 --- a/packages/core/adapters/http-adapter.ts +++ b/packages/core/adapters/http-adapter.ts @@ -143,6 +143,10 @@ export abstract class AbstractHttpAdapter< return path; } + public mapException(error: unknown): unknown { + return error; + } + abstract close(); abstract initHttpServer(options: NestApplicationOptions); abstract useStaticAssets(...args: any[]); diff --git a/packages/core/router/routes-resolver.ts b/packages/core/router/routes-resolver.ts index 451b855df96..e389f883f3d 100644 --- a/packages/core/router/routes-resolver.ts +++ b/packages/core/router/routes-resolver.ts @@ -166,7 +166,7 @@ export class RoutesResolver implements Resolver { res: TResponse, next: Function, ) => { - throw this.mapExternalException(err); + throw this.container.getHttpAdapterRef().mapException(err); }; const handler = this.routerExceptionsFilter.create( {}, @@ -182,31 +182,6 @@ export class RoutesResolver implements Resolver { ); } - public mapExternalException(err: any) { - switch (true) { - // SyntaxError is thrown by Express body-parser when given invalid JSON (#422, #430) - // URIError is thrown by Express when given a path parameter with an invalid percentage - // encoding, e.g. '%FF' (#8915) - case err instanceof SyntaxError || err instanceof URIError: - return new BadRequestException(err.message); - case this.isHttpFastifyError(err): - return new HttpException(err.message, err.statusCode); - default: - return err; - } - } - - private isHttpFastifyError( - error: any, - ): error is Error & { statusCode: number } { - // condition based on this code - https://github.com/fastify/fastify-error/blob/d669b150a82968322f9f7be992b2f6b463272de3/index.js#L22 - return ( - error.statusCode !== undefined && - error instanceof Error && - error.name === 'FastifyError' - ); - } - private getModulePathMetadata(metatype: Type): string | undefined { const modulesContainer = this.container.getModules(); const modulePath = Reflect.getMetadata( diff --git a/packages/core/test/router/routes-resolver.spec.ts b/packages/core/test/router/routes-resolver.spec.ts index cbf035255f0..d78498de424 100644 --- a/packages/core/test/router/routes-resolver.spec.ts +++ b/packages/core/test/router/routes-resolver.spec.ts @@ -1,10 +1,4 @@ -import { - BadRequestException, - HttpException, - Module, - Post, - VersioningType, -} from '@nestjs/common'; +import { Module, Post, VersioningType } from '@nestjs/common'; import { MODULE_PATH } from '@nestjs/common/constants'; import { expect } from 'chai'; import * as sinon from 'sinon'; @@ -18,7 +12,6 @@ import { GraphInspector } from '../../inspector/graph-inspector'; import { SerializedGraph } from '../../inspector/serialized-graph'; import { RoutesResolver } from '../../router/routes-resolver'; import { NoopHttpAdapter } from '../utils/noop-adapter.spec'; -import { createError as createFastifyError } from '@fastify/error'; describe('RoutesResolver', () => { @Controller('global') @@ -320,61 +313,6 @@ describe('RoutesResolver', () => { }); }); - describe('mapExternalExceptions', () => { - describe('when exception prototype is', () => { - describe('SyntaxError', () => { - it('should map to BadRequestException', () => { - const err = new SyntaxError(); - const outputErr = routesResolver.mapExternalException(err); - expect(outputErr).to.be.instanceof(BadRequestException); - }); - }); - describe('URIError', () => { - it('should map to BadRequestException', () => { - const err = new URIError(); - const outputErr = routesResolver.mapExternalException(err); - expect(outputErr).to.be.instanceof(BadRequestException); - }); - }); - describe('FastifyError', () => { - it('should map FastifyError with status code to HttpException', () => { - const FastifyErrorCls = createFastifyError( - 'FST_ERR_CTP_INVALID_MEDIA_TYPE', - 'Unsupported Media Type: %s', - 415, - ); - const error = new FastifyErrorCls(); - - const result = routesResolver.mapExternalException(error); - - expect(result).to.be.instanceOf(HttpException); - expect(result.message).to.equal(error.message); - expect(result.getStatus()).to.equal(415); - }); - - it('should return FastifyError without user status code to Internal Server Error HttpException', () => { - const FastifyErrorCls = createFastifyError( - 'FST_WITHOUT_STATUS_CODE', - 'Error without status code', - ); - const error = new FastifyErrorCls(); - - const result = routesResolver.mapExternalException(error); - expect(result).to.be.instanceOf(HttpException); - expect(result.message).to.equal(error.message); - expect(result.getStatus()).to.equal(500); - }); - }); - describe('other', () => { - it('should behave as an identity', () => { - const err = new Error(); - const outputErr = routesResolver.mapExternalException(err); - expect(outputErr).to.be.eql(err); - }); - }); - }); - }); - describe('registerNotFoundHandler', () => { it('should register not found handler', () => { routesResolver.registerNotFoundHandler(); diff --git a/packages/platform-express/adapters/express-adapter.ts b/packages/platform-express/adapters/express-adapter.ts index 7339d81caef..2d98443cc71 100644 --- a/packages/platform-express/adapters/express-adapter.ts +++ b/packages/platform-express/adapters/express-adapter.ts @@ -1,4 +1,5 @@ import { + BadRequestException, HttpStatus, InternalServerErrorException, Logger, @@ -448,6 +449,18 @@ export class ExpressAdapter extends AbstractHttpAdapter< throw new Error('Unsupported versioning options'); } + public mapException(error: unknown): unknown { + switch (true) { + // SyntaxError is thrown by Express body-parser when given invalid JSON (#422, #430) + // URIError is thrown by Express when given a path parameter with an invalid percentage + // encoding, e.g. '%FF' (#8915) + case error instanceof SyntaxError || error instanceof URIError: + return new BadRequestException(error.message); + default: + return error; + } + } + private trackOpenConnections() { this.httpServer.on('connection', (socket: Duplex) => { this.openConnections.add(socket); diff --git a/packages/platform-express/test/adapters/express-adapter.spec.ts b/packages/platform-express/test/adapters/express-adapter.spec.ts index 8970e117f9c..da3d27b0645 100644 --- a/packages/platform-express/test/adapters/express-adapter.spec.ts +++ b/packages/platform-express/test/adapters/express-adapter.spec.ts @@ -1,9 +1,16 @@ +import { BadRequestException } from '@nestjs/common'; import { ExpressAdapter } from '@nestjs/platform-express'; import { expect } from 'chai'; import * as express from 'express'; import * as sinon from 'sinon'; describe('ExpressAdapter', () => { + let expressAdapter: ExpressAdapter; + + beforeEach(() => { + expressAdapter = new ExpressAdapter(); + }); + afterEach(() => sinon.restore()); describe('registerParserMiddleware', () => { @@ -43,4 +50,24 @@ describe('ExpressAdapter', () => { expect(useSpy.called).to.be.false; }); }); + + describe('mapException', () => { + it('should map URIError with status code to BadRequestException', () => { + const error = new URIError(); + const result = expressAdapter.mapException(error) as BadRequestException; + expect(result).to.be.instanceOf(BadRequestException); + }); + + it('should map SyntaxError with status code to BadRequestException', () => { + const error = new SyntaxError(); + const result = expressAdapter.mapException(error) as BadRequestException; + expect(result).to.be.instanceOf(BadRequestException); + }); + + it('should return error if it is not handler Error', () => { + const error = new Error('Test error'); + const result = expressAdapter.mapException(error); + expect(result).to.equal(error); + }); + }); }); diff --git a/packages/platform-fastify/adapters/fastify-adapter.ts b/packages/platform-fastify/adapters/fastify-adapter.ts index 370a4e4c26f..de8d178d142 100644 --- a/packages/platform-fastify/adapters/fastify-adapter.ts +++ b/packages/platform-fastify/adapters/fastify-adapter.ts @@ -1,6 +1,8 @@ /* eslint-disable @typescript-eslint/no-floating-promises */ import { FastifyCorsOptions } from '@fastify/cors'; import { + BadRequestException, + HttpException, HttpStatus, Logger, RawBodyRequest, @@ -18,6 +20,7 @@ import { LegacyRouteConverter } from '@nestjs/core/router/legacy-route-converter import { FastifyBaseLogger, FastifyBodyParser, + FastifyError, FastifyInstance, FastifyListenOptions, FastifyPluginAsync, @@ -670,6 +673,25 @@ export class FastifyAdapter< return 'fastify'; } + public mapException(error: unknown): unknown { + if (this.isHttpFastifyError(error)) { + return new HttpException(error.message, error.statusCode); + } + + return error; + } + + private isHttpFastifyError( + error: any, + ): error is Error & { statusCode: number } { + // condition based on this code - https://github.com/fastify/fastify-error/blob/d669b150a82968322f9f7be992b2f6b463272de3/index.js#L22 + return ( + error.statusCode !== undefined && + error instanceof Error && + error.name === 'FastifyError' + ); + } + protected registerWithPrefix( factory: | FastifyPluginCallback diff --git a/packages/platform-fastify/test/adapters/fastify-adapter.spec.ts b/packages/platform-fastify/test/adapters/fastify-adapter.spec.ts new file mode 100644 index 00000000000..9fe99d14085 --- /dev/null +++ b/packages/platform-fastify/test/adapters/fastify-adapter.spec.ts @@ -0,0 +1,48 @@ +import { FastifyAdapter } from '../../adapters/fastify-adapter'; +import { expect } from 'chai'; +import { createError } from '@fastify/error'; +import { HttpException } from '@nestjs/common'; + +describe('FastifyAdapter', () => { + let fastifyAdapter: FastifyAdapter; + + beforeEach(() => { + fastifyAdapter = new FastifyAdapter(); + }); + + describe('mapException', () => { + it('should map FastifyError with status code to HttpException', () => { + const FastifyErrorCls = createError( + 'FST_ERR_CTP_INVALID_MEDIA_TYPE', + 'Unsupported Media Type: %s', + 415, + ); + const error = new FastifyErrorCls(); + + const result = fastifyAdapter.mapException(error) as HttpException; + + expect(result).to.be.instanceOf(HttpException); + expect(result.message).to.equal(error.message); + expect(result.getStatus()).to.equal(415); + }); + + it('should return FastifyError without user status code to Internal Server Error HttpException', () => { + const FastifyErrorCls = createError( + 'FST_WITHOUT_STATUS_CODE', + 'Error without status code', + ); + const error = new FastifyErrorCls(); + + const result = fastifyAdapter.mapException(error) as HttpException; + expect(result).to.be.instanceOf(HttpException); + expect(result.message).to.equal(error.message); + expect(result.getStatus()).to.equal(500); + }); + + it('should return error if it is not FastifyError', () => { + const error = new Error('Test error'); + const result = fastifyAdapter.mapException(error); + expect(result).to.equal(error); + }); + }); +});