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

Support exception chaining #312

Merged
merged 2 commits into from
Apr 8, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions lib/raven/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require 'raven/logger'
require 'raven/interfaces/message'
require 'raven/interfaces/exception'
require 'raven/interfaces/single_exception'
require 'raven/interfaces/stack_trace'
require 'raven/interfaces/http'
require 'raven/processor'
Expand Down
27 changes: 19 additions & 8 deletions lib/raven/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,25 @@ def self.from_message(message, options = {})
end

def self.add_exception_interface(evt, exc)
evt.interface(:exception) do |int|
int.type = exc.class.to_s
int.value = exc.to_s
int.module = exc.class.to_s.split('::')[0...-1].join('::')

int.stacktrace = if exc.backtrace
StacktraceInterface.new do |stacktrace|
stacktrace_interface_from(stacktrace, evt, exc.backtrace)
evt.interface(:exception) do |exc_int|
exceptions = [exc]
while exc.respond_to?(:cause) && exc.cause
exceptions << exc.cause
exc = exc.cause
end
exceptions.reverse!

exc_int.values = exceptions.map do |exc|
SingleExceptionInterface.new do |int|
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm, I'm not really a fan of this name/a new class. Why can't you just use multiple instances of ExceptionInterface?

int.type = exc.class.to_s
int.value = exc.to_s
int.module = exc.class.to_s.split('::')[0...-1].join('::')

int.stacktrace = if exc.backtrace
StacktraceInterface.new do |stacktrace|
stacktrace_interface_from(stacktrace, evt, exc.backtrace)
end
end
end
end
end
Expand Down
9 changes: 3 additions & 6 deletions lib/raven/interfaces/exception.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,12 @@ module Raven
class ExceptionInterface < Interface

name 'exception'
attr_accessor :type
attr_accessor :value
attr_accessor :module
attr_accessor :stacktrace
attr_accessor :values
Copy link
Contributor

Choose a reason for hiding this comment

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

Now I can't find the old documentation for how Exception interfaces are supposed to work with Sentry. I thought that info was here: http://sentry.readthedocs.org/en/latest/developer/interfaces/ but it isn't anymore. @dcramer where'd that go?

Copy link
Member

Choose a reason for hiding this comment

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

Docs should be fixed, but this looks correct to me (values is a list of exception instances)


def to_hash(*args)
data = super(*args)
if data[:stacktrace]
data[:stacktrace] = data[:stacktrace].to_hash
if data[:values]
data[:values] = data[:values].map(&:to_hash)
end
data
end
Expand Down
19 changes: 19 additions & 0 deletions lib/raven/interfaces/single_exception.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
require 'raven/interfaces'

module Raven
class SingleExceptionInterface < Interface

attr_accessor :type
attr_accessor :value
attr_accessor :module
attr_accessor :stacktrace

def to_hash(*args)
data = super(*args)
if data[:stacktrace]
data[:stacktrace] = data[:stacktrace].to_hash
end
data
end
end
end
6 changes: 5 additions & 1 deletion lib/raven/processor/removestacktrace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ module Raven
class Processor::RemoveStacktrace < Processor

def process(value)
value[:exception].delete(:stacktrace) if value[:exception]
if value[:exception]
value[:exception][:values].map do |single_exception|
single_exception.delete(:stacktrace) if single_exception[:stacktrace]
end
end

value
end
Expand Down
37 changes: 28 additions & 9 deletions spec/raven/event_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -300,15 +300,15 @@
end

it 'uses the exception class name as the exception type' do
expect(hash[:exception][:type]).to eq('Exception')
expect(hash[:exception][:values][0][:type]).to eq('Exception')
end

it 'uses the exception message as the exception value' do
expect(hash[:exception][:value]).to eq(message)
expect(hash[:exception][:values][0][:value]).to eq(message)
end

it 'does not belong to a module' do
expect(hash[:exception][:module]).to eq('')
expect(hash[:exception][:values][0][:module]).to eq('')
end
end

Expand All @@ -319,7 +319,7 @@ class Exception < Exception; end
let(:exception) { Raven::Test::Exception.new(message) }

it 'sends the module name as part of the exception info' do
expect(hash[:exception][:module]).to eq('Raven::Test')
expect(hash[:exception][:values][0][:module]).to eq('Raven::Test')
end
end

Expand Down Expand Up @@ -352,6 +352,25 @@ class SubExc < BaseExc; end
end
end

# Only check causes when they're supported
if Exception.new.respond_to? :cause
context 'when the exception has a cause' do
let(:exception) { build_exception_with_cause }

