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

Truncate Exception Messages if needed. #636

Closed

Conversation

joshuapinter
Copy link
Contributor

@joshuapinter joshuapinter commented Oct 8, 2020

Goal

If an exception occurs, such as a Mysql2::Error::ConnectionError, where the query is extremely long, this can cause the payload to exceed what Bugsnag will accept and throw a Errno::EPIPE: Broken pipe error when trying to send the request to Bugsnag.

A number of elements are already trimmed when the payload is too large, including meta data, stacktrace code, etc. However, the exception messages were not being trimmed.

This commit adds exception message trimming to Bugsnag::Helpers.trim_if_needed, which should help reduce errors when the queries causing the errors are exceptionally long.

Design

A number of elements are already trimmed when the payload is too large, including meta data, stacktrace code, etc. However, the exception messages were not being trimmed.

This commit adds exception message trimming to Bugsnag::Helpers.trim_if_needed, which should help reduce errors when the queries causing the errors are exceptionally long.

Changeset

Added truncate_exception_messages to Bugsnag::Helpers.trim_if_needed method, which truncates the exception messages if necessary.

Testing

Manually on our Rails 4.2 / Bugsnag 6.17.0 enterprise application.

Added a spec to helper_spec.rb that tests this functionality specifically.

✅ All tests passing when running bundle exec rake.

If an exception occurs, such as a `Mysql2::Error::ConnectionError`, where the query is extremely long, this can cause the payload to exceed what Bugsnag will accept and throw a `Errno::EPIPE: Broken pipe` error when trying to send the request to Bugsnag.

A number of elements are already trimmed when the payload is too large, including meta data, stacktrace code, etc. However, the exception messages were not being trimmed.

This commit adds exception message trimming to `Bugsnag::Helpers.trim_if_needed`, which should help reduce errors when the queries causing the errors are exceptionally long.

It also adds a spec to `helper_spec.rb` that tests this functionality.
@joshuapinter joshuapinter marked this pull request as ready for review October 15, 2020 16:34
@imjoehaines
Copy link
Contributor

Hey @joshuapinter, thanks for the PR

This one will need a bit of discussion internally as we aim to have a single algorithm that all of our libraries use to ensure a payload is always less than the maximum length (for historical reasons, bugsnag-ruby doesn't quite implement that yet!)

I think this change makes sense though — it doesn't seem like there would ever be much value in a >3000 character exception message — but we'll need to make sure we've thought through everything before merging, as well as the nuances like what order we should trim things in

In the meantime, you can implement this using an add_on_error callback:

Bugsnag.configuration.add_on_error(proc do |report|
  report.exceptions.each do |exception|
    next unless exception[:message].length > 3000

    exception[:message] = exception[:message][0...3000]
  end
end)

@joshuapinter
Copy link
Contributor Author

Sounds good @imjoehaines. For now we're just using our fork of it. Whether this change gets merged or if the standardized trimming algorithm includes it, it's an important change. We had multiple customers running into serious issues without knowing it and it was only the couple customers that submitted a Support Request that made us aware of it.

Let me know if you need anything from our end.

Thanks!

@abigailbramble abigailbramble added backlog We hope to fix this feature/bug in the future needs discussion Requires internal analysis/discussion labels Oct 21, 2020
@imjoehaines
Copy link
Contributor

Hey @joshuapinter, I've fixed the conflict and will merge this in #646

Thanks again for the PR — this change turned out to be super important for Ruby 3 as NameError and NoMethodError messages don't get truncated by default anymore (ruby/ruby#3090)

@bugsnagbot bugsnagbot added scheduled Work is starting on this feature/bug and removed backlog We hope to fix this feature/bug in the future labels Dec 21, 2020
@joshuapinter
Copy link
Contributor Author

Good timing then! Thanks for the update and have a great holiday and New Year.

@imjoehaines imjoehaines mentioned this pull request Jan 6, 2021
@mattdyoung mattdyoung added bug Confirmed bug released This feature/bug fix has been released and removed needs discussion Requires internal analysis/discussion scheduled Work is starting on this feature/bug labels Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug released This feature/bug fix has been released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants