Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Add Opbeat.capture_rack_exception #27

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add Opbeat.capture_rack_exception #27

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
74cae00
Add Opbeat.capture_rack_exception
teodor-pripoae Jul 16, 2015
f171706
Fix Opbeat::Rack to use capture_rack_exception method
teodor-pripoae Jul 16, 2015
9e6538b
Put original specs back for Opbeat::Rack since the problem is elsewhere
teodor-pripoae Jul 16, 2015
c970e89
Get back to original state after async tests finish
teodor-pripoae Jul 16, 2015
e3d7bca
Update multi_json
teodor-pripoae Oct 20, 2016
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
11 changes: 11 additions & 0 deletions lib/opbeat.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,17 @@ def capture_exception(exception, options={})
end
end

def capture_rack_exception(exception, env, options={})
exception.set_backtrace caller unless exception.backtrace
if (evt = Event.from_rack_exception(exception, env, options))
if self.configuration.async?
self.configuration.async.call(evt)
else
send(evt)
end
end
end

def capture_message(message, options={})
if (evt = Event.from_message(message, caller, options))
if self.configuration.async?
Expand Down
6 changes: 2 additions & 4 deletions lib/opbeat/rack.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,14 @@ def call(env)
rescue Error => e
raise # Don't capture Opbeat errors
rescue Exception => e
evt = Event.from_rack_exception(e, env)
Opbeat.send(evt)
Opbeat.capture_rack_exception(e, env)
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR :) Can you explain why you don't send the event anymore?

Copy link
Author

Choose a reason for hiding this comment

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

Becouse it is sent in the capture_rack_exception method. Also I think that original method didn't support async and also didn't support ignoring user defined errors. (note: it was also not checking if evt is nil)

Copy link
Author

Choose a reason for hiding this comment

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

Sorry If you got confused about removing the expect(Opbeat).to receive(:send).with(@event) lines from tests. I put them back since the original issue that was getting the specs to fail was from elsewhere.

The async tests were letting the configuration have async callback, and since the middleware now supports async behaviour, the specs needed change. I fixed the async specs to cleanup after finish, so now that changes are not needed anymore.

Copy link
Author

Choose a reason for hiding this comment

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

Also, maybe I didn't explain properly. This PR allows users to call the capture_rack_exception from their code (similar to capture_exception), allowing them to pass rack environment.

raise
end

error = env['rack.exception'] || env['sinatra.error']

if error
evt = Event.from_rack_exception(error, env)
Opbeat.send(evt) if evt
Opbeat.capture_rack_exception(error, env)
end

response
Expand Down
21 changes: 21 additions & 0 deletions spec/opbeat/opbeat_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
allow(Opbeat).to receive(:send) { @send }
allow(Opbeat::Event).to receive(:from_message) { @event }
allow(Opbeat::Event).to receive(:from_exception) { @event }
allow(Opbeat::Event).to receive(:from_rack_exception) { @event }
end

it 'capture_message should send result of Event.from_message' do
Expand Down Expand Up @@ -36,6 +37,26 @@
Opbeat.capture_exception(exception)
end

it 'capture_rack_exception should send result of Event.from_exception built with env and default options' do
exception = build_exception()
rack_env = build_rack_env()

expect(Opbeat::Event).to receive(:from_rack_exception).with(exception, rack_env, {})
expect(Opbeat).to receive(:send).with(@event)

Opbeat.capture_rack_exception(exception, rack_env)
end

it "capture_rack_exception should send result of Event.from_exception built with env and options" do
exception = build_exception()
rack_env = build_rack_env()

expect(Opbeat::Event).to receive(:from_rack_exception).with(exception, rack_env, {:custom => :param})
expect(Opbeat).to receive(:send).with(@event)

Opbeat.capture_rack_exception(exception, rack_env, {:custom => :param})
end

context "async" do
it 'capture_message should send result of Event.from_message' do
async = lambda {}
Expand Down
8 changes: 3 additions & 5 deletions spec/opbeat/rack_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,8 @@ def custom_user
it 'should capture exceptions' do
exception = build_exception()
env = {}

expect(Opbeat::Event).to receive(:from_rack_exception).with(exception, env)
expect(Opbeat).to receive(:send).with(@event)

expect(Opbeat).to receive(:capture_rack_exception).with(exception, env)

app = lambda do |e|
raise exception
Expand All @@ -57,8 +56,7 @@ def custom_user
exception = build_exception()
env = {}

expect(Opbeat::Event).to receive(:from_rack_exception).with(exception, env)
expect(Opbeat).to receive(:send).with(@event)
expect(Opbeat).to receive(:capture_rack_exception).with(exception, env)

app = lambda do |e|
e['rack.exception'] = exception
Expand Down
12 changes: 12 additions & 0 deletions spec/spec_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,15 @@ def build_exception()
return exception
end
end

def build_rack_env()
{
"QUERY_STRING" => "a=1&b=2",
"REMOTE_ADDR" => "::1",
"REMOTE_HOST" => "localhost",
"REQUEST_METHOD" => "GET",
"REQUEST_PATH" => "/index.html",
"HTTP_HOST" => "localhost:3000",
"HTTP_VERSION" => "HTTP/1.1"
}
end