From 8f70b332cc245eb77d701cfd91c6e8d7c4ca69fe Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Mon, 26 Sep 2022 20:15:02 +0200 Subject: [PATCH 1/4] Implement interop exception messages on RubyException * Previously getExceptionMessage()/getExceptionStackTrace() only worked if called on RaiseException and not on the RubyException. * Do not include the exception class name in getExceptionMessage(), the callers should use the meta object name instead. --- doc/contributor/interop_details.md | 68 +++++++++++++++-- spec/truffle/interop/matrix_spec.rb | 75 ++++++++++++++++++- .../core/exception/ExceptionNodes.java | 15 ++++ .../core/exception/RubyException.java | 49 ++++++++++++ .../language/control/RaiseException.java | 8 +- .../truffleruby/ContextPermissionsTest.java | 25 +++++-- .../org/truffleruby/JSR223InteropTest.java | 2 +- .../org/truffleruby/PolyglotInteropTest.java | 4 +- 8 files changed, 222 insertions(+), 24 deletions(-) diff --git a/doc/contributor/interop_details.md b/doc/contributor/interop_details.md index 5905121d566c..1cb2c4681654 100644 --- a/doc/contributor/interop_details.md +++ b/doc/contributor/interop_details.md @@ -16,6 +16,8 @@ - **a `Class`** - **a `Hash`** - **an `Array`** +- **an `Exception`** +- **an `Exception` with a cause** - **`proc {...}`** - **`lambda {...}`** - **a `Method`** @@ -307,11 +309,11 @@ When interop message `getHashValuesIterator` is sent ## Members related messages (incomplete) When interop message `readMember` is sent -- to any non-immediate `Object` like **`nil`**, **`:symbol`**, **a `String`**, **a `BigDecimal`**, **an `Object`**, **a frozen `Object`**, **a `StructWithValue`**, **a `Class`**, **a `Hash`**, **an `Array`**, **`proc {...}`**, **`lambda {...}`**, **a `Method`**, **a `Truffle::FFI::Pointer`**, **polyglot pointer**, **polyglot array** or **polyglot hash** +- to any non-immediate `Object` like **`nil`**, **`:symbol`**, **a `String`**, **a `BigDecimal`**, **an `Object`**, **a frozen `Object`**, **a `StructWithValue`**, **a `Class`**, **a `Hash`**, **an `Array`**, **an `Exception`**, **an `Exception` with a cause**, **`proc {...}`**, **`lambda {...}`**, **a `Method`**, **a `Truffle::FFI::Pointer`**, **polyglot pointer**, **polyglot array** or **polyglot hash** it returns a method with the given name when the method is defined. -- to any non-immediate `Object` like **`nil`**, **`:symbol`**, **a `String`**, **a `BigDecimal`**, **an `Object`**, **a frozen `Object`**, **a `StructWithValue`**, **a `Class`**, **a `Hash`**, **an `Array`**, **`proc {...}`**, **`lambda {...}`**, **a `Method`**, **a `Truffle::FFI::Pointer`**, **polyglot pointer**, **polyglot array** or **polyglot hash** +- to any non-immediate `Object` like **`nil`**, **`:symbol`**, **a `String`**, **a `BigDecimal`**, **an `Object`**, **a frozen `Object`**, **a `StructWithValue`**, **a `Class`**, **a `Hash`**, **an `Array`**, **an `Exception`**, **an `Exception` with a cause**, **`proc {...}`**, **`lambda {...}`**, **a `Method`**, **a `Truffle::FFI::Pointer`**, **polyglot pointer**, **polyglot array** or **polyglot hash** it fails with `UnknownIdentifierException` when the method is not defined. -- to any non-immediate `Object` like **a `String`**, **an `Object`**, **a `StructWithValue`**, **a `Class`**, **a `Hash`**, **an `Array`**, **`proc {...}`**, **`lambda {...}`**, **a `Method`**, **a `Truffle::FFI::Pointer`**, **polyglot pointer**, **polyglot array** or **polyglot hash** +- to any non-immediate `Object` like **a `String`**, **an `Object`**, **a `StructWithValue`**, **a `Class`**, **a `Hash`**, **an `Array`**, **an `Exception`**, **an `Exception` with a cause**, **`proc {...}`**, **`lambda {...}`**, **a `Method`**, **a `Truffle::FFI::Pointer`**, **polyglot pointer**, **polyglot array** or **polyglot hash** it reads the given instance variable. - to **polyglot members** it reads the value stored with the given name. @@ -321,7 +323,7 @@ When interop message `readMember` is sent it fails with `UnsupportedMessageError`. When interop message `writeMember` is sent -- to any non-immediate non-frozen `Object` like **a `String`**, **an `Object`**, **a `StructWithValue`**, **a `Class`**, **a `Hash`**, **an `Array`**, **`proc {...}`**, **`lambda {...}`**, **a `Method`**, **a `Truffle::FFI::Pointer`**, **polyglot pointer**, **polyglot array** or **polyglot hash** +- to any non-immediate non-frozen `Object` like **a `String`**, **an `Object`**, **a `StructWithValue`**, **a `Class`**, **a `Hash`**, **an `Array`**, **an `Exception`**, **an `Exception` with a cause**, **`proc {...}`**, **`lambda {...}`**, **a `Method`**, **a `Truffle::FFI::Pointer`**, **polyglot pointer**, **polyglot array** or **polyglot hash** it writes the given instance variable. - to **polyglot members** it writes the given value under the given name. @@ -332,10 +334,64 @@ When interop message `writeMember` is sent - otherwise it fails with `UnsupportedMessageError`. +## Exception related messages + +When interop message `isException` is sent +- to **an `Exception`** or **an `Exception` with a cause** + it returns true. +- otherwise + it returns false. + +When interop message `throwException` is sent +- to **an `Exception`** or **an `Exception` with a cause** + it throws the exception. +- otherwise + it fails with `UnsupportedMessageError`. + +When interop message `getExceptionType` is sent +- to **an `Exception`** or **an `Exception` with a cause** + it returns the exception type. +- otherwise + it fails with `UnsupportedMessageError`. + +When interop message `hasExceptionMessage` is sent +- to **an `Exception`** or **an `Exception` with a cause** + it returns true. +- otherwise + it returns false. + +When interop message `getExceptionMessage` is sent +- to **an `Exception`** or **an `Exception` with a cause** + it returns the message of the exception. +- otherwise + it fails with `UnsupportedMessageError`. + +When interop message `hasExceptionStackTrace` is sent +- to **an `Exception`** or **an `Exception` with a cause** + it returns true. +- otherwise + it returns false. + +When interop message `getExceptionStackTrace` is sent +- to **an `Exception`** or **an `Exception` with a cause** + it returns the stacktrace of the exception. +- otherwise + it fails with `UnsupportedMessageError`. + +When interop message `hasExceptionCause` is sent +- to **an `Exception` with a cause** + it returns true. +- otherwise + it returns false. + +When interop message `getExceptionCause` is sent +- to **an `Exception` with a cause** + it returns the cause of the exception. +- otherwise + it fails with `UnsupportedMessageError`. + ## Number related messages (missing) ## Instantiation related messages (missing) -## Exception related messages (missing) - ## Time related messages (unimplemented) diff --git a/spec/truffle/interop/matrix_spec.rb b/spec/truffle/interop/matrix_spec.rb index 60440f756d0f..c623a308a778 100644 --- a/spec/truffle/interop/matrix_spec.rb +++ b/spec/truffle/interop/matrix_spec.rb @@ -1,3 +1,5 @@ +# truffleruby_primitives: true + # Copyright (c) 2018, 2021 Oracle and/or its affiliates. All rights reserved. This # code is released under a tri EPL/GPL/LGPL license. You can use it, # redistribute it and/or modify it under the terms of the: @@ -243,6 +245,33 @@ def spec_it(subject) module: Subject.(name: AN_INSTANCE) { Module.new }, hash: Subject.(name: AN_INSTANCE, doc: true) { {} }, array: Subject.(name: AN_INSTANCE, doc: true) { [] }, + # raise & rescue to give it a backtrace + exception: Subject.(name: "an `Exception`", doc: true) do + begin + raise "the exception message" + rescue => e + e + end + end, + exception_with_cause: Subject.(name: "an `Exception` with a cause", doc: true) do + begin + raise "the cause" + rescue + begin + raise "the exception message" + rescue => e + e + end + end + end, + # also test RaiseException since it is what other languages see when they catch an exception from Ruby + raise_exception: Subject.(name: AN_INSTANCE) do + begin + raise "the exception message" + rescue => e + Primitive.exception_get_raise_exception(e) + end + end, proc: Subject.(proc { |v| v }, name: code("proc {...}"), doc: true), lambda: Subject.(-> v { v }, name: code("lambda {...}"), doc: true), @@ -286,6 +315,7 @@ def spec_it(subject) immediate_subjects = [:false, :true, :zero, :small_integer, :zero_float, :small_float] non_immediate_subjects = SUBJECTS.keys - immediate_subjects frozen_subjects = [:big_decimal, :nil, :symbol, :strange_symbol, :frozen_object] + exception_subjects = [:exception, :exception_with_cause, :raise_exception] # not part of the standard matrix, not considered in last rest case EXTRA_SUBJECTS = { @@ -298,7 +328,7 @@ def spec_it(subject) def predicate(name, is, *message_args, &setup) -> subject do setup.call subject if setup - Truffle::Interop.send(name, subject, *message_args).send(is ? :should : :should_not, be_true) + Truffle::Interop.send(name, subject, *message_args).should == is end end @@ -580,7 +610,7 @@ def array_element_predicate(message, predicate, insert_on_true_case) Delimiter["Members related messages (incomplete)"], Message[:readMember, Test.new("returns a method with the given name when the method is defined", "any non-immediate `Object`", - *non_immediate_subjects - [:polyglot_object]) do |subject| + *non_immediate_subjects - [:polyglot_object, :raise_exception]) do |subject| Truffle::Interop.read_member(subject, 'to_s').should == subject.method(:to_s) end, Test.new("fails with `UnknownIdentifierException` when the method is not defined", "any non-immediate `Object`", @@ -626,9 +656,48 @@ def array_element_predicate(message, predicate, insert_on_true_case) end, unsupported_test { |subject| Truffle::Interop.write_member(subject, :something, 'val') }], + Delimiter["Exception related messages"], + Message[:isException, + Test.new("returns true", *exception_subjects, &predicate(:exception?, true)), + Test.new("returns false", &predicate(:exception?, false))], + Message[:throwException, + Test.new("throws the exception", *exception_subjects) do |subject| + -> { Truffle::Interop.throw_exception(subject) }.should raise_error { |e| e.should.equal?(subject) } + end, + unsupported_test { |subject| Truffle::Interop.throw_exception(subject) }], + Message[:getExceptionType, + Test.new("returns the exception type", *exception_subjects) do |subject| + Truffle::Interop.exception_type(subject).should == :RUNTIME_ERROR + end, + unsupported_test { |subject| Truffle::Interop.exception_type(subject) }], + Message[:hasExceptionMessage, + Test.new("returns true", *exception_subjects, &predicate(:has_exception_message?, true)), + Test.new("returns false", &predicate(:has_exception_message?, false))], + Message[:getExceptionMessage, + Test.new("returns the message of the exception", *exception_subjects) do |subject| + Truffle::Interop.exception_message(subject).should == "the exception message" + end, + unsupported_test { |subject| Truffle::Interop.exception_message(subject) }], + Message[:hasExceptionStackTrace, + Test.new("returns true", *exception_subjects, &predicate(:has_exception_stack_trace?, true)), + Test.new("returns false", &predicate(:has_exception_stack_trace?, false))], + Message[:getExceptionStackTrace, + Test.new("returns the stacktrace of the exception", *exception_subjects) do |subject| + stacktrace = Truffle::Interop.exception_stack_trace(subject) + Truffle::Interop.should.has_array_elements?(stacktrace) + end, + unsupported_test { |subject| Truffle::Interop.exception_stack_trace(subject) }], + Message[:hasExceptionCause, + Test.new("returns true", :exception_with_cause, &predicate(:has_exception_cause?, true)), + Test.new("returns false", &predicate(:has_exception_cause?, false))], + Message[:getExceptionCause, + Test.new("returns the cause of the exception", :exception_with_cause) do |subject| + Truffle::Interop.exception_cause(subject).should == subject.cause + end, + unsupported_test { |subject| Truffle::Interop.exception_cause(subject) }], + Delimiter["Number related messages (missing)"], Delimiter["Instantiation related messages (missing)"], - Delimiter["Exception related messages (missing)"], Delimiter["Time related messages (unimplemented)"], ] diff --git a/src/main/java/org/truffleruby/core/exception/ExceptionNodes.java b/src/main/java/org/truffleruby/core/exception/ExceptionNodes.java index 28338ecff060..d6f037605cc5 100644 --- a/src/main/java/org/truffleruby/core/exception/ExceptionNodes.java +++ b/src/main/java/org/truffleruby/core/exception/ExceptionNodes.java @@ -353,4 +353,19 @@ protected int limit() { } + @Primitive(name = "exception_get_raise_exception") + public abstract static class GetRaiseExceptionNode extends CoreMethodArrayArgumentsNode { + + @Specialization + protected Object getRaiseException(RubyException exception) { + RaiseException raiseException = exception.backtrace.getRaiseException(); + if (raiseException != null) { + return raiseException; + } else { + return nil; + } + } + + } + } diff --git a/src/main/java/org/truffleruby/core/exception/RubyException.java b/src/main/java/org/truffleruby/core/exception/RubyException.java index dd044a8ba67b..4c17f08dbfad 100644 --- a/src/main/java/org/truffleruby/core/exception/RubyException.java +++ b/src/main/java/org/truffleruby/core/exception/RubyException.java @@ -11,14 +11,18 @@ import java.util.Set; +import com.oracle.truffle.api.TruffleStackTraceElement; import com.oracle.truffle.api.interop.ExceptionType; import com.oracle.truffle.api.interop.InteropLibrary; +import com.oracle.truffle.api.interop.UnsupportedMessageException; import com.oracle.truffle.api.library.CachedLibrary; import com.oracle.truffle.api.library.ExportLibrary; import com.oracle.truffle.api.library.ExportMessage; import com.oracle.truffle.api.nodes.Node; import org.truffleruby.RubyContext; +import org.truffleruby.RubyLanguage; import org.truffleruby.core.VMPrimitiveNodes.VMRaiseExceptionNode; +import org.truffleruby.core.array.ArrayHelpers; import org.truffleruby.core.array.RubyArray; import org.truffleruby.core.klass.RubyClass; import org.truffleruby.core.proc.RubyProc; @@ -33,6 +37,8 @@ import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.object.Shape; +import static org.truffleruby.language.RubyBaseNode.nil; + @ExportLibrary(InteropLibrary.class) public class RubyException extends RubyDynamicObject implements ObjectGraphNode { @@ -96,6 +102,49 @@ public RuntimeException throwException( public ExceptionType getExceptionType() { return ExceptionType.RUNTIME_ERROR; } + + @ExportMessage + public boolean hasExceptionCause() { + return this.cause != nil; + } + + @ExportMessage + public Object getExceptionCause() throws UnsupportedMessageException { + if (!hasExceptionCause()) { + throw UnsupportedMessageException.create(); + } + return this.cause; + } + + @ExportMessage + public boolean hasExceptionMessage() { + return true; + } + + @ExportMessage + public Object getExceptionMessage() { + return ExceptionOperations.messageToString(this); + } + + @ExportMessage + public boolean hasExceptionStackTrace() { + return this.backtrace != null; + } + + @TruffleBoundary + @ExportMessage + public Object getExceptionStackTrace() throws UnsupportedMessageException { + if (!hasExceptionStackTrace()) { + throw UnsupportedMessageException.create(); + } + + TruffleStackTraceElement[] stackTrace = this.backtrace.getStackTrace(); + Object[] items = new Object[stackTrace.length]; + for (int i = 0; i < items.length; i++) { + items[i] = stackTrace[i].getGuestObject(); + } + return ArrayHelpers.createArray(RubyContext.get(null), RubyLanguage.get(null), items); + } // endregion } diff --git a/src/main/java/org/truffleruby/language/control/RaiseException.java b/src/main/java/org/truffleruby/language/control/RaiseException.java index c091ff60144f..5c76ee56f0d1 100644 --- a/src/main/java/org/truffleruby/language/control/RaiseException.java +++ b/src/main/java/org/truffleruby/language/control/RaiseException.java @@ -15,11 +15,8 @@ import org.truffleruby.RubyContext; import org.truffleruby.core.exception.ExceptionOperations; import org.truffleruby.core.exception.RubyException; -import org.truffleruby.core.module.ModuleFields; import org.truffleruby.language.backtrace.Backtrace; -import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; - /** A ControlFlowException holding a Ruby exception. */ @SuppressWarnings("serial") @ExportLibrary(value = InteropLibrary.class, delegateTo = "exception") @@ -55,11 +52,8 @@ public RubyException getException() { } @Override - @TruffleBoundary public String getMessage() { - final ModuleFields exceptionClass = exception.getLogicalClass().fields; - final String message = ExceptionOperations.messageToString(exception); - return String.format("%s (%s)", message, exceptionClass.getName()); + return ExceptionOperations.messageToString(exception); } } diff --git a/src/test/java/org/truffleruby/ContextPermissionsTest.java b/src/test/java/org/truffleruby/ContextPermissionsTest.java index e5121077315d..f8e546101013 100644 --- a/src/test/java/org/truffleruby/ContextPermissionsTest.java +++ b/src/test/java/org/truffleruby/ContextPermissionsTest.java @@ -39,13 +39,22 @@ public void testPointerNoNative() throws Throwable { Assert.assertTrue(context.eval("ruby", "defined?(Truffle::FFI::Pointer::NULL).nil?").asBoolean()); RubyTest.assertThrows( () -> context.eval("ruby", "Truffle::FFI::Pointer.allocate"), - e -> assertEquals("native access is not allowed (SecurityError)", e.getMessage())); + e -> { + assertEquals("native access is not allowed", e.getMessage()); + assertEquals("SecurityError", e.getGuestObject().getMetaObject().getMetaQualifiedName()); + }); RubyTest.assertThrows( () -> context.eval("ruby", "Truffle::FFI::Pointer.new(4)"), - e -> assertEquals("native access is not allowed (SecurityError)", e.getMessage())); + e -> { + assertEquals("native access is not allowed", e.getMessage()); + assertEquals("SecurityError", e.getGuestObject().getMetaObject().getMetaQualifiedName()); + }); RubyTest.assertThrows( () -> context.eval("ruby", "Truffle::FFI::MemoryPointer.new(4)"), - e -> assertEquals("native access is not allowed (SecurityError)", e.getMessage())); + e -> { + assertEquals("native access is not allowed", e.getMessage()); + assertEquals("SecurityError", e.getGuestObject().getMetaObject().getMetaQualifiedName()); + }); } } @@ -81,7 +90,10 @@ public void testThreadsNoNative() throws Throwable { RubyTest.assertThrows( () -> context.eval("ruby", "File.stat('.')"), - e -> assertEquals("native access is not allowed (SecurityError)", e.getMessage())); + e -> { + assertEquals("native access is not allowed", e.getMessage()); + assertEquals("SecurityError", e.getGuestObject().getMetaObject().getMetaQualifiedName()); + }); } } @@ -97,7 +109,10 @@ public void testNoThreadsEnforcesSingleThreadedOption() throws Throwable { RubyTest.assertThrows( () -> context.eval("ruby", "Thread.new {}.join"), - e -> assertEquals("threads not allowed in single-threaded mode (SecurityError)", e.getMessage())); + e -> { + assertEquals("threads not allowed in single-threaded mode", e.getMessage()); + assertEquals("SecurityError", e.getGuestObject().getMetaObject().getMetaQualifiedName()); + }); } } diff --git a/src/test/java/org/truffleruby/JSR223InteropTest.java b/src/test/java/org/truffleruby/JSR223InteropTest.java index fd58d0a29f9c..bea535c4fd33 100644 --- a/src/test/java/org/truffleruby/JSR223InteropTest.java +++ b/src/test/java/org/truffleruby/JSR223InteropTest.java @@ -69,7 +69,7 @@ public void testNoPermissionsByDefault() { scriptEngine.eval("Process.pid"); fail("should have thrown"); } catch (ScriptException scriptException) { - assertEquals("org.graalvm.polyglot.PolyglotException: native access is not allowed (SecurityError)", + assertEquals("org.graalvm.polyglot.PolyglotException: native access is not allowed", scriptException.getMessage()); } } diff --git a/src/test/java/org/truffleruby/PolyglotInteropTest.java b/src/test/java/org/truffleruby/PolyglotInteropTest.java index 147fb2bf81e2..1d947958f1de 100644 --- a/src/test/java/org/truffleruby/PolyglotInteropTest.java +++ b/src/test/java/org/truffleruby/PolyglotInteropTest.java @@ -200,7 +200,7 @@ public void testTopScopes() { // create a method to test search order context.eval("ruby", "def forty_two; :method; end"); assertEquals("method", context.eval(interactiveSource("forty_two")).asString()); - assertEquals("Method", bindings.getMember("forty_two").getMetaObject().getMetaSimpleName()); + assertEquals("Method", bindings.getMember("forty_two").getMetaObject().getMetaQualifiedName()); bindings.putMember("forty_two", 42); assertEquals(42, bindings.getMember("forty_two").asInt()); @@ -211,7 +211,7 @@ public void testTopScopes() { bindings.putMember("local_var", 42); RubyTest.assertThrows( () -> context.eval("ruby", "local_var"), // non-interactive source - e -> assertEquals("NameError", e.getGuestObject().getMetaObject().getMetaSimpleName())); + e -> assertEquals("NameError", e.getGuestObject().getMetaObject().getMetaQualifiedName())); context.eval(interactiveSource("new_eval_local_var = 88")); assertEquals(88, context.eval(interactiveSource("new_eval_local_var")).asInt()); From fe40a9ef0e1fd50eda2f9317cd632c6abf0aeb30 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Mon, 26 Sep 2022 21:01:48 +0200 Subject: [PATCH 2/4] Include the meta object name for Exception#full_message with a foreign exception --- .../truffleruby/core/truffle/exception_operations.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/main/ruby/truffleruby/core/truffle/exception_operations.rb b/src/main/ruby/truffleruby/core/truffle/exception_operations.rb index 638935ff40e0..8d7f4f074594 100644 --- a/src/main/ruby/truffleruby/core/truffle/exception_operations.rb +++ b/src/main/ruby/truffleruby/core/truffle/exception_operations.rb @@ -94,8 +94,14 @@ def self.receiver_string(receiver) def self.message_and_class(exception, highlight) message = StringValue exception.message.to_s + klass = exception.class.to_s + if Primitive.object_kind_of?(exception, Polyglot::ForeignException) and + Truffle::Interop.has_meta_object?(exception) + klass = "#{klass}: #{Truffle::Interop.meta_qualified_name Truffle::Interop.meta_object(exception)}" + end + if highlight - highlighted_class = " (\e[1;4m#{exception.class}\e[m\e[1m)" + highlighted_class = " (\e[1;4m#{klass}\e[m\e[1m)" if message.include?("\n") first = true result = +'' @@ -113,9 +119,9 @@ def self.message_and_class(exception, highlight) end else if i = message.index("\n") - "#{message[0...i]} (#{exception.class})#{message[i..-1]}" + "#{message[0...i]} (#{klass})#{message[i..-1]}" else - "#{message} (#{exception.class})" + "#{message} (#{klass})" end end end From 1b622a73e952ea7463419789ac2aca3bacbd70f3 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Mon, 26 Sep 2022 22:02:25 +0200 Subject: [PATCH 3/4] Only catch RaiseException for ExceptionOperations#messageToString * Use messageFieldToString() for RaiseException#getMessage() as the context might not be entered. --- .../org/truffleruby/core/exception/ExceptionOperations.java | 4 ++-- .../java/org/truffleruby/language/control/RaiseException.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/truffleruby/core/exception/ExceptionOperations.java b/src/main/java/org/truffleruby/core/exception/ExceptionOperations.java index 7bcfa6b25908..9efbf436fcf1 100644 --- a/src/main/java/org/truffleruby/core/exception/ExceptionOperations.java +++ b/src/main/java/org/truffleruby/core/exception/ExceptionOperations.java @@ -97,7 +97,7 @@ public static String getMessage(Throwable throwable) { } @TruffleBoundary - private static String messageFieldToString(RubyException exception) { + public static String messageFieldToString(RubyException exception) { Object message = exception.message; RubyStringLibrary strings = RubyStringLibrary.getUncached(); if (message == null || message == Nil.INSTANCE) { @@ -115,7 +115,7 @@ public static String messageToString(RubyException exception) { Object messageObject = null; try { messageObject = DispatchNode.getUncached().call(exception, "message"); - } catch (Throwable e) { + } catch (RaiseException e) { // Fall back to the internal message field } if (messageObject != null && RubyStringLibrary.getUncached().isRubyString(messageObject)) { diff --git a/src/main/java/org/truffleruby/language/control/RaiseException.java b/src/main/java/org/truffleruby/language/control/RaiseException.java index 5c76ee56f0d1..d0761e5618bf 100644 --- a/src/main/java/org/truffleruby/language/control/RaiseException.java +++ b/src/main/java/org/truffleruby/language/control/RaiseException.java @@ -53,7 +53,7 @@ public RubyException getException() { @Override public String getMessage() { - return ExceptionOperations.messageToString(exception); + return ExceptionOperations.messageFieldToString(exception); } } From 2c34c011d741b457b3b83ad9e876d54862fefde1 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 27 Sep 2022 12:33:08 +0200 Subject: [PATCH 4/4] Call exc.cause only once in append_causes --- .../core/truffle/exception_operations.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/ruby/truffleruby/core/truffle/exception_operations.rb b/src/main/ruby/truffleruby/core/truffle/exception_operations.rb index 8d7f4f074594..d3c9ad1522a6 100644 --- a/src/main/ruby/truffleruby/core/truffle/exception_operations.rb +++ b/src/main/ruby/truffleruby/core/truffle/exception_operations.rb @@ -193,24 +193,25 @@ def self.backtrace?(exc) end def self.append_causes(str, err, causes, reverse, highlight) - if !Primitive.nil?(err.cause) && Exception === err.cause && !causes.has_key?(err.cause) - causes[err.cause] = true + cause = err.cause + if !Primitive.nil?(cause) && Exception === cause && !causes.has_key?(cause) + causes[cause] = true if reverse - append_causes(str, err.cause, causes, reverse, highlight) - backtrace_message = backtrace_message(highlight, reverse, err.cause.backtrace, err.cause) + append_causes(str, cause, causes, reverse, highlight) + backtrace_message = backtrace_message(highlight, reverse, cause.backtrace, cause) if backtrace_message.empty? str << message_and_class(err, highlight) else str << backtrace_message end else - backtrace_message = backtrace_message(highlight, reverse, err.cause.backtrace, err.cause) + backtrace_message = backtrace_message(highlight, reverse, cause.backtrace, cause) if backtrace_message.empty? str << message_and_class(err, highlight) else str << backtrace_message end - append_causes(str, err.cause, causes, reverse, highlight) + append_causes(str, cause, causes, reverse, highlight) end end end