From 25189e87af882ecf0336ec2960c2ccfea0467ed3 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Tue, 14 Nov 2023 13:55:05 +0200 Subject: [PATCH] Display "unhandled exception" as the message for RuntimeError instances with an empty message Co-authored-by: Kevin Menard --- CHANGELOG.md | 1 + spec/ruby/core/exception/full_message_spec.rb | 43 +++++++++++++++++++ .../core/exception/detailed_message_tags.txt | 1 - .../core/truffle/exception_operations.rb | 29 ++++++++++--- 4 files changed, 68 insertions(+), 6 deletions(-) delete mode 100644 spec/tags/core/exception/detailed_message_tags.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index 6eb54d23397d..1ce6df16f046 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Compatibility: * Fix `rb_enc_vsprintf` and force String encoding instead of converting it (@andrykonchin). * Add `rb_gc_mark_movable` function (@andrykonchin). * Promote `File#path` and `File#to_path` to `IO#path` and `IO#to_path` and make IO#new accept an optional `path:` keyword argument (#3039, @moste00) +* Display "unhandled exception" as the message for `RuntimeError` instances with an empty message (#3255, @nirvdrum). Performance: diff --git a/spec/ruby/core/exception/full_message_spec.rb b/spec/ruby/core/exception/full_message_spec.rb index e15649ca7561..4fad369936b2 100644 --- a/spec/ruby/core/exception/full_message_spec.rb +++ b/spec/ruby/core/exception/full_message_spec.rb @@ -46,6 +46,49 @@ full_message[0].should.end_with?("': Some runtime error (RuntimeError)\n") end + describe "includes details about whether an exception was handled" do + describe "RuntimeError" do + it "should report as unhandled if message is empty" do + err = RuntimeError.new("") + + err.full_message.should =~ /unhandled exception/ + err.full_message(highlight: true).should =~ /unhandled exception/ + err.full_message(highlight: false).should =~ /unhandled exception/ + end + + it "should not report as unhandled if the message is not empty" do + err = RuntimeError.new("non-empty") + + err.full_message.should !~ /unhandled exception/ + err.full_message(highlight: true).should !~ /unhandled exception/ + err.full_message(highlight: false).should !~ /unhandled exception/ + end + + it "should not report as unhandled if the message is nil" do + err = RuntimeError.new(nil) + + err.full_message.should !~ /unhandled exception/ + err.full_message(highlight: true).should !~ /unhandled exception/ + err.full_message(highlight: false).should !~ /unhandled exception/ + end + + it "should not report as unhandled if the message is not specified" do + err = RuntimeError.new() + + err.full_message.should !~ /unhandled exception/ + err.full_message(highlight: true).should !~ /unhandled exception/ + err.full_message(highlight: false).should !~ /unhandled exception/ + end + end + + describe "generic Error" do + it "should not report as unhandled in any event" do + StandardError.new("").full_message.should !~ /unhandled exception/ + StandardError.new("non-empty").full_message.should !~ /unhandled exception/ + end + end + end + it "shows the exception class at the end of the first line of the message when the message contains multiple lines" do begin line = __LINE__; raise "first line\nsecond line" diff --git a/spec/tags/core/exception/detailed_message_tags.txt b/spec/tags/core/exception/detailed_message_tags.txt deleted file mode 100644 index a2ac5fa05772..000000000000 --- a/spec/tags/core/exception/detailed_message_tags.txt +++ /dev/null @@ -1 +0,0 @@ -fails:Exception#detailed_message returns 'unhandled exception' for an instance of RuntimeError with empty message diff --git a/src/main/ruby/truffleruby/core/truffle/exception_operations.rb b/src/main/ruby/truffleruby/core/truffle/exception_operations.rb index af591a11bfaf..28b9b4947d68 100644 --- a/src/main/ruby/truffleruby/core/truffle/exception_operations.rb +++ b/src/main/ruby/truffleruby/core/truffle/exception_operations.rb @@ -91,26 +91,33 @@ def self.receiver_string(receiver) end end + # default implementation of Exception#detailed_message hook def self.detailed_message(exception, highlight) message = StringValue exception.message.to_s - klass = Primitive.class(exception).to_s + exception_class = Primitive.class(exception) + class_name = exception_class.to_s + if Primitive.is_a?(exception, Polyglot::ForeignException) and Truffle::Interop.has_meta_object?(exception) - klass = "#{klass}: #{Truffle::Interop.meta_qualified_name Truffle::Interop.meta_object(exception)}" + class_name = "#{class_name}: #{Truffle::Interop.meta_qualified_name Truffle::Interop.meta_object(exception)}" end if message.empty? - return highlight ? "\n\e[1m#{klass}\e[m" : klass + message = Primitive.equal?(exception_class, RuntimeError) ? 'unhandled exception' : class_name + message = "\n\e[1m#{message}\e[m" if highlight + return message end anonymous_class = Primitive.module_anonymous?(Primitive.class(exception)) if highlight - highlighted_class_string = !anonymous_class ? " (\e[1;4m#{klass}\e[m\e[1m)" : '' + highlighted_class_string = !anonymous_class ? " (\e[1;4m#{class_name}\e[m\e[1m)" : '' + if message.include?("\n") first = true result = +'' + message.each_line do |line| if first first = false @@ -119,12 +126,13 @@ def self.detailed_message(exception, highlight) result << "\n\e[1m#{line.chomp}\e[m" end end + result else "\e[1m#{message}#{highlighted_class_string}\e[m" end else - class_string = !anonymous_class ? " (#{klass})" : '' + class_string = !anonymous_class ? " (#{class_name})" : '' if i = message.index("\n") "#{message[0...i]}#{class_string}#{message[i..-1]}" @@ -134,6 +142,9 @@ def self.detailed_message(exception, highlight) end end + # User can customise exception and override/undefine Exception#detailed_message method. + # This way we need to handle corner cases when #detailed_message is undefined or + # returns something other than String. def self.detailed_message_or_fallback(exception, options) unless Primitive.respond_to?(exception, :detailed_message, false) return detailed_message_fallback(exception, options) @@ -177,15 +188,18 @@ def self.full_message(exception, **options) result = ''.b bt = exception.backtrace || caller(2) + if reverse traceback_msg = if highlight "\e[1mTraceback\e[m (most recent call last):\n" else "Traceback (most recent call last):\n" end + result << traceback_msg append_causes(result, exception, {}.compare_by_identity, reverse, highlight, options) backtrace_message = backtrace_message(highlight, reverse, bt, exception, options) + if backtrace_message.empty? result << detailed_message_or_fallback(exception, options) else @@ -193,13 +207,16 @@ def self.full_message(exception, **options) end else backtrace_message = backtrace_message(highlight, reverse, bt, exception, options) + if backtrace_message.empty? result << detailed_message_or_fallback(exception, options) else result << backtrace_message end + append_causes(result, exception, {}.compare_by_identity, reverse, highlight, options) end + result end @@ -255,6 +272,8 @@ def self.append_causes(str, err, causes, reverse, highlight, options) end end + # Return user provided message if it was specified. + # A message might be computed (and assigned) lazily in some cases (e.g. for NoMethodError). def self.compute_message(exception) message = Primitive.exception_message(exception) # mimic CRuby behaviour and explicitly convert a user provided message to String