You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by ca...@apache.org on 2019/04/02 06:38:29 UTC

[incubator-dubbo] branch master updated: [Dubbo-619] Fix #619 and PR #2956 is not enough (#3634)

This is an automated email from the ASF dual-hosted git repository.

carryxyh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-dubbo.git


The following commit(s) were added to refs/heads/master by this push:
     new f932d7b  [Dubbo-619] Fix #619 and PR #2956 is not enough (#3634)
f932d7b is described below

commit f932d7b453d925b7d29c35130debf41707e77957
Author: zhaixiaoxiang <xx...@126.com>
AuthorDate: Tue Apr 2 14:37:55 2019 +0800

    [Dubbo-619] Fix #619 and PR #2956 is not enough (#3634)
    
    fix for issue #619 and PR #2956
---
 .../main/java/org/apache/dubbo/rpc/RpcResult.java  | 54 +++++++++++------
 .../java/org/apache/dubbo/rpc/RpcResultTest.java   | 70 +++++++++++++++-------
 2 files changed, 83 insertions(+), 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
index dfec74c..8087210 100644
--- 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 class RpcResult extends AbstractResult {
     }
 
     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 class RpcResult extends AbstractResult {
     }
 
     public void setException(Throwable e) {
-        this.exception = e;
+        this.exception = handleStackTraceNull(e);
     }
 
     @Override
@@ -109,4 +92,37 @@ public class RpcResult extends AbstractResult {
     public String toString() {
         return "RpcResult [result=" + result + ", exception=" + exception + "]";
     }
+
+    /**
+     * we need to deal the exception whose stack trace is null.
+     * <p>
+     * see https://github.com/apache/incubator-dubbo/pull/2956
+     * and https://github.com/apache/incubator-dubbo/pull/3634
+     * and https://github.com/apache/incubator-dubbo/issues/619
+     *
+     * @param e exception
+     * @return exception after deal with stack trace
+     */
+    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
index eda5117..9c983a5 100644
--- 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 @@ package org.apache.dubbo.rpc;
 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 class RpcResultTest {
          * 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;
     }
 }