googleapis / nodejs-logging-winston

Node.js client integration between Stackdriver Logging and Winston.
https://cloud.google.com/logging/
Apache License 2.0
105 stars 50 forks source link

fix: Default callback is not called upon error #750

Closed losalex closed 1 year ago

losalex commented 1 year ago

Fixes #<732> 🦕

daniel-sanche commented 1 year ago

We talked about this offline but to summarize for posterity: I have concerns that this catch block could hide errors. If callback throws an exception and this.defaultCallback is set, the error is swallowed. That could result in logs not making it into Cloud Logging, with no warnings being shown to the user. You told me that this should not be a concern because of how Winston callbacks are structured, so I'm going to try to look more into Winston before finishing the review