-
Notifications
You must be signed in to change notification settings - Fork 174
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
Added unhandled functionality, updated tests and integrations #368
Changes from 1 commit
7e399e3
4e40c4b
5d0411e
880affa
cdb80c4
2aa48ca
4d1770d
a7799ee
725f7f1
7023dab
8cba071
f7db8af
9d4f95e
391a755
464ac7f
82519e3
085108c
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 |
---|---|---|
|
@@ -33,12 +33,37 @@ def configure | |
end | ||
|
||
# Explicitly notify of an exception | ||
def notify(exception, auto_notify=false, &block) | ||
if auto_notify && !configuration.auto_notify | ||
def notify(exception, &block) | ||
report = Report.new(exception, configuration) | ||
send_report(report, false, &block) | ||
end | ||
|
||
# For automatic notification of exceptions | ||
def auto_notify(exception, severity_reason, &block) | ||
if !configuration.auto_notify | ||
configuration.debug("Not notifying because auto_notify is disabled") | ||
return | ||
end | ||
|
||
report = Report.new(exception, configuration, true, severity_reason) | ||
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. You could set the severity reason in here to be error? |
||
|
||
send_report(report, true, &block) | ||
end | ||
|
||
# Configuration getters | ||
def configuration | ||
@configuration = nil unless defined?(@configuration) | ||
@configuration || LOCK.synchronize { @configuration ||= Bugsnag::Configuration.new } | ||
end | ||
|
||
# Allow access to "before notify" callbacks | ||
def before_notify_callbacks | ||
Bugsnag.configuration.request_data[:before_callbacks] ||= [] | ||
end | ||
|
||
private | ||
def send_report(report, auto_notify, &block) | ||
|
||
if !configuration.valid_api_key? | ||
configuration.debug("Not notifying due to an invalid api_key") | ||
return | ||
|
@@ -49,8 +74,6 @@ def notify(exception, auto_notify=false, &block) | |
return | ||
end | ||
|
||
report = Report.new(exception, configuration) | ||
|
||
# If this is an auto_notify we yield the block before the any middleware is run | ||
yield(report) if block_given? && auto_notify | ||
if report.ignore? | ||
|
@@ -65,6 +88,9 @@ def notify(exception, auto_notify=false, &block) | |
return | ||
end | ||
|
||
# Store default severity for future reference | ||
initial_severity = report.severity | ||
|
||
# Run users middleware | ||
configuration.middleware.run(report) do | ||
if report.ignore? | ||
|
@@ -80,24 +106,18 @@ def notify(exception, auto_notify=false, &block) | |
return | ||
end | ||
|
||
# Test whether severity has been changed | ||
if report.severity != initial_severity | ||
report.default_severity = false | ||
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. You could make this a little clearer imo with |
||
end | ||
|
||
# Deliver | ||
configuration.info("Notifying #{configuration.endpoint} of #{report.exceptions.last[:errorClass]}") | ||
payload_string = ::JSON.dump(Bugsnag::Helpers.trim_if_needed(report.as_json)) | ||
configuration.debug("Payload: #{payload_string}") | ||
Bugsnag::Delivery[configuration.delivery_method].deliver(configuration.endpoint, payload_string, configuration) | ||
end | ||
end | ||
|
||
# Configuration getters | ||
def configuration | ||
@configuration = nil unless defined?(@configuration) | ||
@configuration || LOCK.synchronize { @configuration ||= Bugsnag::Configuration.new } | ||
end | ||
|
||
# Allow access to "before notify" callbacks | ||
def before_notify_callbacks | ||
Bugsnag.configuration.request_data[:before_callbacks] ||= [] | ||
end | ||
end | ||
end | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,12 @@ def call(mail) | |
yield | ||
rescue Exception => ex | ||
raise ex if [Interrupt, SystemExit, SignalException].include? ex.class | ||
Bugsnag.notify(ex, true) do |report| | ||
Bugsnag.auto_notify(ex, { | ||
:type => "middleware", | ||
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. middleware_handler |
||
:attributes => { | ||
:name => "mailman" | ||
} | ||
}) do |report| | ||
report.severity = "error" | ||
end | ||
raise | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,7 +33,12 @@ def call(env) | |
response = @app.call(env) | ||
rescue Exception => raised | ||
# Notify bugsnag of rack exceptions | ||
Bugsnag.notify(raised, true) do |report| | ||
Bugsnag.auto_notify(raised, { | ||
:type => "middleware_handler", | ||
:attirbutes => { | ||
:name => "rack" | ||
} | ||
}) do |report| | ||
report.severity = "error" | ||
end | ||
|
||
|
@@ -43,7 +48,12 @@ def call(env) | |
|
||
# Notify bugsnag of rack exceptions | ||
if env["rack.exception"] | ||
Bugsnag.notify(env["rack.exception"], true) do |report| | ||
Bugsnag.auto_notify(env["rack.exception"], { | ||
:type => "middleware", | ||
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. middleware_handler |
||
:attributes => { | ||
:name => "rack" | ||
} | ||
}) do |report| | ||
report.severity = "error" | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,12 @@ def run_callbacks(kind, *args, &block) | |
super | ||
rescue StandardError => exception | ||
# This exception will NOT be escalated, so notify it here. | ||
Bugsnag.notify(exception, true) do |report| | ||
Bugsnag.auto_notify(exception, { | ||
:type => "middleware", | ||
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. middleware_handler |
||
:attributes => { | ||
:name => "active record" | ||
} | ||
}) do |report| | ||
report.severity = "error" | ||
end | ||
raise | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,12 @@ class Railtie < Rails::Railtie | |
runner do | ||
at_exit do | ||
if $! | ||
Bugsnag.notify($!, true) do |report| | ||
Bugsnag.auto_notify($!, { | ||
:type => "middleware", | ||
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. middleware_handler |
||
:attributes => { | ||
:name => "railtie" | ||
} | ||
}) do |report| | ||
report.severity = "error" | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,12 @@ def execute_with_bugsnag(args=nil) | |
execute_without_bugsnag(args) | ||
|
||
rescue Exception => ex | ||
Bugsnag.notify(ex, true) do |report| | ||
Bugsnag.auto_notify(ex, { | ||
:type => "middleware", | ||
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. middleware_handler |
||
:attributes => { | ||
:name => "rake" | ||
} | ||
}) do |report| | ||
report.severity = "error" | ||
end | ||
raise | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,12 @@ def self.add_failure_backend | |
end | ||
|
||
def save | ||
Bugsnag.notify(exception, true) do |report| | ||
Bugsnag.auto_notify(exception, { | ||
:type => "middleware", | ||
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. middleware_handler |
||
:attributes => { | ||
:name => "mailman" | ||
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. resque |
||
} | ||
}) do |report| | ||
report.severity = "error" | ||
report.meta_data.merge!({:context => "#{payload['class']}@#{queue}", :payload => payload, :delivery_method => :synchronous}) | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,14 @@ namespace :bugsnag do | |
begin | ||
raise RuntimeError.new("Bugsnag test exception") | ||
rescue => e | ||
Bugsnag.notify(e, {:context => "rake#test_exception"}) | ||
Bugsnag.auto_notify(e, { | ||
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. This is fine to just be a normal notify, doesnt need to be an auto_notify. Its just a way of people testing their bugsnag installations |
||
:type => "middleware", | ||
:attributes => { | ||
:name => "rake" | ||
} | ||
}) do |report| | ||
report.context = "rake#test_exception" | ||
end | ||
end | ||
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.
I dont think we need the auto_notify method. If you look where we build and send the report blocks when auto_notify is set to true are run first so we can change the reports in the block for auto notifications.
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.
So I would just call notify like we did before, just telling notify that this is an auto_notify. Then in the block you can set the severity reason and the severity. Its weird that severity is set in the block, and severity reason outside it right now.