it 'captures the cause' do
expect(hash[:exception][:values].length).to eq(2)
end
end

context 'when the exception has nested causes' do
let(:exception) { build_exception_with_two_causes }

it 'captures nested causes' do
expect(hash[:exception][:values].length).to eq(3)
end
end
end

context 'when the exception has a backtrace' do
let(:exception) do
e = Exception.new(message)
Expand All @@ -363,7 +382,7 @@ class SubExc < BaseExc; end
end

it 'parses the backtrace' do
frames = hash[:exception][:stacktrace][:frames]
frames = hash[:exception][:values][0][:stacktrace][:frames]
expect(frames.length).to eq(2)
expect(frames[0][:lineno]).to eq(1412)
expect(frames[0][:function]).to eq('other_function')
Expand All @@ -382,7 +401,7 @@ class SubExc < BaseExc; end
end

it 'marks filename and in_app correctly' do
frames = hash[:exception][:stacktrace][:frames]
frames = hash[:exception][:values][0][:stacktrace][:frames]
expect(frames[0][:lineno]).to eq(10)
expect(frames[0][:function]).to eq("synchronize")
expect(frames[0][:filename]).to eq("<internal:prelude>")
Expand Down Expand Up @@ -415,7 +434,7 @@ class SubExc < BaseExc; end

it 'marks in_app correctly' do
expect(Raven.configuration.project_root).to eq('/rails/root')
frames = hash[:exception][:stacktrace][:frames]
frames = hash[:exception][:values][0][:stacktrace][:frames]
expect(frames[0][:filename]).to eq("test/some/other/path")
expect(frames[0][:in_app]).to eq(true)
expect(frames[1][:filename]).to eq("/app/some/other/path")
Expand All @@ -440,7 +459,7 @@ class SubExc < BaseExc; end
end

it "doesn't remove any path information under project_root" do
frames = hash[:exception][:stacktrace][:frames]
frames = hash[:exception][:values][0][:stacktrace][:frames]
expect(frames[3][:filename]).to eq("app/models/user.rb")
end
end
Expand All @@ -461,7 +480,7 @@ class SubExc < BaseExc; end
end

it 'strips prefixes in the load path from frame filenames' do
frames = hash[:exception][:stacktrace][:frames]
frames = hash[:exception][:values][0][:stacktrace][:frames]
expect(frames[0][:filename]).to eq('other/path')
end
end
Expand Down
31 changes: 29 additions & 2 deletions spec/raven/processors/removestacktrace_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,37 @@
it 'should remove stacktraces' do
data = Raven::Event.capture_exception(build_exception).to_hash

expect(data[:exception][:stacktrace]).to_not eq(nil)
expect(data[:exception][:values][0][:stacktrace]).to_not eq(nil)
result = @processor.process(data)

expect(result[:exception][:stacktrace]).to eq(nil)
expect(result[:exception][:values][0][:stacktrace]).to eq(nil)
end

# Only check causes when they're supported
if Exception.new.respond_to? :cause
it 'should remove stacktraces from causes' do
data = Raven::Event.capture_exception(build_exception_with_cause).to_hash

expect(data[:exception][:values][0][:stacktrace]).to_not eq(nil)
expect(data[:exception][:values][1][:stacktrace]).to_not eq(nil)
result = @processor.process(data)

expect(result[:exception][:values][0][:stacktrace]).to eq(nil)
expect(result[:exception][:values][1][:stacktrace]).to eq(nil)
end

it 'should remove stacktraces from nested causes' do
data = Raven::Event.capture_exception(build_exception_with_two_causes).to_hash

expect(data[:exception][:values][0][:stacktrace]).to_not eq(nil)
expect(data[:exception][:values][1][:stacktrace]).to_not eq(nil)
expect(data[:exception][:values][2][:stacktrace]).to_not eq(nil)
result = @processor.process(data)

expect(result[:exception][:values][0][:stacktrace]).to eq(nil)
expect(result[:exception][:values][1][:stacktrace]).to eq(nil)
expect(result[:exception][:values][2][:stacktrace]).to eq(nil)
end
end

end
24 changes: 24 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,27 @@ def build_exception
rescue ZeroDivisionError => exception
return exception
end

def build_exception_with_cause
begin
1 / 0
rescue ZeroDivisionError
1 / 0
end
rescue ZeroDivisionError => exception
return exception
end

def build_exception_with_two_causes
begin
begin
1 / 0
rescue ZeroDivisionError
1 / 0
end
rescue ZeroDivisionError
1 / 0
end
rescue ZeroDivisionError => exception
return exception
end