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.

Sign up for GitHub

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

backtrace: special case classloader path #140

Merged
merged 1 commit into from
Dec 15, 2016
Merged

backtrace: special case classloader path #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ Airbrake Ruby Changelog

### master

* Improved JRuby parsing of frames which include classloader
([#140](https://github.com/airbrake/airbrake-ruby/pull/140))

### [v1.6.0][v1.6.0] (October 18, 2016)

* Added support for blacklisting/whitelisting using procs
Expand Down
2 changes: 2 additions & 0 deletions lib/airbrake-ruby/backtrace.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ module Patterns
(?<file>
(?:uri:classloader:/.+(?=:)) # Matches '/META-INF/jruby.home/protocol.rb'
|
(?:uri_3a_classloader_3a_.+(?=:)) # Matches 'uri_3a_classloader_3a_/gems/...'
|
[^:]+ # Matches 'NewlineNode.java'
)
:?
Expand Down
8 changes: 6 additions & 2 deletions spec/backtrace_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,18 @@
let(:backtrace) do
# rubocop:disable Metrics/LineLength
['uri_3a_classloader_3a_.META_minus_INF.jruby_dot_home.lib.ruby.stdlib.net.protocol.rbuf_fill(uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/net/protocol.rb:158)',
'bin.processors.image_uploader.block in make_streams(bin/processors/image_uploader.rb:21)']
'bin.processors.image_uploader.block in make_streams(bin/processors/image_uploader.rb:21)',
'uri_3a_classloader_3a_.gems.faye_minus_websocket_minus_0_dot_10_dot_5.lib.faye.websocket.api.invokeOther13:dispatch_event(uri_3a_classloader_3a_/gems/faye_minus_websocket_minus_0_dot_10_dot_5/lib/faye/websocket/uri:classloader:/gems/faye-websocket-0.10.5/lib/faye/websocket/api.rb:109)',
'tmp.jruby9022301782566983632extract.$dot.META_minus_INF.rails.file(/tmp/jruby9022301782566983632extract/./META-INF/rails.rb:13)']
# rubocop:enable Metrics/LineLength
end

let(:parsed_backtrace) do
# rubocop:disable Metrics/LineLength
[{ file: 'uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/net/protocol.rb', line: 158, function: 'uri_3a_classloader_3a_.META_minus_INF.jruby_dot_home.lib.ruby.stdlib.net.protocol.rbuf_fill' },
{ file: 'bin/processors/image_uploader.rb', line: 21, function: 'bin.processors.image_uploader.block in make_streams' }]
{ file: 'bin/processors/image_uploader.rb', line: 21, function: 'bin.processors.image_uploader.block in make_streams' },
{ file: 'uri_3a_classloader_3a_/gems/faye_minus_websocket_minus_0_dot_10_dot_5/lib/faye/websocket/uri:classloader:/gems/faye-websocket-0.10.5/lib/faye/websocket/api.rb', line: 109, function: 'uri_3a_classloader_3a_.gems.faye_minus_websocket_minus_0_dot_10_dot_5.lib.faye.websocket.api.invokeOther13:dispatch_event' },
Copy link

Choose a reason for hiding this comment

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

Looks like this should be

file: /gems/faye-websocket-0.10.5/lib/faye/websocket/api.rb
function: dispatch_event

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't really know whether the Java classloader stuff is useful, so I guess safe to leave as is. Are you sure it's gibberish?

Copy link

Choose a reason for hiding this comment

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

I am sure that user wants what I posted. If Java classloader stuff is not useful then it is not clear why we are parsing it at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, I'll change it. We are parsing it because I don't want to leave out potentially useful information.

Copy link
Contributor Author

@kyrylo kyrylo Dec 14, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I think the file change makes sense, but the function change isn't obvious how to parse.

Consider:

These backtraces are so random that in order to parse them super precisely, we would need a complex regexp.

Copy link

Choose a reason for hiding this comment

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

Okay, I dont insist and we can revisit when JRuby users show up.

https://github.com/airbrake/airbrake-ruby/blob/master/spec/backtrace_spec.rb#L83

should be "block in make_streams", because "bin.processors.image_uploader" is generated from file

{ file: '/tmp/jruby9022301782566983632extract/./META-INF/rails.rb', line: 13, function: 'tmp.jruby9022301782566983632extract.$dot.META_minus_INF.rails.file' }]
# rubocop:enable Metrics/LineLength
end

Expand Down