Current implementation of HttpException and its child classes is great as it provides uniform and easy to use system. The issue arises when you try to copy that behaviour in a custom exception(e,g. 412 PreconditionFailed error) and try to copy the behaviour of provided exceptions. The process is tedious because the docs are outdated and then you take a look in the source code, and realise you don't have access to createHttpExceptionBody function so you can't just copy the constructor with different error code
IMO, that function should be exported so we can make our custom errors uniform with the built in ones without the need to create a function in our source.
Documentation should be updated to show a way built-in exceptions are constructed and show the usage of createHttpExceptionBody
Trying to make uniform HTTP exceptions
@iveselin In my opinion, a developer shouldn't need to create his/her custom HttpException since the HTTP status codes are always the same - for every application. Therefore I'd rather see PreconditionFailedException provided by NestJS out of the box - or am I missing something?
@BrunnerLivio that would be even nicer, but then you should add all of them (all that you can remember 馃槃 ). Otherwise for every exception someone needs, he would need to raise an issue, wait for the resoultion etc etc... Maybe middleground would be even better, add more common ones out of the box and export the createHttpExceptionBody function for the ones you have forgotten/chosed not to implement.
Interesting. createHttpExceptionBody was never supposed to be a part of the public API, but your use-case sounds quite interesting. We could theoretically expose it publicly, but I'm afraid that making the API more bloat will make it less straightforward for other devs. Any ideas?
Well, call me bias since I am already using it, I just copied the function and needed utils to my project and created PreconditionFailedException so it can be used until this discussion resolves.
import { HttpException, HttpStatus } from '@nestjs/common';
import { createHttpExceptionBody } from './exception.utils';
export class PreconditionFailedException extends HttpException {
constructor(message?: string | object | any, error = 'Precondition Failed') {
super(
createHttpExceptionBody(message, error, HttpStatus.PRECONDITION_FAILED),
HttpStatus.PRECONDITION_FAILED,
);
}
}
And this would be a great use-case, ability to create a missing exception and use it right away, not being forced to wait for new release of Nest etc. etc.
@BrunnerLivio 's proposal to create missing HTTP exceptions would, IMO, clog the API even more. You have covered most common exceptions and that is great, other exceptions are rare and if you really need them, you should be capable to create one for yourself. (Section in the docs about this would be great in this case)
proposal to create missing HTTP exceptions would, IMO, clog the API even more.
@iveselin Agree. There are way more HTTP status codes than I thought.
@kamilmysliwiec How about just making createHttpExceptionBody a member of HttpException?
Sounds good! @BrunnerLivio
Would you like to create a PR @iveselin, to move createHttpExceptionBody as a member of HttpException and update the dependent excpetions? :)
I won't make any promises, but I will try it over the weekend. Will keep you updated :upside_down_face:
and + we should also rename the name of this method then. Either createBody or createResponseBody should be enough
@iveselin Would you be able to either do a PR with a doc update? Alternatively, if you'd like to just sketch out the use case, and where you think we need to add documentation, I'd be happy to make the update. I get the general picture, but if you could sketch out a bit more detail of where you see it fitting into the docs, that would help.
I wont have much time in the next few weeks, so I will just add an example of usage.
Documentation should probably go under the HttpExceptions because this feature is meant to be used as a tool for making unimplemented exceptions that have uniform response as the built in ones.
We should also add the example that would look something like this:
export class NotFoundException extends HttpException {
constructor(message?: string | object | any, error = 'Not Found') {
super(
HttpException.createBody(message, error, HttpStatus.NOT_FOUND),
HttpStatus.NOT_FOUND,
);
}
}
@johnbiundo IMO, this is rather a sort of tip/trick on how to achieve similar behavior to the built-in exceptions. If we want to add this to the documentation, we shouldn't put it under Filters to don't overwhelm developers. I'd even say that it's very easy to find manually now, so if anyone decides to create its own set of exceptions, it should be pretty simple (just look at one the built-in exception source code). I'm not sure if we should add extra chapter just for that
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Most helpful comment
@johnbiundo IMO, this is rather a sort of tip/trick on how to achieve similar behavior to the built-in exceptions. If we want to add this to the documentation, we shouldn't put it under Filters to don't overwhelm developers. I'd even say that it's very easy to find manually now, so if anyone decides to create its own set of exceptions, it should be pretty simple (just look at one the built-in exception source code). I'm not sure if we should add extra chapter just for that