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

should try/catch on JSON.stringify #9

Closed
cainus opened this issue Jan 24, 2013 · 2 comments
Closed

should try/catch on JSON.stringify #9

cainus opened this issue Jan 24, 2013 · 2 comments

Comments

@cainus
Copy link

cainus commented Jan 24, 2013

Currently if meta has circular references, logging breaks with some nasty infinite recursion. I think this is probably worth fixing at least for the fact that logging shouldn't bring down an application, no matter how dumb the developer was for passing in a self-referencing object (and yes, I was that dumb).

I'd just do something like this at the top of Syslog.prototype.log():

if (meta){
try {
meta = JSON.stringify(data);
} catch(ex){
meta = " NOTE: Could not stringify meta: " + JSON.stringify(ex);
}
}

...but you might have a smarter way. It seems like the problem could manifest in winston.clone(), so you might want to fix this in winston itself instead, but this fix makes the otherwise unsafe stringify() on 119 safe too.

@chaudhryjunaid
Copy link

There's also this:
https://github.com/isaacs/json-stringify-safe

@santigimeno
Copy link
Member

It doesn't seem we're stringifying meta anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants