fix: Move axios error processing in LoggerService#122
Conversation
Services might catch the error and just log it, so it would not be propagated at top level and pretty-printed. Extending LoggerService takes care of those use cases.
| } | ||
|
|
||
| this.logger.log('error', message, { error }); | ||
| this.logger.log('error', message, { error: this.processMessage(error) }); |
There was a problem hiding this comment.
I think the naming is a bit confusing. We have a method that takes error and message, but its error that we pass to another method called processMessage
There was a problem hiding this comment.
I agree. How would you name the method?
There was a problem hiding this comment.
🤣 erm .... well ... maybe just have a processError method. And check it's an error first.
Like, would we ever want to process a string really?
There was a problem hiding this comment.
we need the same functionality for logger.info as it is reasonable a function might call it with an axios error (I know this happens in a couple of places)
There was a problem hiding this comment.
Yeah, am aware of that ... was envisaging the check would happen there as well.
tomisaacmarshall
left a comment
tomisaacmarshall
left a comment
There was a problem hiding this comment.
Approving as the naming point is just a suggestion really.
|
🎉 This PR is included in version 1.2.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
Services might catch the error and just log it, so it would not be
propagated at top level and pretty-printed. Extending LoggerService
takes care of those use cases.