Skip to content

Commit f932d7b

Browse files
maoyizcarryxyh
authored andcommitted
[Dubbo-619] Fix #619 and PR #2956 is not enough (#3634)
fix for issue #619 and PR #2956
1 parent 1df4a21 commit f932d7b

File tree

2 files changed

+83
-41
lines changed

2 files changed

+83
-41
lines changed

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcResult.java

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -35,29 +35,12 @@ public RpcResult(Object result) {
3535
}
3636

3737
public RpcResult(Throwable exception) {
38-
this.exception = exception;
38+
this.exception = handleStackTraceNull(exception);
3939
}
4040

4141
@Override
4242
public Object recreate() throws Throwable {
4343
if (exception != null) {
44-
// fix issue#619
45-
try {
46-
// get Throwable class
47-
Class clazz = exception.getClass();
48-
while (!clazz.getName().equals(Throwable.class.getName())) {
49-
clazz = clazz.getSuperclass();
50-
}
51-
// get stackTrace value
52-
Field stackTraceField = clazz.getDeclaredField("stackTrace");
53-
stackTraceField.setAccessible(true);
54-
Object stackTrace = stackTraceField.get(exception);
55-
if (stackTrace == null) {
56-
exception.setStackTrace(new StackTraceElement[0]);
57-
}
58-
} catch (Exception e) {
59-
// ignore
60-
}
6144
throw exception;
6245
}
6346
return result;
@@ -97,7 +80,7 @@ public Throwable getException() {
9780
}
9881

9982
public void setException(Throwable e) {
100-
this.exception = e;
83+
this.exception = handleStackTraceNull(e);
10184
}
10285

10386
@Override
@@ -109,4 +92,37 @@ public boolean hasException() {
10992
public String toString() {
11093
return "RpcResult [result=" + result + ", exception=" + exception + "]";
11194
}
95+
96+
/**
97+
* we need to deal the exception whose stack trace is null.
98+
* <p>
99+
* see https://github.com/apache/incubator-dubbo/pull/2956
100+
* and https://github.com/apache/incubator-dubbo/pull/3634
101+
* and https://github.com/apache/incubator-dubbo/issues/619
102+
*
103+
* @param e exception
104+
* @return exception after deal with stack trace
105+
*/
106+
private Throwable handleStackTraceNull(Throwable e) {
107+
if (e != null) {
108+
try {
109+
// get Throwable class
110+
Class clazz = e.getClass();
111+
while (!clazz.getName().equals(Throwable.class.getName())) {
112+
clazz = clazz.getSuperclass();
113+
}
114+
// get stackTrace value
115+
Field stackTraceField = clazz.getDeclaredField("stackTrace");
116+
stackTraceField.setAccessible(true);
117+
Object stackTrace = stackTraceField.get(e);
118+
if (stackTrace == null) {
119+
e.setStackTrace(new StackTraceElement[0]);
120+
}
121+
} catch (Throwable t) {
122+
// ignore
123+
}
124+
}
125+
126+
return e;
127+
}
112128
}

dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/RpcResultTest.java

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,28 +20,62 @@
2020
import org.junit.jupiter.api.Assertions;
2121
import org.junit.jupiter.api.Test;
2222

23-
import static org.junit.jupiter.api.Assertions.fail;
24-
2523
public class RpcResultTest {
2624
@Test
27-
public void testRecreateWithNormalException() {
25+
public void testRpcResultWithNormalException() {
2826
NullPointerException npe = new NullPointerException();
2927
RpcResult rpcResult = new RpcResult(npe);
30-
try {
31-
rpcResult.recreate();
32-
fail();
33-
} catch (Throwable throwable) {
34-
StackTraceElement[] stackTrace = throwable.getStackTrace();
35-
Assertions.assertNotNull(stackTrace);
36-
Assertions.assertTrue(stackTrace.length > 1);
28+
29+
StackTraceElement[] stackTrace = rpcResult.getException().getStackTrace();
30+
Assertions.assertNotNull(stackTrace);
31+
Assertions.assertTrue(stackTrace.length > 1);
32+
}
33+
34+
/**
35+
* please run this test in Run mode
36+
*/
37+
@Test
38+
public void testRpcResultWithEmptyStackTraceException() {
39+
Throwable throwable = buildEmptyStackTraceException();
40+
if (throwable == null) {
41+
return;
3742
}
43+
RpcResult rpcResult = new RpcResult(throwable);
44+
45+
StackTraceElement[] stackTrace = rpcResult.getException().getStackTrace();
46+
Assertions.assertNotNull(stackTrace);
47+
Assertions.assertTrue(stackTrace.length == 0);
48+
}
49+
50+
@Test
51+
public void testSetExceptionWithNormalException() {
52+
NullPointerException npe = new NullPointerException();
53+
RpcResult rpcResult = new RpcResult();
54+
rpcResult.setException(npe);
55+
56+
StackTraceElement[] stackTrace = rpcResult.getException().getStackTrace();
57+
Assertions.assertNotNull(stackTrace);
58+
Assertions.assertTrue(stackTrace.length > 1);
3859
}
3960

4061
/**
4162
* please run this test in Run mode
4263
*/
4364
@Test
44-
public void testRecreateWithEmptyStackTraceException() {
65+
public void testSetExceptionWithEmptyStackTraceException() {
66+
Throwable throwable = buildEmptyStackTraceException();
67+
if (throwable == null) {
68+
return;
69+
}
70+
RpcResult rpcResult = new RpcResult();
71+
rpcResult.setException(throwable);
72+
73+
StackTraceElement[] stackTrace = rpcResult.getException().getStackTrace();
74+
Assertions.assertNotNull(stackTrace);
75+
Assertions.assertTrue(stackTrace.length == 0);
76+
}
77+
78+
private Throwable buildEmptyStackTraceException() {
4579
// begin to construct a NullPointerException with empty stackTrace
4680
Throwable throwable = null;
4781
Long begin = System.currentTimeMillis();
@@ -59,19 +93,11 @@ public void testRecreateWithEmptyStackTraceException() {
5993
* may be there is -XX:-OmitStackTraceInFastThrow or run in Debug mode
6094
*/
6195
if (throwable == null) {
62-
System.out.println("###testRecreateWithEmptyStackTraceException fail to construct NPE");
63-
return;
96+
System.out.println("###buildEmptyStackTraceException fail to construct NPE");
97+
return null;
6498
}
6599
// end construct a NullPointerException with empty stackTrace
66100

67-
RpcResult rpcResult = new RpcResult(throwable);
68-
try {
69-
rpcResult.recreate();
70-
fail();
71-
} catch (Throwable t) {
72-
StackTraceElement[] stackTrace = t.getStackTrace();
73-
Assertions.assertNotNull(stackTrace);
74-
Assertions.assertTrue(stackTrace.length == 0);
75-
}
101+
return throwable;
76102
}
77103
}

0 commit comments

Comments
 (0)