From 342e4f5bb6420691044e7f39d232d9e232543ff6 Mon Sep 17 00:00:00 2001 From: earthchen Date: Wed, 14 Dec 2022 21:34:56 +0800 Subject: [PATCH] fix tri filter onError (#11133) * fix tri filter onError * fix tri filter onError * fix tri filter onError * fix ut --- .../rpc/protocol/tri/DeadlineFuture.java | 19 +++++++++++-------- .../rpc/protocol/tri/DeadlineFutureTest.java | 11 +++-------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/DeadlineFuture.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/DeadlineFuture.java index 113dab51c2..f2985759fd 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/DeadlineFuture.java +++ b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/DeadlineFuture.java @@ -115,7 +115,7 @@ public class DeadlineFuture extends CompletableFuture { @Override public boolean cancel(boolean mayInterruptIfRunning) { timeoutTask.cancel(); - doReceived(TriRpcStatus.CANCELLED, null); + doReceived(TriRpcStatus.CANCELLED, new AppResponse(TriRpcStatus.CANCELLED.asException())); return true; } @@ -127,12 +127,13 @@ public class DeadlineFuture extends CompletableFuture { if (isDone() || isCancelled() || isCompletedExceptionally()) { return; } - if (status.isOk()) { - this.complete(appResponse); - } else { - this.completeExceptionally( - status.appendDescription("RemoteAddress:" + address).asException()); - } + // Still needs to be discussed here, but for now, that's it + // Remove the judgment of status is ok, + // because the completelyExceptionally method will lead to the onError method in the filter, + // but there are also exceptions in the onResponse in the filter,which is a bit confusing. + // We recommend only handling onResponse in which onError is called for handling + this.complete(appResponse); + // the result is returning, but the caller thread may still waiting // to avoid endless waiting for whatever reason, notify caller thread to return. @@ -178,7 +179,9 @@ public class DeadlineFuture extends CompletableFuture { private void notifyTimeout() { final TriRpcStatus status = TriRpcStatus.DEADLINE_EXCEEDED.withDescription( getTimeoutMessage()); - DeadlineFuture.this.doReceived(status, null); + AppResponse timeoutResponse = new AppResponse(); + timeoutResponse.setException(status.asException()); + DeadlineFuture.this.doReceived(status, timeoutResponse); } } diff --git a/dubbo-rpc/dubbo-rpc-triple/src/test/java/org/apache/dubbo/rpc/protocol/tri/DeadlineFutureTest.java b/dubbo-rpc/dubbo-rpc-triple/src/test/java/org/apache/dubbo/rpc/protocol/tri/DeadlineFutureTest.java index be5af72e37..09d03513eb 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/test/java/org/apache/dubbo/rpc/protocol/tri/DeadlineFutureTest.java +++ b/dubbo-rpc/dubbo-rpc-triple/src/test/java/org/apache/dubbo/rpc/protocol/tri/DeadlineFutureTest.java @@ -28,8 +28,6 @@ import org.junit.jupiter.api.Test; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; -import static org.junit.jupiter.api.Assertions.fail; - class DeadlineFutureTest { @Test @@ -40,12 +38,9 @@ class DeadlineFutureTest { DeadlineFuture timeout = DeadlineFuture.newFuture(service, method, address, 10, ImmediateEventExecutor.INSTANCE); TimeUnit.MILLISECONDS.sleep(20); - try { - timeout.get(); - fail(); - } catch (ExecutionException e) { - Assertions.assertTrue(e.getCause() instanceof StatusRpcException); - } + AppResponse timeoutResponse = timeout.get(); + Assertions.assertTrue(timeoutResponse.getException() instanceof StatusRpcException); + DeadlineFuture success = DeadlineFuture.newFuture(service, method, address, 1000, ImmediateEventExecutor.INSTANCE);