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

Save HTTP exception description as a property of HttpException #5809

Closed
egormkn opened this issue Nov 27, 2020 · 5 comments
Closed

Save HTTP exception description as a property of HttpException #5809

egormkn opened this issue Nov 27, 2020 · 5 comments
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@egormkn
Copy link

egormkn commented Nov 27, 2020

Feature Request

Is your feature request related to a problem? Please describe.

I have an issue when implementing a custom ExceptionFilter for an MVC application. When the exception filter catches a HttpException, I want to render a template with HTTP error code, description, and message (e.g. 401 Unauthorized: Wrong username or password). While I can easily get error code and message, getting the HTTP error description (Unauthorized in this case) is harder than it should be: it is stored in the error property of JSON response. At the same time HttpException has a name property, that is always initialized to 'Error'.

Describe the solution you'd like

I'd like the name property of HttpException to be initialized with HTTP error description.

Teachability, Documentation, Adoption, Migration Strategy

Here is how it can be used:

import { Catch, ArgumentsHost, HttpStatus, Logger, ExceptionFilter, HttpException } from '@nestjs/common';
import { Request, Response } from 'express';

@Catch()
export class DefaultExceptionFilter implements ExceptionFilter<any> {
  private readonly logger = new Logger('ExceptionHandler');

  catch(exception: any, host: ArgumentsHost) {
    const ctx = host.switchToHttp();
    const req = ctx.getRequest<Request>();
    const res = ctx.getResponse<Response>();
                         // now it is always equal to 'Error' ---v
    const name = exception instanceof HttpException ? exception.name : 'Internal Server Error';
    const message = exception instanceof HttpException ? exception.message : '';
    const status =
      exception instanceof HttpException ? exception.getStatus() : HttpStatus.INTERNAL_SERVER_ERROR;

    res.status(status).render('error', { name, message, status });
    if (!(exception instanceof HttpException)) {
      return 'message' in exception
        ? this.logger.error(exception.message, exception.stack)
        : this.logger.error(exception);
    }
  }
}
@egormkn egormkn added needs triage This issue has not been looked into type: enhancement 🐺 labels Nov 27, 2020
@kamilmysliwiec
Copy link
Member

I'd like the name property of HttpException to be initialized with HTTP error description.

name property should be equal to the name of the class. Currently it's always Error (which is incorrect). I'll fix it shortly.

In the meantime, you can use this:

Object.getPrototypeOf(exception).constructor.name

as w workaround

@egormkn
Copy link
Author

egormkn commented Nov 27, 2020

I think that it would be nice to be able to get a human-readable description of an error (Not Found instead of NotFoundException). While I can get it from the error property of a JSON response (that might not even be there) or derive by myself from status code, this doesn't seem to be the right way.
I thought about adding an optional description argument to the HttpException constructor, that will save description from its subclasses.

@egormkn egormkn changed the title Save http exception description as exception.name Save HTTP exception description as a property of HttpException Nov 27, 2020
DrPuneetGaur added a commit to DrPuneetGaur/nest that referenced this issue Dec 4, 2020
Added a method to set name property of exception object equal to class name which currently is always Error

Closes nestjs#5809
DrPuneetGaur added a commit to DrPuneetGaur/nest that referenced this issue Dec 4, 2020
Added a method to set name property of exception object equal to class name which currently is always Error. Updated tests.

Closes nestjs#5809
DrPuneetGaur added a commit to DrPuneetGaur/nest that referenced this issue Dec 4, 2020
Added a method to set name property of exception object equal to class name which currently is always Error. Updated tests.

Closes nestjs#5809
@kamilmysliwiec
Copy link
Member

You could always generate a human-readable description based on the serialized class names (e.g., by adding spaces between uppercased characters). Let's track this here #5859

@JaapWeijland
Copy link

JaapWeijland commented Feb 15, 2021

Hi there, sorry to resurrect this issue, but I think I experience something that is related and thus not yet fixed.

I have two files, a service and a controller. In the service, I perform an API call. When this API call returns a response indicating that I am unauthorized, I throw a new UnauthorizedException(). The controller catches all errors thrown by the service, checks them and if needed, complements them. An UnauthorizedException may be rethrown as is, so I do the following to check whether the error is an UnauthorizedException:

try {
...
} catch (error) {
    if (error instanceof UnauthorizedException) throw error;
    throw new InternalServerErrorException(error);
}

However, error instanceof UnauthorizedException always returns false. I did some research and stumbled upon the following FAQ about Typescript: https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work:

As part of substituting the value of this with the value returned by a super(...) call, subclassing Error, Array, and others may no longer work as expected.

...and:

instanceof will be broken between instances of the subclass and their instances, so (new FooError()) instanceof FooError will return false.

Since UnauthorizedException is an indirect subclass of Error, I think we're dealing with the same kind of issue here, namely that the prototype of the error is incorrectly set. Is this correct?

@kamilmysliwiec
Copy link
Member

Please, use our Discord channel (support) for such questions. We are using GitHub to track bugs, feature requests, and potential improvements.

@nestjs nestjs locked and limited conversation to collaborators Feb 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs triage This issue has not been looked into type: enhancement 🐺
Projects
None yet
Development

No branches or pull requests

3 participants