From 21687ea4e529d2aacf4c10ae01b7075cf6746d9d Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Sat, 29 Oct 2022 14:56:59 -0400 Subject: [PATCH 1/7] feat(core): display class's name on request mapping exceptions --- .../exceptions/unknown-request-mapping.exception.ts | 5 +++-- packages/core/errors/messages.ts | 8 +++++++- packages/core/router/router-explorer.ts | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/core/errors/exceptions/unknown-request-mapping.exception.ts b/packages/core/errors/exceptions/unknown-request-mapping.exception.ts index b0d64b2128c..310415f5ab8 100644 --- a/packages/core/errors/exceptions/unknown-request-mapping.exception.ts +++ b/packages/core/errors/exceptions/unknown-request-mapping.exception.ts @@ -1,8 +1,9 @@ +import type { Type } from '@nestjs/common'; import { RuntimeException } from './runtime.exception'; import { UNKNOWN_REQUEST_MAPPING } from '../messages'; export class UnknownRequestMappingException extends RuntimeException { - constructor() { - super(UNKNOWN_REQUEST_MAPPING); + constructor(metatypeWrongPlaced: Type) { + super(UNKNOWN_REQUEST_MAPPING(metatypeWrongPlaced)); } } diff --git a/packages/core/errors/messages.ts b/packages/core/errors/messages.ts index b0a26e8d1d3..b80ba4321b0 100644 --- a/packages/core/errors/messages.ts +++ b/packages/core/errors/messages.ts @@ -172,8 +172,14 @@ export const INVALID_CLASS_SCOPE_MESSAGE = ( name || 'This class' } is marked as a scoped provider. Request and transient-scoped providers can't be used in combination with "get()" method. Please, use "resolve()" instead.`; +export const UNKNOWN_REQUEST_MAPPING = (metatypeWrongPlaced: Type) => { + const className = metatypeWrongPlaced.name; + return className + ? `An invalid controller has been detected. "${className}" do not have the @Controller() decorator but it is being listed in the controllers array of some module.` + : `An invalid controller has been detected. Perhaps, one of your controllers is missing @Controller() decorator.`; +}; + export const INVALID_MIDDLEWARE_CONFIGURATION = `An invalid middleware configuration has been passed inside the module 'configure()' method.`; -export const UNKNOWN_REQUEST_MAPPING = `An invalid controller has been detected. Perhaps, one of your controllers is missing @Controller() decorator.`; export const UNHANDLED_RUNTIME_EXCEPTION = `Unhandled Runtime Exception.`; export const INVALID_EXCEPTION_FILTER = `Invalid exception filters (@UseFilters()).`; export const MICROSERVICES_PACKAGE_NOT_FOUND_EXCEPTION = `Unable to load @nestjs/microservices package. (Please make sure that it's already installed.)`; diff --git a/packages/core/router/router-explorer.ts b/packages/core/router/router-explorer.ts index 92f1915be44..b158fd18151 100644 --- a/packages/core/router/router-explorer.ts +++ b/packages/core/router/router-explorer.ts @@ -116,7 +116,7 @@ export class RouterExplorer { const path = Reflect.getMetadata(PATH_METADATA, metatype); if (isUndefined(path)) { - throw new UnknownRequestMappingException(); + throw new UnknownRequestMappingException(metatype); } if (Array.isArray(path)) { return path.map(p => addLeadingSlash(p)); From 9e8094d33efa59fe0218d2a0b4fae7a53f8e1f75 Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Sat, 29 Oct 2022 15:13:54 -0400 Subject: [PATCH 2/7] test(core): cover missing metadata on controller class scneario --- packages/core/test/router/router-explorer.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/core/test/router/router-explorer.spec.ts b/packages/core/test/router/router-explorer.spec.ts index daeff71f93b..cf0d49cb949 100644 --- a/packages/core/test/router/router-explorer.spec.ts +++ b/packages/core/test/router/router-explorer.spec.ts @@ -19,6 +19,7 @@ import { RoutePathFactory } from '../../router/route-path-factory'; import { RouterExceptionFilters } from '../../router/router-exception-filters'; import { RouterExplorer } from '../../router/router-explorer'; import { NoopHttpAdapter } from '../utils/noop-adapter.spec'; +import { UnknownRequestMappingException } from '../../errors/exceptions/unknown-request-mapping.exception'; describe('RouterExplorer', () => { @Controller('global') @@ -51,6 +52,8 @@ describe('RouterExplorer', () => { public getTestUsingArray() {} } + class ClassWithMissingControllerDecorator {} + let routerBuilder: RouterExplorer; let injector: Injector; let exceptionsFilter: RouterExceptionFilters; @@ -315,6 +318,15 @@ describe('RouterExplorer', () => { '/global-alias', ]); }); + + it("should throw UnknownRequestMappingException when missing the `@Controller()` decorator in the class, displaying class's name", () => { + expect(() => + routerBuilder.extractRouterPath(ClassWithMissingControllerDecorator), + ).to.throw( + UnknownRequestMappingException, + /ClassWithMissingControllerDecorator/, + ); + }); }); describe('createRequestScopedHandler', () => { From 8af21d1197e817a9265b3e757cabfe877679231b Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Sat, 29 Oct 2022 15:15:34 -0400 Subject: [PATCH 3/7] chore: ignore from git swap files created by vim --- .gitignore | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.gitignore b/.gitignore index 5943850da34..b256a3db1e4 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,13 @@ node_modules/ /.devcontainer *.code-workspace +# Vim +[._]*.s[a-v][a-z] +[._]*.sw[a-p] +[._]s[a-rt-v][a-z] +[._]ss[a-gi-z] +[._]sw[a-p] + # bundle packages/**/*.d.ts packages/**/*.js From 1d8c56a77f9035dcdcc3e00bbd51bcbc64e6e39f Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Sat, 29 Oct 2022 18:21:53 -0400 Subject: [PATCH 4/7] test(core): fix scenario on external exceptions handler --- .../external-exceptions-handler.spec.ts | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/core/test/exceptions/external-exceptions-handler.spec.ts b/packages/core/test/exceptions/external-exceptions-handler.spec.ts index 8f39cd76f47..d488029ebce 100644 --- a/packages/core/test/exceptions/external-exceptions-handler.spec.ts +++ b/packages/core/test/exceptions/external-exceptions-handler.spec.ts @@ -2,22 +2,23 @@ import { expect } from 'chai'; import { of } from 'rxjs'; import * as sinon from 'sinon'; import { ExternalExceptionsHandler } from '../../exceptions/external-exceptions-handler'; +import { ExternalExceptionFilter } from '../../exceptions/external-exception-filter'; describe('ExternalExceptionsHandler', () => { let handler: ExternalExceptionsHandler; beforeEach(() => { handler = new ExternalExceptionsHandler(); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore The 'logger' property is private but we want to avoid showing useless error logs + ExternalExceptionFilter.logger.error = () => {}; }); describe('next', () => { - it('should method returns expected stream with message when exception is unknown', async () => { + it('should method returns expected stream with message when exception is unknown', () => { const error = new Error(); - try { - await handler.next(error, null); - } catch (err) { - expect(err).to.be.eql(error); - } + expect(() => handler.next(error, null)).to.throw(error); }); describe('when "invokeCustomFilters" returns value', () => { const observable$ = of(true); @@ -49,6 +50,7 @@ describe('ExternalExceptionsHandler', () => { describe('when filters array is not empty', () => { let filters, funcSpy; class TestException {} + class AnotherTestException {} beforeEach(() => { funcSpy = sinon.spy(); @@ -73,6 +75,12 @@ describe('ExternalExceptionsHandler', () => { }); }); describe('when filter does not exists in filters array', () => { + beforeEach(() => { + filters = [ + { exceptionMetatypes: [AnotherTestException], func: funcSpy }, + ]; + (handler as any).filters = filters; + }); it('should not call funcSpy', () => { handler.invokeCustomFilters(new TestException(), null); expect(funcSpy.notCalled).to.be.true; From 2fe543dbae9d83638a984a1f9188d34e021ac278 Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Sun, 30 Oct 2022 15:32:11 -0400 Subject: [PATCH 5/7] test(core): cover the scenario where the `@Catch()` is missing --- .../external-exception-filter-context.spec.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/core/test/exceptions/external-exception-filter-context.spec.ts b/packages/core/test/exceptions/external-exception-filter-context.spec.ts index 755ec302165..5c849fc88f4 100644 --- a/packages/core/test/exceptions/external-exception-filter-context.spec.ts +++ b/packages/core/test/exceptions/external-exception-filter-context.spec.ts @@ -1,5 +1,6 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; +import { ExceptionFilter } from '../../../common'; import { Catch } from '../../../common/decorators/core/catch.decorator'; import { UseFilters } from '../../../common/decorators/core/exception-filters.decorator'; import { ApplicationConfig } from '../../application-config'; @@ -13,7 +14,10 @@ describe('ExternalExceptionFilterContext', () => { class CustomException {} @Catch(CustomException) - class ExceptionFilter { + class ExceptionFilter implements ExceptionFilter { + public catch(exc, res) {} + } + class ClassWithNoMetadata implements ExceptionFilter { public catch(exc, res) {} } @@ -59,6 +63,11 @@ describe('ExternalExceptionFilterContext', () => { exceptionFilter.reflectCatchExceptions(new ExceptionFilter()), ).to.be.eql([CustomException]); }); + it('should return an empty array when metadata was found', () => { + expect( + exceptionFilter.reflectCatchExceptions(new ClassWithNoMetadata()), + ).to.be.eql([]); + }); }); describe('createConcreteContext', () => { class InvalidFilter {} From 4d3f283d662308b34e5d7306e4bef63feb758ea4 Mon Sep 17 00:00:00 2001 From: Kamil Mysliwiec Date: Mon, 7 Nov 2022 10:42:17 +0100 Subject: [PATCH 6/7] Update packages/core/errors/exceptions/unknown-request-mapping.exception.ts --- .../errors/exceptions/unknown-request-mapping.exception.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/errors/exceptions/unknown-request-mapping.exception.ts b/packages/core/errors/exceptions/unknown-request-mapping.exception.ts index 310415f5ab8..561390bd943 100644 --- a/packages/core/errors/exceptions/unknown-request-mapping.exception.ts +++ b/packages/core/errors/exceptions/unknown-request-mapping.exception.ts @@ -3,7 +3,7 @@ import { RuntimeException } from './runtime.exception'; import { UNKNOWN_REQUEST_MAPPING } from '../messages'; export class UnknownRequestMappingException extends RuntimeException { - constructor(metatypeWrongPlaced: Type) { - super(UNKNOWN_REQUEST_MAPPING(metatypeWrongPlaced)); + constructor(metatype: Type) { + super(UNKNOWN_REQUEST_MAPPING(metatype)); } } From ee82c7bf2e04a346248d1c452f453020b670efb2 Mon Sep 17 00:00:00 2001 From: Kamil Mysliwiec Date: Mon, 7 Nov 2022 10:43:37 +0100 Subject: [PATCH 7/7] Update packages/core/errors/messages.ts --- packages/core/errors/messages.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/errors/messages.ts b/packages/core/errors/messages.ts index b80ba4321b0..2d327280ef6 100644 --- a/packages/core/errors/messages.ts +++ b/packages/core/errors/messages.ts @@ -172,11 +172,11 @@ export const INVALID_CLASS_SCOPE_MESSAGE = ( name || 'This class' } is marked as a scoped provider. Request and transient-scoped providers can't be used in combination with "get()" method. Please, use "resolve()" instead.`; -export const UNKNOWN_REQUEST_MAPPING = (metatypeWrongPlaced: Type) => { - const className = metatypeWrongPlaced.name; +export const UNKNOWN_REQUEST_MAPPING = (metatype: Type) => { + const className = metatype.name; return className - ? `An invalid controller has been detected. "${className}" do not have the @Controller() decorator but it is being listed in the controllers array of some module.` - : `An invalid controller has been detected. Perhaps, one of your controllers is missing @Controller() decorator.`; + ? `An invalid controller has been detected. "${className}" does not have the @Controller() decorator but it is being listed in the "controllers" array of some module.` + : `An invalid controller has been detected. Perhaps, one of your controllers is missing the @Controller() decorator.`; }; export const INVALID_MIDDLEWARE_CONFIGURATION = `An invalid middleware configuration has been passed inside the module 'configure()' method.`;