From 5939984828953f1deba122ab61117c135226496f Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 25 Aug 2020 16:01:33 +0100 Subject: [PATCH 1/5] Workaround Bundler friendly errors not reporting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We avoid reporting SystemExit exceptions as they aren't (usually) errors. However, Bundler's friendly errors will wrap an exception in a SystemExit — in this case we need to unwrap to the original exception --- lib/bugsnag.rb | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 96f2e99b8..29891efaf 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -136,7 +136,9 @@ def register_at_exit @exit_handler_added = true at_exit do if $! - Bugsnag.notify($!, true) do |report| + exception = unwrap_bundler_exception($!) + + Bugsnag.notify(exception, true) do |report| report.severity = 'error' report.severity_reason = { :type => Bugsnag::Report::UNHANDLED_EXCEPTION @@ -390,6 +392,39 @@ def report_to_json(report) ::JSON.dump(trimmed) end + + ## + # When running a script with 'bundle exec', uncaught exceptions will be + # converted to "friendly errors" which has the side effect of wrapping them + # in a SystemExit + # + # By default we ignore SystemExit, so need to unwrap the original exception + # in order to avoid ignoring real errors + # + # @param exception [Exception] + # @return [Exception] + def unwrap_bundler_exception(exception) + running_in_bundler = ENV.include?('BUNDLE_BIN_PATH') + + # See if this exception came from Bundler's 'with_friendly_errors' method + return exception unless running_in_bundler + return exception unless exception.is_a?(SystemExit) + return exception unless exception.respond_to?(:cause) + return exception unless exception.backtrace.first.include?('/bundler/friendly_errors.rb') + return exception if exception.cause.nil? + + unwrapped = exception.cause + + # We may need to unwrap another level if the exception came from running + # an executable file directly (i.e. 'bundle exec '). In this case + # there can be a SystemExit from 'with_friendly_errors' _and_ a SystemExit + # from 'kernel_load' + return unwrapped unless unwrapped.is_a?(SystemExit) + return unwrapped unless unwrapped.backtrace.first.include?('/bundler/cli/exec.rb') + return unwrapped if unwrapped.cause.nil? + + unwrapped.cause + end end end # rubocop:enable Metrics/ModuleLength From 66d5d66ebd2ed49193a4941d0bc64d57e7fa9930 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 7 Sep 2020 14:15:19 +0100 Subject: [PATCH 2/5] Make unhandled fixtures executable --- features/fixtures/plain/app/unhandled/custom_error.rb | 6 +++--- features/fixtures/plain/app/unhandled/interrupt.rb | 6 +++--- features/fixtures/plain/app/unhandled/load_error.rb | 6 +++--- features/fixtures/plain/app/unhandled/local_jump_error.rb | 6 +++--- features/fixtures/plain/app/unhandled/name_error.rb | 6 +++--- features/fixtures/plain/app/unhandled/no_method_error.rb | 6 +++--- features/fixtures/plain/app/unhandled/runtime_error.rb | 6 +++--- features/fixtures/plain/app/unhandled/syntax_error.rb | 6 +++--- features/fixtures/plain/app/unhandled/system_call_error.rb | 6 +++--- features/fixtures/plain/app/unhandled/system_exit.rb | 6 +++--- 10 files changed, 30 insertions(+), 30 deletions(-) mode change 100644 => 100755 features/fixtures/plain/app/unhandled/custom_error.rb mode change 100644 => 100755 features/fixtures/plain/app/unhandled/interrupt.rb mode change 100644 => 100755 features/fixtures/plain/app/unhandled/load_error.rb mode change 100644 => 100755 features/fixtures/plain/app/unhandled/local_jump_error.rb mode change 100644 => 100755 features/fixtures/plain/app/unhandled/name_error.rb mode change 100644 => 100755 features/fixtures/plain/app/unhandled/no_method_error.rb mode change 100644 => 100755 features/fixtures/plain/app/unhandled/runtime_error.rb mode change 100644 => 100755 features/fixtures/plain/app/unhandled/syntax_error.rb mode change 100644 => 100755 features/fixtures/plain/app/unhandled/system_call_error.rb mode change 100644 => 100755 features/fixtures/plain/app/unhandled/system_exit.rb diff --git a/features/fixtures/plain/app/unhandled/custom_error.rb b/features/fixtures/plain/app/unhandled/custom_error.rb old mode 100644 new mode 100755 index ee95cb34a..2cb7f8d97 --- a/features/fixtures/plain/app/unhandled/custom_error.rb +++ b/features/fixtures/plain/app/unhandled/custom_error.rb @@ -1,9 +1,9 @@ -require './app' +#!/usr/bin/env ruby +require_relative '../app' configure_basics -add_at_exit class CustomError < RuntimeError end -raise CustomError.new "Oh no" \ No newline at end of file +raise CustomError.new "Oh no" diff --git a/features/fixtures/plain/app/unhandled/interrupt.rb b/features/fixtures/plain/app/unhandled/interrupt.rb old mode 100644 new mode 100755 index 3f70df75d..0971cae5a --- a/features/fixtures/plain/app/unhandled/interrupt.rb +++ b/features/fixtures/plain/app/unhandled/interrupt.rb @@ -1,6 +1,6 @@ -require './app' +#!/usr/bin/env ruby +require_relative '../app' configure_basics -add_at_exit -Process.kill("INT", Process.pid) \ No newline at end of file +Process.kill("INT", Process.pid) diff --git a/features/fixtures/plain/app/unhandled/load_error.rb b/features/fixtures/plain/app/unhandled/load_error.rb old mode 100644 new mode 100755 index 96580d381..f5b6aae03 --- a/features/fixtures/plain/app/unhandled/load_error.rb +++ b/features/fixtures/plain/app/unhandled/load_error.rb @@ -1,6 +1,6 @@ -require './app' +#!/usr/bin/env ruby +require_relative '../app' configure_basics -add_at_exit -require 'abc/def/ghi' \ No newline at end of file +require 'abc/def/ghi' diff --git a/features/fixtures/plain/app/unhandled/local_jump_error.rb b/features/fixtures/plain/app/unhandled/local_jump_error.rb old mode 100644 new mode 100755 index 92d7b28ae..516ecd0f9 --- a/features/fixtures/plain/app/unhandled/local_jump_error.rb +++ b/features/fixtures/plain/app/unhandled/local_jump_error.rb @@ -1,10 +1,10 @@ -require './app' +#!/usr/bin/env ruby +require_relative '../app' configure_basics -add_at_exit def call_block yield 50 end -call_block \ No newline at end of file +call_block diff --git a/features/fixtures/plain/app/unhandled/name_error.rb b/features/fixtures/plain/app/unhandled/name_error.rb old mode 100644 new mode 100755 index 457f0f59f..227832da0 --- a/features/fixtures/plain/app/unhandled/name_error.rb +++ b/features/fixtures/plain/app/unhandled/name_error.rb @@ -1,6 +1,6 @@ -require './app' +#!/usr/bin/env ruby +require_relative '../app' configure_basics -add_at_exit -foo \ No newline at end of file +foo diff --git a/features/fixtures/plain/app/unhandled/no_method_error.rb b/features/fixtures/plain/app/unhandled/no_method_error.rb old mode 100644 new mode 100755 index 8593c1827..f8f04f606 --- a/features/fixtures/plain/app/unhandled/no_method_error.rb +++ b/features/fixtures/plain/app/unhandled/no_method_error.rb @@ -1,6 +1,6 @@ -require './app' +#!/usr/bin/env ruby +require_relative '../app' configure_basics -add_at_exit -Kernel.foo \ No newline at end of file +Kernel.foo diff --git a/features/fixtures/plain/app/unhandled/runtime_error.rb b/features/fixtures/plain/app/unhandled/runtime_error.rb old mode 100644 new mode 100755 index d0fcd4608..0ac427d97 --- a/features/fixtures/plain/app/unhandled/runtime_error.rb +++ b/features/fixtures/plain/app/unhandled/runtime_error.rb @@ -1,6 +1,6 @@ -require './app' +#!/usr/bin/env ruby +require_relative '../app' configure_basics -add_at_exit -fail \ No newline at end of file +fail diff --git a/features/fixtures/plain/app/unhandled/syntax_error.rb b/features/fixtures/plain/app/unhandled/syntax_error.rb old mode 100644 new mode 100755 index 2777b1567..379350d6b --- a/features/fixtures/plain/app/unhandled/syntax_error.rb +++ b/features/fixtures/plain/app/unhandled/syntax_error.rb @@ -1,6 +1,6 @@ -require './app' +#!/usr/bin/env ruby +require_relative '../app' configure_basics -add_at_exit -require './unhandled/bad_syntax.rb' \ No newline at end of file +require './unhandled/bad_syntax.rb' diff --git a/features/fixtures/plain/app/unhandled/system_call_error.rb b/features/fixtures/plain/app/unhandled/system_call_error.rb old mode 100644 new mode 100755 index b1da0c661..54f84db06 --- a/features/fixtures/plain/app/unhandled/system_call_error.rb +++ b/features/fixtures/plain/app/unhandled/system_call_error.rb @@ -1,6 +1,6 @@ -require './app' +#!/usr/bin/env ruby +require_relative '../app' configure_basics -add_at_exit -File.open("abc/def/ghi") \ No newline at end of file +File.open("abc/def/ghi") diff --git a/features/fixtures/plain/app/unhandled/system_exit.rb b/features/fixtures/plain/app/unhandled/system_exit.rb old mode 100644 new mode 100755 index 12ddc795b..c6f556209 --- a/features/fixtures/plain/app/unhandled/system_exit.rb +++ b/features/fixtures/plain/app/unhandled/system_exit.rb @@ -1,6 +1,6 @@ -require './app' +#!/usr/bin/env ruby +require_relative '../app' configure_basics -add_at_exit -exit \ No newline at end of file +exit From 1feeccc6285dce8529420137f6a5e803a33383bb Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 7 Sep 2020 14:15:56 +0100 Subject: [PATCH 3/5] Run tests with bundle exec & bundle exec ruby --- .../plain_features/unhandled_errors.feature | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/features/plain_features/unhandled_errors.feature b/features/plain_features/unhandled_errors.feature index 2294042f6..c3d80ee87 100644 --- a/features/plain_features/unhandled_errors.feature +++ b/features/plain_features/unhandled_errors.feature @@ -1,7 +1,7 @@ Feature: Plain unhandled errors Scenario Outline: An unhandled error sends a report - Given I run the service "plain-ruby" with the command "bundle exec ruby unhandled/.rb" + Given I run the service "plain-ruby" with the command " unhandled/.rb" And I wait to receive a request Then the request is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" And the event "unhandled" is true @@ -12,22 +12,32 @@ Scenario Outline: An unhandled error sends a report And the "lineNumber" of stack frame 0 equals Examples: - | file | error | lineNumber | - | runtime_error | RuntimeError | 6 | - | load_error | LoadError | 6 | - | syntax_error | SyntaxError | 6 | - | local_jump_error | LocalJumpError | 7 | - | name_error | NameError | 6 | - | no_method_error | NoMethodError | 6 | - | system_call_error | Errno::ENOENT | 6 | - | custom_error | CustomError | 9 | + | file | error | lineNumber | command | + | runtime_error | RuntimeError | 6 | bundle exec | + | load_error | LoadError | 6 | bundle exec | + | syntax_error | SyntaxError | 6 | bundle exec | + | local_jump_error | LocalJumpError | 7 | bundle exec | + | name_error | NameError | 6 | bundle exec | + | no_method_error | NoMethodError | 6 | bundle exec | + | system_call_error | Errno::ENOENT | 6 | bundle exec | + | custom_error | CustomError | 9 | bundle exec | + | runtime_error | RuntimeError | 6 | bundle exec ruby | + | load_error | LoadError | 6 | bundle exec ruby | + | syntax_error | SyntaxError | 6 | bundle exec ruby | + | local_jump_error | LocalJumpError | 7 | bundle exec ruby | + | name_error | NameError | 6 | bundle exec ruby | + | no_method_error | NoMethodError | 6 | bundle exec ruby | + | system_call_error | Errno::ENOENT | 6 | bundle exec ruby | + | custom_error | CustomError | 9 | bundle exec ruby | Scenario Outline: An unhandled error doesn't send a report - When I run the service "plain-ruby" with the command "bundle exec ruby unhandled/.rb" + When I run the service "plain-ruby" with the command " unhandled/.rb" And I wait for 1 second Then I should receive no requests Examples: - | file | - | interrupt | - | system_exit | \ No newline at end of file + | file | command | + | interrupt | bundle exec | + | system_exit | bundle exec | + | interrupt | bundle exec ruby | + | system_exit | bundle exec ruby | From de64cba6b4c42ec1273e9574a31167afdf39d104 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 7 Sep 2020 15:44:33 +0100 Subject: [PATCH 4/5] Add test with exit after an exception This proves that we don't unwrap exceptions too much when unwrapping Bundler's friendly errors. In this case, the backtrace will look something like this: 1. Bundler friendly errors 2. Our exit 3. The exception we raised We want to drop 1. because it may be hiding a real exception, however we don't want to drop 2. because it's a deliberate exit This test proves that we don't go too far and end up with an exception that we would notify about --- .../plain/app/unhandled/exit_after_exception.rb | 10 ++++++++++ features/plain_features/unhandled_errors.feature | 12 +++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) create mode 100755 features/fixtures/plain/app/unhandled/exit_after_exception.rb diff --git a/features/fixtures/plain/app/unhandled/exit_after_exception.rb b/features/fixtures/plain/app/unhandled/exit_after_exception.rb new file mode 100755 index 000000000..6a73d07d6 --- /dev/null +++ b/features/fixtures/plain/app/unhandled/exit_after_exception.rb @@ -0,0 +1,10 @@ +#!/usr/bin/env ruby +require_relative '../app' + +configure_basics + +begin + raise 'oh no' +rescue + exit +end diff --git a/features/plain_features/unhandled_errors.feature b/features/plain_features/unhandled_errors.feature index c3d80ee87..2b227dc73 100644 --- a/features/plain_features/unhandled_errors.feature +++ b/features/plain_features/unhandled_errors.feature @@ -36,8 +36,10 @@ Scenario Outline: An unhandled error doesn't send a report Then I should receive no requests Examples: - | file | command | - | interrupt | bundle exec | - | system_exit | bundle exec | - | interrupt | bundle exec ruby | - | system_exit | bundle exec ruby | + | file | command | + | interrupt | bundle exec | + | system_exit | bundle exec | + | exit_after_exception | bundle exec | + | interrupt | bundle exec ruby | + | system_exit | bundle exec ruby | + | exit_after_exception | bundle exec ruby | From d428bf6cc3b6c79c90c6bd780a2bae6f2bfb0a1a Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 8 Sep 2020 13:38:50 +0100 Subject: [PATCH 5/5] Add changelog entry --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f8e3b490..68cb20f21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ Changelog ========= +## TBD + +### Enhancements + +* Bugsnag should now report uncaught exceptions inside Bundler's 'friendly errors' + | [#634](https://github.com/bugsnag/bugsnag-ruby/pull/634) + ## 6.17.0 (27 August 2020) ### Enhancements