[GR-19220] Display "unhandled exception" as the message for RuntimeError instances with an empty message

PullRequest: truffleruby/4057

Author: andrykonchin

CHANGELOG.md

@@ -17,6 +17,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:
 

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"

spec/tags/core/exception/detailed_message_tags.txt

@@ -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,29 +188,35 @@ 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
           result << backtrace_message
         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