Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.
Open
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
11 changes: 11 additions & 0 deletions lib/opbeat.rb
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
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
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
2 changes: 1 addition & 1 deletion opbeat.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Gem::Specification.new do |gem|
gem.extra_rdoc_files = ["README.md", "LICENSE"]

gem.add_dependency "faraday", [">= 0.7.6", "< 0.10"]
gem.add_dependency "multi_json", "~> 1.0"
gem.add_dependency "multi_json", "~> 1.12.1"

gem.add_development_dependency "bundler", "~> 1.7"
gem.add_development_dependency "rake", "~> 10.0"
Expand Down
25 changes: 25 additions & 0 deletions spec/opbeat/opbeat_spec.rb
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,7 +37,31 @@
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
after do
Opbeat.configuration.async = false
end

it 'capture_message should send result of Event.from_message' do
async = lambda {}
message = "Test message"
Expand Down
7 changes: 3 additions & 4 deletions spec/opbeat/rack_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,13 @@ def custom_user
@send = double("send")
@event = double("event")
allow(Opbeat).to receive(:send) { @send }
allow(Opbeat::Event).to receive(:from_rack_exception) { @event }
end

it 'should capture exceptions' do
exception = build_exception()
env = {}
expect(Opbeat::Event).to receive(:from_rack_exception).with(exception, env)

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

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

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

app = lambda do |e|
Expand Down
12 changes: 12 additions & 0 deletions spec/spec_helper.rb
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