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

nested_exception: fix backtrace parsing #29

Merged
merged 1 commit into from
Jan 18, 2016
Merged

Conversation

kyrylo
Copy link
Contributor

@kyrylo kyrylo commented Jan 15, 2016

While working on airbrake/airbrake#479 I
stumbled upon a bug in Airbrake Ruby where the default filter couldn't
process error payload because :backtrace was nil. Returning an
empty array in case the real backtrace is missing seems to be a more
secure approach.

@kyrylo kyrylo force-pushed the fix-backtrace-parsing branch from dbf45db to 1c5dadf Compare January 18, 2016 10:17
@kyrylo
Copy link
Contributor Author

kyrylo commented Jan 18, 2016

PTAL

@@ -35,7 +35,7 @@ def unwind_exceptions
end

def parse_backtrace(exception)
return if exception.backtrace.nil? || exception.backtrace.none?
return [] if exception.backtrace.nil? || exception.backtrace.none?
Backtrace.parse(exception)

Choose a reason for hiding this comment

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

Please move this check to Backtrace.parse or change this method to:

return [] if exception.backtrace.nil? || exception.backtrace.none?
Backtrace.parse(exception.backtrace)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the check to Backtrace.parse. Wanted to do the latter, but the Java exceptions check in Backtrace makes it a more complex refactoring (which is not worth it).

@vmihailenco
Copy link

LGTM

@kyrylo kyrylo force-pushed the fix-backtrace-parsing branch from 1c5dadf to 34cd679 Compare January 18, 2016 11:20
While working on airbrake/airbrake#479 I
stumbled upon a bug in Airbrake Ruby where the default filter couldn't
process error payload because `:backtrace` was `nil`. Returning an
empty array in case the real backtrace is missing seems to be a more
secure approach.
@kyrylo kyrylo force-pushed the fix-backtrace-parsing branch from 34cd679 to 934c81c Compare January 18, 2016 11:27
kyrylo added a commit that referenced this pull request Jan 18, 2016
nested_exception: fix backtrace parsing
@kyrylo kyrylo merged commit 53134c8 into master Jan 18, 2016
@kyrylo kyrylo deleted the fix-backtrace-parsing branch January 18, 2016 11:37
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