diff --git a/lib/cli/kit/error_handler.rb b/lib/cli/kit/error_handler.rb index 1a98774..00f1deb 100644 --- a/lib/cli/kit/error_handler.rb +++ b/lib/cli/kit/error_handler.rb @@ -114,12 +114,20 @@ def triage_all_exceptions(&block) e.bug? ? CLI::Kit::EXIT_BUG : CLI::Kit::EXIT_FAILURE_BUT_NOT_BUG end + #: (Exception? error) -> bool + def caused_by_benign_signal?(error) + current = error #: Exception? + while current + return true if current.is_a?(SignalException) && SIGNALS_THAT_ARENT_BUGS.include?(current.message) + + current = current.cause + end + false + end + #: (Exception? error) -> Exception? def exception_for_submission(error) - # happens on normal non-error termination - return if error.nil? - - return unless error.bug? + return if error.nil? || !error.bug? || caused_by_benign_signal?(error) case error when SignalException diff --git a/test/cli/kit/error_handler_test.rb b/test/cli/kit/error_handler_test.rb index eb18f3b..98eec9d 100644 --- a/test/cli/kit/error_handler_test.rb +++ b/test/cli/kit/error_handler_test.rb @@ -176,6 +176,39 @@ def test_non_bug_signal skip end + def test_benign_signal_wrapped_by_epipe_is_not_reported + # When a finalizer raises Errno::EPIPE after a SIGTERM, the EPIPE + # becomes the top-level exception with SIGTERM as its cause. + # The error handler should still filter it as benign. + wrapped_error = build_wrapped_error( + cause: SignalException.new('SIGTERM'), + outer: Errno::EPIPE, + ) + + @rep.expects(:report).never + @eh.report_exception(wrapped_error) + end + + def test_benign_signal_wrapped_by_ebadf_is_not_reported + wrapped_error = build_wrapped_error( + cause: SignalException.new('SIGHUP'), + outer: Errno::EBADF, + ) + + @rep.expects(:report).never + @eh.report_exception(wrapped_error) + end + + def test_non_benign_signal_wrapped_is_still_reported + wrapped_error = build_wrapped_error( + cause: SignalException.new('SIGSEGV'), + outer: Errno::EPIPE, + ) + + @rep.expects(:report).once + @eh.report_exception(wrapped_error) + end + def test_bug_signal # e.g. SIGSEGV skip @@ -195,6 +228,15 @@ def test_exit_1 private + # Builds an exception where `outer` has `cause` as its cause. + # We can't use raise/rescue with SignalException because Ruby + # actually delivers the signal. Instead, we stub #cause. + def build_wrapped_error(cause:, outer:) + error = outer.is_a?(Exception) ? outer : outer.new + error.define_singleton_method(:cause) { cause } + error + end + def error_handler(tool_name: nil, dev_mode: false) ErrorHandler.new( log_file: @tf.path, exception_reporter: @rep, tool_name: tool_name, dev_mode: dev_mode,