Skip to content
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

Turn off reporting this meaningless warning to Rollbar #2046

Merged
merged 1 commit into from
Dec 18, 2020

Conversation

mbarnett
Copy link
Contributor

Context

Rollbar is kind of unusable right now if you don't filter out warnings, because we send a ActiveStorage::UnrepresentableError warning every time someone views an item that has a file as its "logo" which isn't able to be turned into a thumbnail. The problem is that this is in almost all cases totally normal – we know that you can't thumbnail an Excel Spreadsheet, and we have a fallback to display a generic logo for these cases, so this doesn't need to be reported.

What's New

Tell Rollbar gem to stop reporting this error.

Other Options

I'm also somewhat tempted to remove this:

logger.warn("#{logo.record_type} with id: #{logo.record_id} and thumbnail #{logo.name} threw an error.")
Rollbar.warn("#{logo.record_type} with id: #{logo.record_id} and thumbnail #{logo.name} threw an error.", e)

which warns when a jpg or png or whatever can't be thumbnailed because it's unparseable/corrupted – this is a real error, but there's really nothing we can or do do to resolve this, so it just seems like noise. Thoughts?

Copy link
Contributor

@murny murny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

For your question:

which warns when a jpg or png or whatever can't be thumbnailed because it's unparseable/corrupted – this is a real error, but there's really nothing we can or do do to resolve this, so it just seems like noise. Thoughts?

If there's nothing we can do to fix these issues, then yeah it's just noise. Do these types of issues happen a lot? is there a reason they are unparsable/corrupted?

If there is literally nothing we can do on our side and we just ignore them anyways, then absolutely, not much worth alerting us to these issues if we can't do anything to fix them. So make sense to just disable 🤔.

@mbarnett
Copy link
Contributor Author

mbarnett commented Dec 18, 2020

Put it this way, we've recorded 3,724 MiniMagick::Invalid: improper image header errors in the last 2 years, and that's just one variant that crops up for PNGs.

I think there are under a hundred items with corrupt images, but they tend to get crawled regularly, so they're noisy-ish

edit: would be good to audit them somehow and just hand a list off to Leah/Sean/Metadata and get them cleaned up. Should be pretty doable

@murny
Copy link
Contributor

murny commented Dec 18, 2020

Yeah makes sense and probably best path forward regarding create a list for product owners to go through for auditing and try to fix the issue 👍

@mbarnett
Copy link
Contributor Author

I'll throw a ticket in the backlog for it

@mbarnett mbarnett merged commit f4297b1 into master Dec 18, 2020
@mbarnett mbarnett deleted the msb/ignore_meaningless_warning branch December 18, 2020 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants