-
Notifications
You must be signed in to change notification settings - Fork 68
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
filter_chain: don't try to filter nil files #120
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,10 @@ class FilterChain | |
notice[:errors].each do |error| | ||
Gem.path.each do |gem_path| | ||
error[:backtrace].each do |frame| | ||
frame[:file].sub!(/\A#{gem_path}/, '[GEM_ROOT]'.freeze) | ||
# If the frame is unparseable, then 'file' is nil, thus nothing to | ||
# filter (all frame's data is in 'function' instead). | ||
next unless (file = frame[:file]) | ||
file.sub!(/\A#{gem_path}/, '[GEM_ROOT]'.freeze) | ||
end | ||
end | ||
end | ||
|
@@ -67,7 +70,8 @@ def root_directory_filter(root_directory) | |
proc do |notice| | ||
notice[:errors].each do |error| | ||
error[:backtrace].each do |frame| | ||
frame[:file].sub!(/\A#{root_directory}/, '[PROJECT_ROOT]'.freeze) | ||
next unless (file = frame[:file]) | ||
file.sub!(/\A#{root_directory}/, '[PROJECT_ROOT]'.freeze) | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also you probably should combine those 2 filters together to save some CPU time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it would be possible to do that without creating messy code. This particular filter executes only if the |
||
end | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to have a comment here repeating what you said in PR description "when airbrake-ruby tries to send an exception with a backtrace, which couldn't be parsed. In that case we put the whole frame into function, leaving line and file nil."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added.