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

Logging error handling #3

Open
santiagoaguiar opened this issue Aug 3, 2015 · 3 comments
Open

Logging error handling #3

santiagoaguiar opened this issue Aug 3, 2015 · 3 comments

Comments

@santiagoaguiar
Copy link

It seems that now to handle errors during logging we need to define an onError function for the stream object.

IMO, it should provide a default onError function that swallow errors, since logging should never make your application to crash, which is the current behavior if you haven't defined this function.

Also, maybe since the stream inherits from Writeable, error handling should be performed by using a stream.on('error', function(err) {...}) instead of using this onError function. Or otherwise, we could pass the error function as part of the options. It doesn't feel right to have to define that function directly over the stream.

Opinions?

@jerroydmoore
Copy link

+1 on using stream.on('error', (err) => {...})

README.md states this is a Writable stream, so this module should implement that interface instead of defining its own. Additionally onError is not documented in the README.md.

@ubershmekel
Copy link

I just got a crash for lack of connectivity too. Logging should not kill servers.

@shawnbissell
Copy link

This just happened to us as well when we hit a ThrottlingException. We ended up needing to wrap the logging calls in a domain to avoid the error from blowing up the Express web server. There needs to be a documented best practice and examples on how to handled these types of errors.

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

4 participants