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

Bugfix: Make errors serializable by default #2512

Merged
merged 7 commits into from
Jul 9, 2019

Conversation

csidell-earny
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ X ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

When trying to print the error it resolves to 'Error: [object Object]' which is unhelpful and means that the error objects are not serializable.

What is the new behavior?

This adds a toString() function to HttpException that will serialize only when the message is an object. It does not modify anything other than serialization of the error (which there was none)

Does this PR introduce a breaking change?

[ ] Yes
[ X ] No

The only potential breaking change is if some library somewhere expects the error to return [object Object] which would be an invalid use of this error.

@csidell-earny
Copy link
Contributor Author

This would be potentially useful for RpcException or WsException which I can add on to this PR if necessary.

@coveralls
Copy link

coveralls commented Jul 4, 2019

Pull Request Test Coverage Report for Build 3488

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.007%) to 95.157%

Totals Coverage Status
Change from base Build 3467: 0.007%
Covered Lines: 3537
Relevant Lines: 3717

💛 - Coveralls

@kamilmysliwiec
Copy link
Member

Thanks for your contribution! I left a few comments, could you look at them? :)

@csidell-earny
Copy link
Contributor Author

Sure, made the changes recommended by your comments.

@kamilmysliwiec
Copy link
Member

Thank you!

@kamilmysliwiec kamilmysliwiec merged commit 9870126 into nestjs:master Jul 9, 2019
@lock
Copy link

lock bot commented Oct 7, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants