feature: remove TripleRpcException #8712

x-shadow-man · 2021-09-07T08:58:43Z

What is the purpose of the change

Remove and support dubbo status to triple status mapping for issue #8659

Brief changelog

Verifying this change

Checklist

Make sure there is a GitHub_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
Add some description to dubbo-website project if you are requesting to add a feature.
GitHub Actions works fine on your own branch.
If this contribution is large, please follow the Software Donation Guide.

guohao · 2021-09-07T09:47:30Z

Plz fix code format

x-shadow-man · 2021-09-07T10:45:35Z

Plz fix code format

done. pls feel free to review, thx

codecov-commenter · 2021-09-07T13:08:07Z

Codecov Report

Merging #8712 (22f194a) into 3.0 (bdd253f) will increase coverage by 0.04%.
The diff coverage is 10.71%.

❗ Current head 22f194a differs from pull request most recent head e0f2871. Consider uploading reports for the commit e0f2871 to get more accurate results

@@             Coverage Diff              @@
##                3.0    #8712      +/-   ##
============================================
+ Coverage     63.69%   63.73%   +0.04%     
- Complexity      313      314       +1     
============================================
  Files          1146     1145       -1     
  Lines         48145    48160      +15     
  Branches       7252     7250       -2     
============================================
+ Hits          30668    30697      +29     
+ Misses        14111    14103       -8     
+ Partials       3366     3360       -6

Impacted Files	Coverage Δ
...c/main/java/org/apache/dubbo/rpc/RpcException.java	`83.33% <ø> (ø)`
...rg/apache/dubbo/rpc/protocol/tri/ClientStream.java	`80.00% <0.00%> (ø)`
.../org/apache/dubbo/rpc/protocol/tri/GrpcStatus.java	`41.66% <0.00%> (-3.21%)`	⬇️
...ache/dubbo/rpc/protocol/tri/UnaryClientStream.java	`29.62% <0.00%> (ø)`
.../dubbo/rpc/protocol/tri/service/TriHealthImpl.java	`33.33% <0.00%> (+0.52%)`	⬆️
...ache/dubbo/rpc/protocol/tri/UnaryServerStream.java	`47.22% <13.33%> (-4.30%)`	⬇️
...pc/protocol/tri/TripleHttp2FrameServerHandler.java	`40.90% <21.05%> (ø)`
...ava/org/apache/dubbo/registry/AddressListener.java	`0.00% <0.00%> (-100.00%)`	⬇️
...he/dubbo/rpc/cluster/router/state/RouterCache.java	`66.66% <0.00%> (-11.12%)`	⬇️
...n/java/org/apache/dubbo/common/bytecode/Proxy.java	`66.43% <0.00%> (-4.80%)`	⬇️
... and 27 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bdd253f...e0f2871. Read the comment docs.

guohao · 2021-09-08T02:36:48Z

dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/GrpcStatus.java

+                code = Code.ABORTED;
+                break;
+            case TIMEOUT_TERMINATE:
+                code = Code.OUT_OF_RANGE;


DEADLINE_EXCEEDED

guohao · 2021-09-08T02:40:34Z

dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/GrpcStatus.java

+                code = Code.PERMISSION_DENIED;
+                break;
+            case LIMIT_EXCEEDED_EXCEPTION:
+                code = Code.ABORTED;


https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md

UNAVAILABLE will be better

guohao · 2021-09-08T02:46:37Z

dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/GrpcStatus.java

+        }
+        return fromCode(code);
+    }
+


Add NETWORK_EXCEPTION -> UNAVAILABLE
SERIALIZATION_EXCEPTION -> INTERNAL

guohao · 2021-09-08T03:46:25Z

...-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/UnaryServerStream.java

-                        if (exception instanceof TripleRpcException) {
-                            transportError(((TripleRpcException) exception).getStatus(), response.getObjectAttachments());
+                        if (exception instanceof RpcException) {
+                            transportError(rpcExceptionCodeToGrpc(((RpcException) exception).getCode()), response.getObjectAttachments());


detail msg in RPCException has been dropped, should be pass to Status' description

guohao · 2021-09-08T03:48:22Z

...-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/UnaryServerStream.java

-                    if (e instanceof TripleRpcException) {
-                        transportError(((TripleRpcException) e).getStatus(), response.getObjectAttachments());
+                    if (e instanceof RpcException) {
+                        transportError(rpcExceptionCodeToGrpc(((RpcException) e).getCode()), response.getObjectAttachments());


Same issue as above

EarthChen · 2021-09-08T03:54:08Z

EarthChen · 2021-09-08T04:24:56Z

dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/GrpcStatus.java

@@ -82,6 +83,28 @@ public static byte toDubboStatus(Code code) {
        return status;
    }

+    public static GrpcStatus rpcExceptionCodeToGrpc(int rpcExceptionCode) {


Would it be better to replace it with enum？and put them in map and getValue

EarthChen

LGTM

x-shadow-man added 2 commits September 7, 2021 16:58

add feature: remove TripleRpcException

09508ed

Merge branch '3.0' into feat/remove-triple-rpc-exception

d8a9c7a

remove TripleRpcException

8a69bd1

fix import format

0b11dd4

guohao requested changes Sep 8, 2021

View reviewed changes

optimize rpcExceptionCodeToGrpc

b82c966

guohao reviewed Sep 8, 2021

View reviewed changes

EarthChen requested changes Sep 8, 2021

View reviewed changes

Fill exception msg

22f194a

EarthChen approved these changes Sep 8, 2021

View reviewed changes

guohao approved these changes Sep 8, 2021

View reviewed changes

x-shadow-man added 2 commits September 8, 2021 13:38

add msg

bc251d9

fix msg

e0f2871

guohao merged commit 3aa7168 into apache:3.0 Sep 8, 2021

wxbty deleted the feat/remove-triple-rpc-exception branch March 18, 2023 14:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: remove TripleRpcException #8712

feature: remove TripleRpcException #8712

x-shadow-man commented Sep 7, 2021

guohao commented Sep 7, 2021

x-shadow-man commented Sep 7, 2021

codecov-commenter commented Sep 7, 2021 •

edited

Loading

guohao Sep 8, 2021

guohao Sep 8, 2021

guohao Sep 8, 2021

guohao Sep 8, 2021

guohao Sep 8, 2021

EarthChen commented Sep 8, 2021

EarthChen Sep 8, 2021

EarthChen left a comment

feature: remove TripleRpcException #8712

feature: remove TripleRpcException #8712

Conversation

x-shadow-man commented Sep 7, 2021

What is the purpose of the change

Brief changelog

Verifying this change

Checklist

guohao commented Sep 7, 2021

x-shadow-man commented Sep 7, 2021

codecov-commenter commented Sep 7, 2021 • edited Loading

Codecov Report

guohao Sep 8, 2021

Choose a reason for hiding this comment

guohao Sep 8, 2021

Choose a reason for hiding this comment

guohao Sep 8, 2021

Choose a reason for hiding this comment

guohao Sep 8, 2021

Choose a reason for hiding this comment

guohao Sep 8, 2021

Choose a reason for hiding this comment

EarthChen commented Sep 8, 2021

EarthChen Sep 8, 2021

Choose a reason for hiding this comment

EarthChen left a comment

Choose a reason for hiding this comment

codecov-commenter commented Sep 7, 2021 •

edited

Loading