cleanup NativeException

core/src/main/java/org/jruby/NativeException.java

@@ -27,8 +27,6 @@
  ***** END LICENSE BLOCK *****/
 package org.jruby;
 
-import java.io.PrintStream;
-
 import java.lang.reflect.Member;
 import org.jruby.anno.JRubyClass;
 import org.jruby.anno.JRubyMethod;
@@ -41,18 +39,19 @@
 public class NativeException extends RubyException {
 
     private final Throwable cause;
+    private final String messageAsJavaString;
     public static final String CLASS_NAME = "NativeException";
 
     public NativeException(Ruby runtime, RubyClass rubyClass, Throwable cause) {
         super(runtime, rubyClass);
         this.cause = cause;
-        this.message = runtime.newString(cause.getClass().getName() + ": " + searchStackMessage(cause));
+        this.messageAsJavaString = cause.getClass().getName() + ": " + searchStackMessage(cause);
     }
 
     private NativeException(Ruby runtime, RubyClass rubyClass) {
-        super(runtime, rubyClass);
-        this.cause   = new Throwable();
-        this.message = RubyString.newEmptyString(runtime);
+        super(runtime, rubyClass, null);
+        this.cause = new Throwable();
+        this.messageAsJavaString = null;
     }
 
     private static ObjectAllocator NATIVE_EXCEPTION_ALLOCATOR = new ObjectAllocator() {
@@ -85,10 +84,27 @@ public IRubyObject cause(Block unusedBlock) {
     public final IRubyObject backtrace() {
         IRubyObject rubyTrace = super.backtrace();
         if ( rubyTrace.isNil() ) return rubyTrace;
-
         final Ruby runtime = getRuntime();
         final RubyArray rTrace = (RubyArray) rubyTrace;
         StackTraceElement[] jTrace = cause.getStackTrace();
+
+        // NOTE: with the new filtering ruby trace will already include the source (Java) part
+        if ( rTrace.size() > 0 && jTrace.length > 0 ) {
+            final String r0 = rTrace.eltInternal(0).toString();
+            // final StackTraceElement j0 = jTrace[0];
+            final String method = jTrace[0].getMethodName();
+            final String file = jTrace[0].getFileName();
+            if ( method != null && file != null &&
+                r0.indexOf(method) != -1 && r0.indexOf(file) != -1 ) {
+                return rTrace; // as is
+            }
+        }
+        // so join-ing is mostly unnecessary, but just in case (due compatibility) make sure :
+
+        return joinedBacktrace(runtime, rTrace, jTrace);
+    }
+
+    private static RubyArray joinedBacktrace(final Ruby runtime, final RubyArray rTrace, final StackTraceElement[] jTrace) {
         final IRubyObject[] trace = new IRubyObject[jTrace.length + rTrace.size()];
         final StringBuilder line = new StringBuilder(32);
         for ( int i = 0; i < jTrace.length; i++ ) {
@@ -150,6 +166,22 @@ public void trimStackTrace(Member target) {
         }
     }
 
+    @Override
+    public final IRubyObject getMessage() {
+        if (message == null) {
+            if (messageAsJavaString == null) {
+                return message = getRuntime().getNil();
+            }
+            return message = getRuntime().newString(messageAsJavaString);
+        }
+        return message;
+    }
+
+    @Override
+    public final String getMessageAsJavaString() {
+        return messageAsJavaString;
+    }
+
     public final Throwable getCause() {
         return cause;
     }

core/src/main/java/org/jruby/RubyException.java

@@ -62,8 +62,11 @@
  */
 @JRubyClass(name="Exception")
 public class RubyException extends RubyObject {
+
     protected RubyException(Ruby runtime, RubyClass rubyClass) {
-        this(runtime, rubyClass, null);
+        super(runtime, rubyClass);
+        // this.message = null; its an internal constructor for sub-classes
+        this.cause = RubyBasicObject.UNDEF;
     }
 
     public RubyException(Ruby runtime, RubyClass rubyClass, String message) {
@@ -133,10 +136,9 @@ public RubyException exception(IRubyObject[] args) {
 
     @JRubyMethod(name = "to_s")
     public IRubyObject to_s(ThreadContext context) {
-        if (message.isNil()) {
-            return context.runtime.newString(getMetaClass().getRealClass().getName());
-        }
-        return message.asString();
+        final IRubyObject msg = getMessage();
+        if ( ! msg.isNil() ) return msg.asString();
+        return context.runtime.newString(getMetaClass().getRealClass().getName());
     }
 
     @Deprecated
@@ -343,8 +345,7 @@ public void marshalTo(Ruby runtime, Object obj, RubyClass type,
 
             marshalStream.registerLinkTarget(exc);
             List<Variable<Object>> attrs = exc.getVariableList();
-            attrs.add(new VariableEntry<Object>(
-                    "mesg", exc.message == null ? runtime.getNil() : exc.message));
+            attrs.add(new VariableEntry<Object>("mesg", exc.getMessage()));
             attrs.add(new VariableEntry<Object>("bt", exc.getBacktrace()));
             marshalStream.dumpVariables(attrs);
         }
@@ -388,12 +389,24 @@ public static IRubyObject newException(ThreadContext context, RubyClass exceptio
         return exceptionClass.callMethod(context, "new", message.convertToString());
     }
 
+    /**
+     * @return error message if provided or nil
+     */
     public IRubyObject getMessage() {
-        return message;
+        return message == null ? getRuntime().getNil() : message;
+    }
+
+    public String getMessageAsJavaString() {
+        final IRubyObject msg = getMessage();
+        return msg.isNil() ? null : msg.toString();
     }
 
     private BacktraceData backtraceData;
     private IRubyObject backtrace;
+    /**
+     * @deprecated do not access the field directly
+     * @see #getMessage()
+     */
     public IRubyObject message;
     IRubyObject cause;
 

core/src/main/java/org/jruby/exceptions/RaiseException.java

@@ -83,7 +83,7 @@ public RaiseException(RubyException actException) {
      * @param backtrace
      */
     public RaiseException(RubyException exception, IRubyObject backtrace) {
-        super(exception.message.toString());
+        super(exception.getMessageAsJavaString());
         if (DEBUG) {
             Thread.dumpStack();
         }
@@ -132,7 +132,7 @@ public RaiseException(Ruby runtime, RubyClass excptnClass, String msg, IRubyObje
     }
 
     public RaiseException(RubyException exception, boolean nativeException) {
-        super(exception.message.toString());
+        super(exception.getMessageAsJavaString());
         if (DEBUG) {
             Thread.dumpStack();
         }
@@ -142,16 +142,13 @@ public RaiseException(RubyException exception, boolean nativeException) {
     }
 
     public RaiseException(Throwable cause, NativeException nativeException) {
-        super(buildMessage(cause), cause);
-        providedMessage = buildMessage(cause);
+        super(nativeException.getMessageAsJavaString(), cause);
+        providedMessage = super.getMessage(); // cause.getClass().getName() + ": " + message
         setException(nativeException, true);
         preRaise(nativeException.getRuntime().getCurrentContext(), nativeException.getCause().getStackTrace());
         setStackTrace(RaiseException.javaTraceFromRubyTrace(exception.getBacktraceElements()));
     }
 
-    /**
-     * Method still in use by jruby-openssl <= 0.5.2
-     */
     public static RaiseException createNativeRaiseException(Ruby runtime, Throwable cause) {
         return createNativeRaiseException(runtime, cause, null);
     }
@@ -164,22 +161,10 @@ public static RaiseException createNativeRaiseException(Ruby runtime, Throwable
         return new RaiseException(cause, nativeException);
     }
 
-    private static String buildMessage(Throwable exception) {
-        StringBuilder sb = new StringBuilder();
-        StringWriter stackTrace = new StringWriter();
-        exception.printStackTrace(new PrintWriter(stackTrace));
-
-        sb.append("Native Exception: '").append(exception.getClass()).append("'; ");
-        sb.append("Message: ").append(exception.getMessage()).append("; ");
-        sb.append("StackTrace: ").append(stackTrace.getBuffer());
-
-        return sb.toString();
-    }
-
     @Override
     public String getMessage() {
         if (providedMessage == null) {
-            providedMessage = "(" + exception.getMetaClass().getBaseName() + ") " + exception.message(exception.getRuntime().getCurrentContext()).asJavaString();
+            providedMessage = '(' + exception.getMetaClass().getBaseName() + ") " + exception.message(exception.getRuntime().getCurrentContext()).asJavaString();
         }
         return providedMessage;
     }
@@ -209,10 +194,10 @@ private void preRaise(ThreadContext context, StackTraceElement[] javaTrace) {
     }
 
     private boolean requiresBacktrace(ThreadContext context) {
-        IRubyObject debugMode = context.runtime.getGlobalVariables().get("$DEBUG");
+        IRubyObject debugMode;
         // We can only omit backtraces of descendents of Standard error for 'foo rescue nil'
         return context.exceptionRequiresBacktrace ||
-                (debugMode != null && debugMode.isTrue()) ||
+                ((debugMode = context.runtime.getGlobalVariables().get("$DEBUG")) != null && debugMode.isTrue()) ||
                 ! context.runtime.getStandardError().isInstance(exception);
     }
 

core/src/test/java/org/jruby/javasupport/TestJava.java

@@ -2,27 +2,17 @@
 
 import java.lang.reflect.Method;
 
-import org.jruby.RubyModule;
-import org.jruby.RubyString;
+import org.jruby.*;
 import org.jruby.exceptions.RaiseException;
 import org.jruby.runtime.builtin.IRubyObject;
 import org.junit.Test;
 
-import org.jruby.Ruby;
+import org.jruby.test.ThrowingConstructor;
 
 class A {
     public static class C extends B {}
 }
-
-class B extends A {
-    public B() {}
-
-    B(int param) {
-        if (param == -1) {
-            throw new IllegalStateException("param == -1");
-        }
-    }
-}
+class B extends A { }
 
 public class TestJava extends junit.framework.TestCase {
 
@@ -101,6 +91,7 @@ public void test_get_java_class() {
             assert false;
         }
         catch (RaiseException ex) {
+            assertEquals("(NameError) cannot load Java class java.lang.BOGUS22", ex.getMessage());
             assertNotNull(ex.getCause());
             assertEquals(ClassNotFoundException.class, ex.getCause().getClass());
         }
@@ -109,7 +100,9 @@ public void test_get_java_class() {
     @Test
     public void testJavaConstructorExceptionHandling() throws Exception {
         final Ruby runtime = Ruby.newInstance();
-        JavaConstructor constructor = JavaConstructor.create(runtime, B.class.getDeclaredConstructor(int.class));
+        JavaConstructor constructor = JavaConstructor.create(runtime,
+                ThrowingConstructor.class.getDeclaredConstructor(Integer.class)
+        );
         assert constructor.new_instance(new IRubyObject[] { runtime.newFixnum(0) }) != null;
 
         assert constructor.new_instance(new Object[] { 1 }) != null;
@@ -121,6 +114,58 @@ public void testJavaConstructorExceptionHandling() throws Exception {
         catch (RaiseException ex) {
             assertEquals("(ArgumentError) wrong number of arguments (0 for 1)", ex.getMessage());
             assertNull(ex.getCause());
+            assertNotNull(ex.getException());
+            assertEquals("wrong number of arguments (0 for 1)", ex.getException().getMessage().toString());
+        }
+
+        try {
+            constructor.new_instance(new Object[] { -1 });
+            assert false;
+        }
+        catch (RaiseException ex) {
+            // ex.printStackTrace();
+            assertEquals("java.lang.IllegalStateException: param == -1", ex.getMessage());
+            StackTraceElement e0 = ex.getStackTrace()[0];
+            assertEquals("org.jruby.test.ThrowingConstructor", e0.getClassName());
+            assertEquals("<init>", e0.getMethodName());
+
+            assertNotNull(ex.getCause());
+            assert ex.getCause() instanceof IllegalStateException;
+
+            assertNotNull(ex.getException());
+            assert ex.getException() instanceof NativeException;
+            assertEquals("java.lang.IllegalStateException: param == -1", ex.getException().message(runtime.getCurrentContext()).toString());
+            assertEquals("java.lang.IllegalStateException: param == -1", ex.getException().getMessage().toString());
+            assertEquals("param == -1", ex.getCause().getMessage());
+
+            assert ex.getException().backtrace() instanceof RubyArray;
+            RubyArray backtrace = (RubyArray) ex.getException().backtrace();
+            //    org/jruby/test/ThrowingConstructor.java:8:in `<init>'
+            //    java/lang/reflect/Constructor.java:423:in `newInstance'
+            //    org/jruby/javasupport/JavaConstructor.java:222:in `new_instance'
+            //    java/lang/reflect/Method.java:498:in `invoke'
+            //    junit/framework/TestCase.java:176:in `runTest'
+            assert backtrace.get(0).toString().contains("org/jruby/test/ThrowingConstructor.java");
+        }
+
+        try {
+            constructor.new_instance(new Object[] { null }); // new IllegalStateException() null cause message
+            assert false;
+        }
+        catch (RaiseException ex) {
+            // ex.printStackTrace();
+            assertEquals("java.lang.IllegalStateException: null", ex.getMessage());
+
+            assertNotNull(ex.getCause());
+            assert ex.getCause() instanceof IllegalStateException;
+
+            assertNotNull(ex.getException());
+            assert ex.getException() instanceof NativeException;
+            assertEquals("java.lang.IllegalStateException: null", ex.getException().message(runtime.getCurrentContext()).toString());
+            assertEquals("java.lang.IllegalStateException: null", ex.getException().getMessage().toString());
+            assertNull(ex.getCause().getMessage());
+
+            assert ex.getException().backtrace() instanceof RubyArray;
         }
     }
 

core/src/test/java/org/jruby/test/ThrowingConstructor.java

@@ -0,0 +1,10 @@
+package org.jruby.test;
+
+public class ThrowingConstructor {
+    // public ThrowingConstructor() { }
+
+    public ThrowingConstructor(Integer param) {
+        if (param == null) throw new IllegalStateException();
+        if (param < 0) throw new IllegalStateException("param == " + param);
+    }
+}