From e88ed316d2d40b1062acb03bec1c2b7bdc3511f5 Mon Sep 17 00:00:00 2001 From: Gord Pearson Date: Tue, 17 Mar 2026 13:51:51 -0400 Subject: [PATCH] Walk exception cause chain to filter benign signals wrapped by finalizer errors When a process receives SIGTERM/SIGHUP/SIGINT, an ensure block (e.g. Dev::Finalize.deliver!) can raise a secondary exception such as Errno::EPIPE or Errno::EBADF when it tries to write to a closed stderr. This secondary exception replaces the original SignalException as the top-level error, but keeps it accessible via Exception#cause. Previously, exception_for_submission only checked the top-level exception for SignalException, so these wrapped signals fell through to the else branch and were incorrectly reported as bugs. Add caused_by_benign_signal? which walks the entire cause chain, so SIGTERM/SIGHUP/SIGINT are still filtered even when buried as a cause. Co-authored-by: Claude --- lib/cli/kit/error_handler.rb | 16 +++++++++--- test/cli/kit/error_handler_test.rb | 42 ++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/lib/cli/kit/error_handler.rb b/lib/cli/kit/error_handler.rb index 1a987747..00f1deb9 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 eb18f3b5..98eec9d3 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,