-
Notifications
You must be signed in to change notification settings - Fork 100
Refactor: improve debug logging (log catched exceptions) #280
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
Conversation
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.
Generally LGTM, but there are some tracelog not guarded by logger.trace? &&
. If we apply the rule to lazy evaluate the log expressions, probably it should be applied everywhere a tracelog is used, WDYT?
added |
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.
Thanks @kares LGTM
tried to normalize traces involving
:filename
or:path
to be more consistent - easier to grep.otherwise the intent is mostly (debug) logging exceptions the plugin's expected to rescue and continue running.
particularly, I wasn't sure why a watched file keeps going through the read loop (having new content)
but comes out as never reading anything on Windows (e.g. from here), knowing whether there's an EOF or file-access gone wrong would confirm whether there's a deeper issue in the plugin or not.
have also raised a few traces to the debug level - ones that seem to provide more value than just tracing what the plugin is processing atm.