Add useful message for logger silence error

[rossta]

The introduction of the quiet assets middleware assumes that ::Rails.logger responds to #silence. For projects using incompatible logger objects, like Ruby's Logger, a (potentially confusing) NoMethodError is raised in development when config.assets.quiet is enabled.

This change now provides an informative error message to help developers update their configured logger to take advantage of quiet asset logging.

Fixes #376

[rails-bot]

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @rafaelfranca (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[rafaelfranca]

Is not better to check if the logger respond to silence and raise an exception if not instead of rescue an exception?

[rossta]

Yes, that sounds like a great idea. I'll make that change and see if I can address the build errors.

[matthewd]

Is it worth considering just in-lining LoggerSilence instead? It's friendlier for users to call, but doing it manually is not exactly complicated.

[rossta]

Yes, that's another good idea I considered but wasn't sure how it'd be
received. It wouldn't require much code, it'd just be a case of duplicated
functionality. Thoughts?

On Friday, October 7, 2016, Matthew Draper [email protected] wrote:

Is it worth considering just in-lining LoggerSilence instead? It's
friendlier for users to call, but doing it manually is not exactly
complicated.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#381 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAtmfYsIqBowLmJ9h310cAwr4KjpJQYks5qxsmwgaJpZM4KRNEj
.

Ross Kaffenberger
@rossta

[matthewd]

Ah, looking closer, I see LoggerSilence is only as short as it is because it uses local_level, which is also our invention.

Perhaps Rails.logger should guarantee it'll return a "Rails-like" logger interface -- wrapping the originally supplied one if needed. But that seems a separate enterprise.

+1 on respond_to?(:silence) here for now.

[rossta]

Changes made, let me know if you have any more suggestions.

[rossta]

@rafaelfranca Just checking on this to see if there's anything else I can do to help move it forward. Thanks!

[ioquatix]

@rafaelfranca we just ran into this issue too.

[ioquatix]

Just for reference, this is how it's handled in core rails:

https://github.com/rails/rails/blob/f95c0b7e96eb36bc3efc0c5beffbb9e84ea664e4/actionpack/lib/action_dispatch/middleware/debug_view.rb#L46-L54

[ioquatix]

Thanks @tenderlove