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

Catch JSON stringily errors. #195

Merged
merged 1 commit into from
Nov 14, 2016
Merged

Conversation

jalleyne
Copy link
Contributor

@jalleyne jalleyne commented Apr 8, 2015

There is a chance that there may be circular references in the Object which will cause the debug message to throw an exception. If the exception is not handled on the application level this will cause node to crash. As a debug library, errors in here should not have such a serious impact on the application.

There is a chance that there may be circular references in the Object which will cause the debug message to throw an exception. If the exception is not handled on the application level this will cause node to crash. As a debug library, errors in here should not have such a serious impact on the application.
@stephenmathieson
Copy link
Contributor

imo, this falls out of scope for this module.. see what nate says though

@jalleyne
Copy link
Contributor Author

jalleyne commented Apr 9, 2015

Completely agree that argument validation is out of scope, but dont think its a good idea that debug code can throw unexpected errors. This is sort of an odd case.

@ronkorving
Copy link

Since it's debug doing the stringify operation, it's its responsibility not to throw. Node.js's own console.log deals with this gracefully by mentioning circular references explicitly. I think that's the only correct way to deal with this.

@jalleyne
Copy link
Contributor Author

Any updates/ further thoughts on this?

@world
Copy link

world commented May 30, 2016

I haven't had this happen yet but would be worried if node crashed from debug.

@TooTallNate
Copy link
Contributor

👍

@thebigredgeek
Copy link
Contributor

Looks good to me! Merging

@thebigredgeek thebigredgeek added bug This issue identifies a malfunction change-patch This proposes or provides a change that requires a patch release labels Nov 14, 2016
@thebigredgeek thebigredgeek merged commit 3491ad6 into debug-js:master Nov 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue identifies a malfunction change-patch This proposes or provides a change that requires a patch release
Development

Successfully merging this pull request may close these issues.

6 participants