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

Exception backtrace could be empty #307

Merged
merged 1 commit into from
Aug 18, 2016
Merged

Exception backtrace could be empty #307

merged 1 commit into from
Aug 18, 2016

Conversation

etehtsea
Copy link
Contributor

@etehtsea etehtsea commented Jul 13, 2016

I faced an issue when exception that had happen in my app don't show in Bugsnag UI.
I found that bugsnag backend silently swallow notification if there is empty stacktrace in any of exceptions.

:exceptions => [
  {:errorClass=>"Oxblood::Connection::TimeoutError", :message=>"Oxblood::Connection::TimeoutError", :stacktrace => <STACKTRACE>},
  {:errorClass=>"IO::EAGAINWaitReadable", :message=>"Resource temporarily unavailable - errno backtraces disabled; run with -Xerrno.backtrace=true to enable", :stacktrace=>[]}
]

Possibly it should be fixed in backend itself, but I can't do this. This is workaround for such issue.

P.S. Why don't you use 202 Accepted instead of 200 OK? It will cause much less confusion for such users (like me) who haven't looked in API documentation yet.
P.P.S. Possibly returning something like task_id that could be later used for checking processing status (using /status/:task_id) could be helpful for debugging such situations. This won't cause huge overhead if it will be made with, for example, 1 hour auto-expiration.


def stacktrace(backtrace)
backtrace = caller if !backtrace || backtrace.empty?
backtrace.map do |trace|
Copy link

Choose a reason for hiding this comment

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

(exception.backtrace.presence || caller).map do |trace| ?

Copy link

@t3hk0d3 t3hk0d3 Aug 17, 2016

Choose a reason for hiding this comment

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

@renius
#presence requires ActiveSupport, which is not in dependencies.

@kattrali
Copy link
Contributor

Thank you for the contribution, @etehtsea. Its true, exceptions without stacktraces aren’t handled by Bugsnag, as without a stacktrace, errors are generally not actionable since there’s no indication where they originated or what to do with them. However, the library behavior to not fall through to caller looks like a regression.

P.S. Why don't you use 202 Accepted instead of 200 OK? It will cause much less confusion for such users (like me) who haven't looked in API documentation yet.

This is something to be fixed in a future version :) Depending on future implementations, 202 or 204 may be appropriate.

P.P.S. Possibly returning something like task_id that could be later used for checking processing status (using /status/:task_id) could be helpful for debugging such situations. This won't cause huge overhead if it will be made with, for example, 1 hour auto-expiration.

Interesting idea! I’ll forward that along to the backend teams.

@t3hk0d3
Copy link

t3hk0d3 commented Aug 17, 2016

@kattrali Is there anything preventing this PR from merging?

@martin308
Copy link
Contributor

This looks good to me, I'll get this merged now and aim to sort out a release in the next day

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.

5 participants