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
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
123 changes: 64 additions & 59 deletions packages/common/test/exceptions/http.exception.spec.ts
Expand Up @@ -6,51 +6,59 @@ import {
} from '../../exceptions';

describe('HttpException', () => {
it('should return a response as a string when input is a string', () => {
const message = 'My error message';
expect(new HttpException(message, 404).getResponse()).to.be.eql(
'My error message',
);
});

it('should return a response as an object when input is an object', () => {
const message = {
msg: 'My error message',
reason: 'this can be a human readable reason',
anything: 'else',
};
expect(new HttpException(message, 404).getResponse()).to.be.eql(message);
});
describe('getResponse', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here, I decided to group the related tests together under a common describe block. Many tests were actually testing the result for getResponse() method, while others were related to serialization. This aggregation made it easier for me to understand what was being tested. If you have strong feelings against it @kamilmysliwiec , let me know and I can revert it. However, this separation helped me notice there was a duplicated test for serialization. the test under the otherwise describe block was basically the same as the it('should be serializable', () => { }. So, in conclusion, I think this was a positive change.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good!

it('should return a response as a string when input is a string', () => {
const message = 'My error message';
expect(new HttpException(message, 404).getResponse()).to.be.eql(
'My error message',
);
});

it('should return a message from a built-in exception as an object', () => {
const message = 'My error message';
expect(new BadRequestException(message).getResponse()).to.be.eql({
statusCode: 400,
error: 'Bad Request',
message: 'My error message',
it('should return a response as an object when input is an object', () => {
const message = {
msg: 'My error message',
reason: 'this can be a human readable reason',
anything: 'else',
};
expect(new HttpException(message, 404).getResponse()).to.be.eql(message);
});
});

it('should return an object even when the message is undefined', () => {
expect(new BadRequestException().getResponse()).to.be.eql({
statusCode: 400,
message: 'Bad Request',
it('should return a message from a built-in exception as an object', () => {
const message = 'My error message';
expect(new BadRequestException(message).getResponse()).to.be.eql({
statusCode: 400,
error: 'Bad Request',
message: 'My error message',
});
});
});

it('should return a status code', () => {
expect(new BadRequestException().getStatus()).to.be.eql(400);
expect(new NotFoundException().getStatus()).to.be.eql(404);
it('should return an object even when the message is undefined', () => {
expect(new BadRequestException().getResponse()).to.be.eql({
statusCode: 400,
message: 'Bad Request',
});
});
});

it('should return a response', () => {
expect(new BadRequestException().getResponse()).to.be.eql({
message: 'Bad Request',
statusCode: 400,
describe('built-in exceptions', () => {
describe('getStatus', () => {
it('should return given status code', () => {
expect(new BadRequestException().getStatus()).to.be.eql(400);
expect(new NotFoundException().getStatus()).to.be.eql(404);
});
});
expect(new NotFoundException().getResponse()).to.be.eql({
message: 'Not Found',
statusCode: 404,

describe('getResponse', () => {
it('should return a response with default message and status code', () => {
expect(new BadRequestException().getResponse()).to.be.eql({
message: 'Bad Request',
statusCode: 400,
});
expect(new NotFoundException().getResponse()).to.be.eql({
message: 'Not Found',
statusCode: 404,
});
});
});
});

Expand All @@ -59,31 +67,28 @@ describe('HttpException', () => {
expect(error instanceof Error).to.be.true;
});

it('should be serializable', () => {
const message = 'Some Error';
const error = new HttpException(message, 400);
expect(`${error}`).to.be.eql(`HttpException: ${message}`);
});
describe('when serializing', () => {
describe('and "response" parameter is a string', () => {
it('should concatenate HttpException with the given message', () => {
const responseAsString = 'Some Error';
const error = new HttpException(responseAsString, 400);
expect(`${error}`).to.be.eql(`HttpException: ${responseAsString}`);
expect(`${error}`.includes('[object Object]')).to.not.be.true;
});
});

describe('when "response" is an object', () => {
it('should use default message', () => {
const obj = { foo: 'bar' };
const error = new HttpException(obj, 400);
const badRequestError = new BadRequestException(obj);
describe('and "response" parameter is an object', () => {
it('should use default message', () => {
const responseAsObject = { foo: 'bar' };
const error = new HttpException(responseAsObject, 400);
const badRequestError = new BadRequestException(responseAsObject);

expect(`${error}`).to.be.eql(`HttpException: Http Exception`);
expect(`${badRequestError}`).to.be.eql(
`BadRequestException: Bad Request Exception`,
);
expect(`${error}`.includes('[object Object]')).to.not.be.true;
expect(`${badRequestError}`.includes('[object Object]')).to.not.be.true;
});
describe('otherwise', () => {
it('should concat strings', () => {
const test = 'test message';
const error = new HttpException(test, 400);
expect(`${error}`).to.be.eql(`HttpException: ${test}`);
expect(`${error}`).to.be.eql(`HttpException: Http Exception`);
expect(`${badRequestError}`).to.be.eql(
`BadRequestException: Bad Request Exception`,
);
expect(`${error}`.includes('[object Object]')).to.not.be.true;
expect(`${badRequestError}`.includes('[object Object]')).to.not.be.true;
});
});
});
Expand Down