-
Notifications
You must be signed in to change notification settings - Fork 113
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
Don't crash on non-writable logger file #160
Conversation
59b602e
to
cc85a7a
Compare
begin | ||
@logger = Logger.new(path) | ||
@logger.formatter = log_formatter | ||
rescue Exception => error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I agree that we should be careful here, we probably don't want to rescue Exception
and turn it into a warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite sure what all the errors that can be thrown here, so I'll change it to StandardError
using rescue => e
.
Not sure why I thought rescuing Exception was a good thing, especially since we discussed it earlier.. 🤔
a39dfbd
to
c4a355a
Compare
9228bb6
to
8e8de22
Compare
c4a355a
to
7e871c9
Compare
Rebased on updated master |
724e7b2
to
05119f9
Compare
Updated to rescue on SystemCallError. Seems to encapsulate most IO errors we can trigger |
When the parent path is writable, but the logger file itself is not, do no longer crash. Also updated specs to be more descriptive and cover more cases.
05119f9
to
bd90694
Compare
When the parent path is writable, but the logger file itself is not, do no longer crash.
Also updated specs to be more descriptive and cover more cases.
Based on #158
Part of fix for #153