fix apache#619 and PR apache#2956 is not enough

maoyiz · Mar 14, 2019 · b59551a · b59551a
1 parent ea3fbc8
commit b59551a
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 41 deletions.
diff --git a/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcResult.java b/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcResult.java
@@ -35,29 +35,12 @@ public RpcResult(Object result) {
     }
 
     public RpcResult(Throwable exception) {
-        this.exception = exception;
+        this.exception = handleStackTraceNull(exception);
     }
 
     @Override
     public Object recreate() throws Throwable {
         if (exception != null) {
-            // fix issue#619
-            try {
-                // get Throwable class
-                Class clazz = exception.getClass();
-                while (!clazz.getName().equals(Throwable.class.getName())) {
-                    clazz = clazz.getSuperclass();
-                }
-                // get stackTrace value
-                Field stackTraceField = clazz.getDeclaredField("stackTrace");
-                stackTraceField.setAccessible(true);
-                Object stackTrace = stackTraceField.get(exception);
-                if (stackTrace == null) {
-                    exception.setStackTrace(new StackTraceElement[0]);
-                }
-            } catch (Exception e) {
-                // ignore
-            }
             throw exception;
         }
         return result;
@@ -97,7 +80,7 @@ public Throwable getException() {
     }
 
     public void setException(Throwable e) {
-        this.exception = e;
+        this.exception = handleStackTraceNull(e);
     }
 
     @Override
@@ -109,4 +92,32 @@ public boolean hasException() {
     public String toString() {
         return "RpcResult [result=" + result + ", exception=" + exception + "]";
     }
+
+    /**
+     * fix issue#619
+     * @param e
+     * @return
+     */
+    private Throwable handleStackTraceNull(Throwable e) {
+        if (e != null) {
+            try {
+                // get Throwable class
+                Class clazz = e.getClass();
+                while (!clazz.getName().equals(Throwable.class.getName())) {
+                    clazz = clazz.getSuperclass();
+                }
+                // get stackTrace value
+                Field stackTraceField = clazz.getDeclaredField("stackTrace");
+                stackTraceField.setAccessible(true);
+                Object stackTrace = stackTraceField.get(e);
+                if (stackTrace == null) {
+                    e.setStackTrace(new StackTraceElement[0]);
+                }
+            } catch (Throwable t) {
+                // ignore
+            }
+        }
+
+        return e;
+    }
 }
diff --git a/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/RpcResultTest.java b/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/RpcResultTest.java
@@ -20,28 +20,62 @@
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
-import static org.junit.jupiter.api.Assertions.fail;
-
 public class RpcResultTest {
     @Test
-    public void testRecreateWithNormalException() {
+    public void testRpcResultWithNormalException() {
         NullPointerException npe = new NullPointerException();
         RpcResult rpcResult = new RpcResult(npe);
-        try {
-            rpcResult.recreate();
-            fail();
-        } catch (Throwable throwable) {
-            StackTraceElement[] stackTrace = throwable.getStackTrace();
-            Assertions.assertNotNull(stackTrace);
-            Assertions.assertTrue(stackTrace.length > 1);
+
+        StackTraceElement[] stackTrace = rpcResult.getException().getStackTrace();
+        Assertions.assertNotNull(stackTrace);
+        Assertions.assertTrue(stackTrace.length > 1);
+    }
+
+    /**
+     * please run this test in Run mode
+     */
+    @Test
+    public void testRpcResultWithEmptyStackTraceException() {
+        Throwable throwable = buildEmptyStackTraceException();
+        if (throwable == null) {
+            return;
         }
+        RpcResult rpcResult = new RpcResult(throwable);
+
+        StackTraceElement[] stackTrace = rpcResult.getException().getStackTrace();
+        Assertions.assertNotNull(stackTrace);
+        Assertions.assertTrue(stackTrace.length == 0);
+    }
+
+    @Test
+    public void testSetExceptionWithNormalException() {
+        NullPointerException npe = new NullPointerException();
+        RpcResult rpcResult = new RpcResult();
+        rpcResult.setException(npe);
+
+        StackTraceElement[] stackTrace = rpcResult.getException().getStackTrace();
+        Assertions.assertNotNull(stackTrace);
+        Assertions.assertTrue(stackTrace.length > 1);
     }
 
     /**
      * please run this test in Run mode
      */
     @Test
-    public void testRecreateWithEmptyStackTraceException() {
+    public void testSetExceptionWithEmptyStackTraceException() {
+        Throwable throwable = buildEmptyStackTraceException();
+        if (throwable == null) {
+            return;
+        }
+        RpcResult rpcResult = new RpcResult();
+        rpcResult.setException(throwable);
+
+        StackTraceElement[] stackTrace = rpcResult.getException().getStackTrace();
+        Assertions.assertNotNull(stackTrace);
+        Assertions.assertTrue(stackTrace.length == 0);
+    }
+
+    private Throwable buildEmptyStackTraceException() {
         // begin to construct a NullPointerException with empty stackTrace
         Throwable throwable = null;
         Long begin = System.currentTimeMillis();
@@ -59,19 +93,11 @@ public void testRecreateWithEmptyStackTraceException() {
          * may be there is -XX:-OmitStackTraceInFastThrow or run in Debug mode
          */
         if (throwable == null) {
-            System.out.println("###testRecreateWithEmptyStackTraceException fail to construct NPE");
-            return;
+            System.out.println("###buildEmptyStackTraceException fail to construct NPE");
+            return null;
         }
         // end construct a NullPointerException with empty stackTrace
 
-        RpcResult rpcResult = new RpcResult(throwable);
-        try {
-            rpcResult.recreate();
-            fail();
-        } catch (Throwable t) {
-            StackTraceElement[] stackTrace = t.getStackTrace();
-            Assertions.assertNotNull(stackTrace);
-            Assertions.assertTrue(stackTrace.length == 0);
-        }
+        return throwable;
     }
 }