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(common): add error options object #10460

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
b5fe06d
feat(common): add error options object
thiagomini Oct 25, 2022
f5edb0f
chore(common): add deprecated warning
thiagomini Oct 26, 2022
587fbae
feat(common): add error cause option
thiagomini Oct 26, 2022
071c4c5
refactor(common): change parameter assignment
thiagomini Oct 26, 2022
d63b6df
refactor(common): rearrange test blocks
thiagomini Oct 26, 2022
7e763fa
refactor(common): rename method parameter
thiagomini Oct 26, 2022
1a8ada5
test(common): add http exception test
thiagomini Oct 26, 2022
4ded779
refactor(common): add description attribute
thiagomini Oct 27, 2022
c5c818e
chore(common): change deprecated message
thiagomini Oct 27, 2022
60cb953
feat(common): add error cause option
thiagomini Oct 27, 2022
7a71035
refactor(common): extract description and options
thiagomini Oct 27, 2022
a4a8557
refactor(common): extract description and options
thiagomini Oct 27, 2022
5a186af
docs(common): add static method docs
thiagomini Oct 27, 2022
86c5084
test(common): test error cause option
thiagomini Oct 27, 2022
472c711
test(common): aggregate exception tests
thiagomini Oct 27, 2022
88ac6aa
feat(common): add error cause option
thiagomini Oct 27, 2022
1dff9c0
docs(common): update exception docs
thiagomini Oct 27, 2022
c3ced1d
feat(common): add error cause option
thiagomini Oct 27, 2022
f629dc1
feat(common): add error cause option
thiagomini Oct 27, 2022
bd7daf1
feat(common): add error cause option
thiagomini Oct 27, 2022
0328a5c
feat(common): add error cause option
thiagomini Oct 27, 2022
11d2763
feat(common): add error cause option
thiagomini Oct 27, 2022
b815143
feat(common): add error cause option
thiagomini Oct 27, 2022
789e99c
feat(common): add error cause option
thiagomini Oct 27, 2022
cd44ff3
feat(common): add error cause option
thiagomini Oct 27, 2022
3a10258
feat(common): add error cause option
thiagomini Oct 27, 2022
d569aaa
feat(common): add error cause option
thiagomini Oct 27, 2022
2416df6
feat(common): add error cause option
thiagomini Oct 27, 2022
dfc28fc
feat(common): add error cause option
thiagomini Oct 27, 2022
d40ebd1
feat(common): add error cause option
thiagomini Oct 27, 2022
5031f41
feat(common): add error cause option
thiagomini Oct 27, 2022
e8abd9a
feat(common): add error cause option
thiagomini Oct 27, 2022
f62acac
feat(common): add error cause option
thiagomini Oct 27, 2022
b4aca5d
feat(common): add error cause option
thiagomini Oct 27, 2022
379b82c
feat(common): add error cause option
thiagomini Oct 27, 2022
971ce5f
test(common): reuse error cause variable
thiagomini Oct 27, 2022
45b6cc1
test(common): add http exception tests
thiagomini Oct 27, 2022
ab881e3
test(common): add http exception tests
thiagomini Oct 27, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 25 additions & 4 deletions packages/common/exceptions/http.exception.ts
@@ -1,5 +1,9 @@
import { isObject, isString } from '../utils/shared.utils';

export type HttpExceptionOptions = {
thiagomini marked this conversation as resolved.
Show resolved Hide resolved
cause?: Error;
};

/**
* Defines the base Nest HTTP exception, which is handled by the default
* Exceptions Handler.
Expand All @@ -13,12 +17,22 @@ export class HttpException extends Error {
* Instantiate a plain HTTP Exception.
*
* @example
* `throw new HttpException()`
* throw new HttpException()
* throw new HttpException('message', HttpStatus.BAD_REQUEST)
* throw new HttpException({ reason: 'this can be a human readable reason' }, HttpStatus.BAD_REQUEST)
* throw new HttpException(new Error('Cause Error'), HttpStatus.BAD_REQUEST)
* throw new HttpException('custom message', HttpStatus.BAD_REQUEST, {
* cause: new Error('Cause Error'),
* })
Comment on lines +27 to +33
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a bunch of examples to better illustrate the possibilities here.

*
*
* @usageNotes
* The constructor arguments define the response and the HTTP response status code.
* - The `response` argument (required) defines the JSON response body.
* - The `response` argument (required) defines the JSON response body. alternatively, it can also be
* an error object that is used to define an error [cause](https://nodejs.org/en/blog/release/v16.9.0/#error-cause).
* - The `status` argument (required) defines the HTTP Status Code.
* - The `options` argument (optional) defines additional error options. Currently, it supports the `cause` attribute,
* and can be used as an alternative way to specify the error cause: `const error = new HttpException('description', 400, { cause: new Error() });`
*
* By default, the JSON response body contains two properties:
* - `statusCode`: the Http Status Code.
Expand All @@ -31,12 +45,14 @@ export class HttpException extends Error {
* The `status` argument is required, and should be a valid HTTP status code.
* Best practice is to use the `HttpStatus` enum imported from `nestjs/common`.
*
* @param response string or object describing the error condition.
* @param response string, object describing the error condition or the error cause.
* @param status HTTP response status code.
* @param options An object used to add an error cause.
*/
constructor(
private readonly response: string | Record<string, any>,
private readonly status: number,
private readonly options?: HttpExceptionOptions,
) {
super();
this.initMessage();
Expand All @@ -53,7 +69,12 @@ export class HttpException extends Error {
* - https://nodejs.org/en/blog/release/v16.9.0/#error-cause
* - https://github.com/microsoft/TypeScript/issues/45167
*/
public initCause() {
public initCause(): void {
if (this.options?.cause) {
this.cause = this.options.cause;
return;
}

if (this.response instanceof Error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have yet to define what is the expected behavior when someone mistakenly passes both the first argument as an Error and also the options object with a cause. In that case, should we always consider the cause property or throw an error, @kamilmysliwiec ?

Following up on this ☝️ here.

Since we now have a dedicated options object, perhaps we can just log the warning down when this.response instanceof Error === true saying that from now on, the { cause: ... } option should be used instead (and the previous syntax is deprecated)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we could do that, but then this would be a breaking change to the users that are already using the first parameter as the error cause. Is it okay to break this interface and just log a warning?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thiagomini the idea is to log the warning now and introduce a breaking change as part of the next major release

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm I see, alright then, I'll keep both options and just log the deprecated message.

this.cause = this.response;
}
Expand Down
14 changes: 14 additions & 0 deletions packages/common/test/exceptions/http.exception.spec.ts
Expand Up @@ -139,5 +139,19 @@ describe('HttpException', () => {

expect(cause).to.be.eql(message);
});

it('configures a cause when message is a string and the options object is passed', () => {
const causeError = new Error('Some Error');

const customDescription = 'custom description';
const error = new HttpException(customDescription, 400, {
cause: causeError,
});

expect(`${error}`).to.be.eql(`HttpException: ${customDescription}`);
const { cause } = error;

expect(cause).to.be.eql(causeError);
});
});
});