Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): display class's name on request mapping exceptions #10479

Merged
7 changes: 7 additions & 0 deletions .gitignore
Expand Up @@ -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
Expand Down
@@ -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(metatype: Type) {
super(UNKNOWN_REQUEST_MAPPING(metatype));
}
}
8 changes: 7 additions & 1 deletion packages/core/errors/messages.ts
Expand Up @@ -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.`;
};

kamilmysliwiec marked this conversation as resolved.
Show resolved Hide resolved
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.)`;
2 changes: 1 addition & 1 deletion packages/core/router/router-explorer.ts
Expand Up @@ -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));
Expand Down
@@ -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';
Expand All @@ -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) {}
}

Expand Down Expand Up @@ -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 {}
Expand Down
20 changes: 14 additions & 6 deletions packages/core/test/exceptions/external-exceptions-handler.spec.ts
Expand Up @@ -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);
Expand Down Expand Up @@ -49,6 +50,7 @@ describe('ExternalExceptionsHandler', () => {
describe('when filters array is not empty', () => {
let filters, funcSpy;
class TestException {}
class AnotherTestException {}

beforeEach(() => {
funcSpy = sinon.spy();
Expand All @@ -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;
Expand Down
12 changes: 12 additions & 0 deletions packages/core/test/router/router-explorer.spec.ts
Expand Up @@ -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')
Expand Down Expand Up @@ -51,6 +52,8 @@ describe('RouterExplorer', () => {
public getTestUsingArray() {}
}

class ClassWithMissingControllerDecorator {}

let routerBuilder: RouterExplorer;
let injector: Injector;
let exceptionsFilter: RouterExceptionFilters;
Expand Down Expand Up @@ -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', () => {
Expand Down