From ba4276687b6cb1dc12be88fdc23b958c4d26cb9a Mon Sep 17 00:00:00 2001 From: yzhfd Date: Mon, 22 Sep 2025 17:46:36 +0800 Subject: [PATCH 1/9] =?UTF-8?q?=E4=BF=AE=E5=A4=8Dhttp=20timeout=E5=90=8E?= =?UTF-8?q?=E9=87=8D=E5=A4=8Dresponse=E7=9A=84bug?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../trpc/proto/http/server/AbstractHttpExecutor.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java index 73eeb11f9..ce471fed3 100644 --- a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java +++ b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java @@ -58,6 +58,8 @@ public abstract class AbstractHttpExecutor { private static final Logger logger = LoggerFactory.getLogger(AbstractHttpExecutor.class); + private volatile boolean isTimeout = false; + /** * Http codec tool */ @@ -82,6 +84,7 @@ protected void execute(HttpServletRequest request, HttpServletResponse response, requestTimeout = methodInfoAndInvoker.getInvoker().getConfig().getRequestTimeout(); } if (requestTimeout > 0 && !countDownLatch.await(requestTimeout, TimeUnit.MILLISECONDS)) { + isTimeout = true; throw TRpcException.newFrameException(ErrorCode.TRPC_SERVER_TIMEOUT_ERR, "wait http request execute timeout"); } else { @@ -128,6 +131,10 @@ private void invokeRpcRequest(ProviderInvoker invoker, DefRequest rpcRequest, CompletionStage future = invoker.invoke(rpcRequest); future.whenComplete((result, t) -> { try { + if (isTimeout) { + return; + } + // Throw the call exception, which will be handled uniformly by the exception handling program. if (t != null) { throw t; From 482db7c0395be2cb251019f9d6e63c29a1e24566 Mon Sep 17 00:00:00 2001 From: yzhfd Date: Fri, 10 Oct 2025 16:24:59 +0800 Subject: [PATCH 2/9] =?UTF-8?q?=E9=87=8D=E6=9E=84=E4=B8=80=E4=B8=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../proto/http/server/AbstractHttpExecutor.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java index ce471fed3..a0e431e61 100644 --- a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java +++ b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java @@ -43,6 +43,7 @@ import java.nio.charset.StandardCharsets; import java.util.Enumeration; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.CompletionStage; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -58,8 +59,6 @@ public abstract class AbstractHttpExecutor { private static final Logger logger = LoggerFactory.getLogger(AbstractHttpExecutor.class); - private volatile boolean isTimeout = false; - /** * Http codec tool */ @@ -68,6 +67,7 @@ public abstract class AbstractHttpExecutor { protected void execute(HttpServletRequest request, HttpServletResponse response, RpcMethodInfoAndInvoker methodInfoAndInvoker) { + AtomicBoolean isTimeout = new AtomicBoolean(false); try { DefRequest rpcRequest = buildDefRequest(request, response, methodInfoAndInvoker); @@ -75,7 +75,7 @@ protected void execute(HttpServletRequest request, HttpServletResponse response, CountDownLatch countDownLatch = new CountDownLatch(1); // use a thread pool for asynchronous processing - invokeRpcRequest(methodInfoAndInvoker.getInvoker(), rpcRequest, countDownLatch); + invokeRpcRequest(methodInfoAndInvoker.getInvoker(), rpcRequest, countDownLatch, isTimeout); // If the request carries a timeout, use this timeout to wait for the request to be processed. // If not carried, use the default timeout. @@ -84,7 +84,7 @@ protected void execute(HttpServletRequest request, HttpServletResponse response, requestTimeout = methodInfoAndInvoker.getInvoker().getConfig().getRequestTimeout(); } if (requestTimeout > 0 && !countDownLatch.await(requestTimeout, TimeUnit.MILLISECONDS)) { - isTimeout = true; + isTimeout.set(true); throw TRpcException.newFrameException(ErrorCode.TRPC_SERVER_TIMEOUT_ERR, "wait http request execute timeout"); } else { @@ -112,7 +112,8 @@ protected void execute(HttpServletRequest request, HttpServletResponse response, * * @param countDownLatch latch used to wait for the request processing */ - private void invokeRpcRequest(ProviderInvoker invoker, DefRequest rpcRequest, CountDownLatch countDownLatch) { + private void invokeRpcRequest(ProviderInvoker invoker, DefRequest rpcRequest, CountDownLatch countDownLatch, + AtomicBoolean isTimeout) { WorkerPool workerPool = invoker.getConfig().getWorkerPoolObj(); @@ -131,7 +132,7 @@ private void invokeRpcRequest(ProviderInvoker invoker, DefRequest rpcRequest, CompletionStage future = invoker.invoke(rpcRequest); future.whenComplete((result, t) -> { try { - if (isTimeout) { + if (isTimeout.get()) { return; } From bf74bfc2dff1204517df29db888882e63dd1eb63 Mon Sep 17 00:00:00 2001 From: yzhfd Date: Fri, 10 Oct 2025 17:36:03 +0800 Subject: [PATCH 3/9] =?UTF-8?q?=E9=87=8D=E6=9E=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../http/server/AbstractHttpExecutor.java | 153 +++++++++++------- 1 file changed, 93 insertions(+), 60 deletions(-) diff --git a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java index a0e431e61..3f2033d62 100644 --- a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java +++ b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java @@ -38,11 +38,14 @@ import com.tencent.trpc.proto.http.common.RpcServerContextWithHttp; import com.tencent.trpc.proto.http.common.TrpcServletRequestWrapper; import com.tencent.trpc.proto.http.common.TrpcServletResponseWrapper; +import java.io.IOException; import java.lang.reflect.Type; import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; import java.util.Enumeration; import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.CompletionStage; import java.util.concurrent.CountDownLatch; @@ -66,36 +69,33 @@ public abstract class AbstractHttpExecutor { protected void execute(HttpServletRequest request, HttpServletResponse response, RpcMethodInfoAndInvoker methodInfoAndInvoker) { - - AtomicBoolean isTimeout = new AtomicBoolean(false); + AtomicBoolean responded = new AtomicBoolean(false); try { - DefRequest rpcRequest = buildDefRequest(request, response, methodInfoAndInvoker); - - CountDownLatch countDownLatch = new CountDownLatch(1); - + CompletableFuture completionFuture = new CompletableFuture<>(); // use a thread pool for asynchronous processing - invokeRpcRequest(methodInfoAndInvoker.getInvoker(), rpcRequest, countDownLatch, isTimeout); - - // If the request carries a timeout, use this timeout to wait for the request to be processed. - // If not carried, use the default timeout. + invokeRpcRequest(methodInfoAndInvoker.getInvoker(), rpcRequest, completionFuture, responded); long requestTimeout = rpcRequest.getMeta().getTimeout(); if (requestTimeout <= 0) { requestTimeout = methodInfoAndInvoker.getInvoker().getConfig().getRequestTimeout(); } - if (requestTimeout > 0 && !countDownLatch.await(requestTimeout, TimeUnit.MILLISECONDS)) { - isTimeout.set(true); - throw TRpcException.newFrameException(ErrorCode.TRPC_SERVER_TIMEOUT_ERR, - "wait http request execute timeout"); + if (requestTimeout > 0) { + try { + completionFuture.get(requestTimeout, TimeUnit.MILLISECONDS); + } catch (TimeoutException ex) { + if (responded.compareAndSet(false, true)) { + sendTimeoutResponse(request, response); + } + } } else { - countDownLatch.await(); + completionFuture.get(); } - } catch (Exception ex) { logger.error("dispatch request [{}] error", request, ex); - doErrorReply(request, response, ex); + if (responded.compareAndSet(false, true)) { + doErrorReply(request, response, ex); + } } - } /** @@ -110,58 +110,91 @@ protected void execute(HttpServletRequest request, HttpServletResponse response, /** * Request processing * - * @param countDownLatch latch used to wait for the request processing + * @param invoker the invoker + * @param rpcRequest the rpc request + * @param completionFuture the completion future + * @param responded the responded flag */ - private void invokeRpcRequest(ProviderInvoker invoker, DefRequest rpcRequest, CountDownLatch countDownLatch, - AtomicBoolean isTimeout) { + private void invokeRpcRequest(ProviderInvoker invoker, DefRequest rpcRequest, + CompletableFuture completionFuture, + AtomicBoolean responded) { WorkerPool workerPool = invoker.getConfig().getWorkerPoolObj(); - - if (null == workerPool) { - logger.error("dispatch rpcRequest [{}] error, workerPool is empty", rpcRequest); - throw TRpcException.newFrameException(ErrorCode.TRPC_SERVER_NOSERVICE_ERR, - "not found service, workerPool is empty"); + if (workerPool == null) { + logger.error("Worker pool is not available"); + completionFuture.completeExceptionally( + TRpcException.newFrameException(ErrorCode.TRPC_SERVER_NOSERVICE_ERR, "Worker pool not available") + ); + return; } workerPool.execute(() -> { - - // Get the original http response - HttpServletResponse response = getOriginalResponse(rpcRequest); - - // Invoke the routing implementation method to handle the request. - CompletionStage future = invoker.invoke(rpcRequest); - future.whenComplete((result, t) -> { - try { - if (isTimeout.get()) { - return; + try { + HttpServletResponse response = getOriginalResponse(rpcRequest); + + CompletionStage rpcFuture = invoker.invoke(rpcRequest); + + rpcFuture.whenComplete((result, throwable) -> { + try { + if (responded.get()) { + return; + } + + if (throwable != null) { + throw throwable; + } + + if (result.getException() != null) { + throw result.getException(); + } + + if (responded.compareAndSet(false, true)) { + response.setStatus(HttpStatus.SC_OK); + httpCodec.writeHttpResponse(response, result); + response.flushBuffer(); + } + + completionFuture.complete(null); + } catch (Throwable t) { + handleError(t, rpcRequest, response, responded, completionFuture); } + }); - // Throw the call exception, which will be handled uniformly by the exception handling program. - if (t != null) { - throw t; - } + } catch (Exception e) { + handleError(e, rpcRequest, getOriginalResponse(rpcRequest), responded, completionFuture); + } + }); + } - // Throw a business logic exception, which will be handled uniformly - // by the exception handling program. - Throwable ex = result.getException(); - if (ex != null) { - throw ex; - } + /** + * Handle error + */ + private void handleError(Throwable t, DefRequest rpcRequest, HttpServletResponse response, + AtomicBoolean responded, CompletableFuture completionFuture) { + try { + if (responded.compareAndSet(false, true)) { + HttpServletRequest request = getOriginalRequest(rpcRequest); + logger.warn("Request processing failed: {}", request.getRequestURI(), t); + httpErrorReply(request, response, + ErrorResponse.create(request, HttpStatus.SC_INTERNAL_SERVER_ERROR, t)); + } + } finally { + completionFuture.completeExceptionally(t); + } + } - // normal response - response.setStatus(HttpStatus.SC_OK); - httpCodec.writeHttpResponse(response, result); - response.flushBuffer(); - } catch (Throwable e) { - HttpServletRequest request = getOriginalRequest(rpcRequest); - logger.warn("reply message error, channel: [{}], msg:[{}]", request.getRemoteAddr(), request, e); - httpErrorReply(request, response, - ErrorResponse.create(request, HttpStatus.SC_SERVICE_UNAVAILABLE, e)); - } finally { - countDownLatch.countDown(); - } - }); - }); + /** + * Send timeout response + */ + private void sendTimeoutResponse(HttpServletRequest request, HttpServletResponse response) { + try { + response.setStatus(HttpStatus.SC_GATEWAY_TIMEOUT); + response.getWriter().write("Request Timeout"); + response.flushBuffer(); + logger.warn("Request timeout: {} {}", request.getMethod(), request.getRequestURI()); + } catch (IOException e) { + logger.error("Failed to send timeout response", e); + } } /** From 64546c09a49ca78dee6e00a7f30eea03b21029ad Mon Sep 17 00:00:00 2001 From: yzhfd Date: Fri, 10 Oct 2025 17:46:57 +0800 Subject: [PATCH 4/9] =?UTF-8?q?=E9=87=8D=E6=9E=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../http/server/AbstractHttpExecutor.java | 27 +++++++------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java index 3f2033d62..5552eb64d 100644 --- a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java +++ b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java @@ -73,8 +73,12 @@ protected void execute(HttpServletRequest request, HttpServletResponse response, try { DefRequest rpcRequest = buildDefRequest(request, response, methodInfoAndInvoker); CompletableFuture completionFuture = new CompletableFuture<>(); + // use a thread pool for asynchronous processing invokeRpcRequest(methodInfoAndInvoker.getInvoker(), rpcRequest, completionFuture, responded); + + // If the request carries a timeout, use this timeout to wait for the request to be processed. + // If not carried, use the default timeout. long requestTimeout = rpcRequest.getMeta().getTimeout(); if (requestTimeout <= 0) { requestTimeout = methodInfoAndInvoker.getInvoker().getConfig().getRequestTimeout(); @@ -84,7 +88,8 @@ protected void execute(HttpServletRequest request, HttpServletResponse response, completionFuture.get(requestTimeout, TimeUnit.MILLISECONDS); } catch (TimeoutException ex) { if (responded.compareAndSet(false, true)) { - sendTimeoutResponse(request, response); + doErrorReply(request, response, TRpcException.newFrameException(ErrorCode.TRPC_SERVER_TIMEOUT_ERR, + "wait http request execute timeout")); } } } else { @@ -120,10 +125,10 @@ private void invokeRpcRequest(ProviderInvoker invoker, DefRequest rpcRequest, AtomicBoolean responded) { WorkerPool workerPool = invoker.getConfig().getWorkerPoolObj(); - if (workerPool == null) { - logger.error("Worker pool is not available"); + if (null == workerPool) { + logger.error("dispatch rpcRequest [{}] error, workerPool is empty", rpcRequest); completionFuture.completeExceptionally( - TRpcException.newFrameException(ErrorCode.TRPC_SERVER_NOSERVICE_ERR, "Worker pool not available") + TRpcException.newFrameException(ErrorCode.TRPC_SERVER_NOSERVICE_ERR, "not found service, workerPool is empty") ); return; } @@ -183,20 +188,6 @@ private void handleError(Throwable t, DefRequest rpcRequest, HttpServletResponse } } - /** - * Send timeout response - */ - private void sendTimeoutResponse(HttpServletRequest request, HttpServletResponse response) { - try { - response.setStatus(HttpStatus.SC_GATEWAY_TIMEOUT); - response.getWriter().write("Request Timeout"); - response.flushBuffer(); - logger.warn("Request timeout: {} {}", request.getMethod(), request.getRequestURI()); - } catch (IOException e) { - logger.error("Failed to send timeout response", e); - } - } - /** * Build the context request. * From 76dd4b6b8ebb8d11bf0d161b891149ebe8961318 Mon Sep 17 00:00:00 2001 From: yzhfd Date: Fri, 10 Oct 2025 17:57:39 +0800 Subject: [PATCH 5/9] =?UTF-8?q?=E6=8F=90=E4=BA=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../trpc/proto/http/server/AbstractHttpExecutor.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java index 5552eb64d..4e75f4f71 100644 --- a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java +++ b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java @@ -71,7 +71,9 @@ protected void execute(HttpServletRequest request, HttpServletResponse response, RpcMethodInfoAndInvoker methodInfoAndInvoker) { AtomicBoolean responded = new AtomicBoolean(false); try { + DefRequest rpcRequest = buildDefRequest(request, response, methodInfoAndInvoker); + CompletableFuture completionFuture = new CompletableFuture<>(); // use a thread pool for asynchronous processing @@ -125,6 +127,7 @@ private void invokeRpcRequest(ProviderInvoker invoker, DefRequest rpcRequest, AtomicBoolean responded) { WorkerPool workerPool = invoker.getConfig().getWorkerPoolObj(); + if (null == workerPool) { logger.error("dispatch rpcRequest [{}] error, workerPool is empty", rpcRequest); completionFuture.completeExceptionally( @@ -179,9 +182,8 @@ private void handleError(Throwable t, DefRequest rpcRequest, HttpServletResponse try { if (responded.compareAndSet(false, true)) { HttpServletRequest request = getOriginalRequest(rpcRequest); - logger.warn("Request processing failed: {}", request.getRequestURI(), t); - httpErrorReply(request, response, - ErrorResponse.create(request, HttpStatus.SC_INTERNAL_SERVER_ERROR, t)); + logger.warn("reply message error, channel: [{}], msg:[{}]", request.getRemoteAddr(), request, t); + httpErrorReply(request, response, ErrorResponse.create(request, HttpStatus.SC_SERVICE_UNAVAILABLE, t)); } } finally { completionFuture.completeExceptionally(t); From 5fa4dd3a50b4eadfb8b12fa9dc25997756144a2e Mon Sep 17 00:00:00 2001 From: yzhfd Date: Fri, 10 Oct 2025 18:03:31 +0800 Subject: [PATCH 6/9] =?UTF-8?q?=E6=8F=90=E4=BA=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../trpc/proto/http/server/AbstractHttpExecutor.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java index 4e75f4f71..9beba1f3a 100644 --- a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java +++ b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java @@ -90,7 +90,8 @@ protected void execute(HttpServletRequest request, HttpServletResponse response, completionFuture.get(requestTimeout, TimeUnit.MILLISECONDS); } catch (TimeoutException ex) { if (responded.compareAndSet(false, true)) { - doErrorReply(request, response, TRpcException.newFrameException(ErrorCode.TRPC_SERVER_TIMEOUT_ERR, + doErrorReply(request, response, + TRpcException.newFrameException(ErrorCode.TRPC_SERVER_TIMEOUT_ERR, "wait http request execute timeout")); } } @@ -130,9 +131,8 @@ private void invokeRpcRequest(ProviderInvoker invoker, DefRequest rpcRequest, if (null == workerPool) { logger.error("dispatch rpcRequest [{}] error, workerPool is empty", rpcRequest); - completionFuture.completeExceptionally( - TRpcException.newFrameException(ErrorCode.TRPC_SERVER_NOSERVICE_ERR, "not found service, workerPool is empty") - ); + completionFuture.completeExceptionally(TRpcException.newFrameException(ErrorCode.TRPC_SERVER_NOSERVICE_ERR, + "not found service, workerPool is empty")); return; } From a785045b2d75bf1b8a5726a82bad35bfc455082e Mon Sep 17 00:00:00 2001 From: yzhfd Date: Sat, 11 Oct 2025 15:11:43 +0800 Subject: [PATCH 7/9] =?UTF-8?q?=E4=BC=98=E5=8C=96=E6=B3=A8=E9=87=8A?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../trpc/proto/http/server/AbstractHttpExecutor.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java index 9beba1f3a..f4a39dab8 100644 --- a/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java +++ b/trpc-proto/trpc-proto-http/src/main/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutor.java @@ -138,8 +138,9 @@ private void invokeRpcRequest(ProviderInvoker invoker, DefRequest rpcRequest, workerPool.execute(() -> { try { + // Get the original http response HttpServletResponse response = getOriginalResponse(rpcRequest); - + // Invoke the routing implementation method to handle the request. CompletionStage rpcFuture = invoker.invoke(rpcRequest); rpcFuture.whenComplete((result, throwable) -> { @@ -148,14 +149,18 @@ private void invokeRpcRequest(ProviderInvoker invoker, DefRequest rpcRequest, return; } + // Throw the call exception, which will be handled uniformly by the exception handling program. if (throwable != null) { throw throwable; } + // Throw a business logic exception, which will be handled uniformly + // by the exception handling program. if (result.getException() != null) { throw result.getException(); } + // normal response if (responded.compareAndSet(false, true)) { response.setStatus(HttpStatus.SC_OK); httpCodec.writeHttpResponse(response, result); From e54f370e7a3d65282cda3527726e4ca58549f1a2 Mon Sep 17 00:00:00 2001 From: yzhfd Date: Sun, 12 Oct 2025 12:15:39 +0800 Subject: [PATCH 8/9] =?UTF-8?q?=E5=A2=9E=E5=8A=A0=E5=8D=95=E6=B5=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../http/server/AbstractHttpExecutorTest.java | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/trpc-proto/trpc-proto-http/src/test/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutorTest.java b/trpc-proto/trpc-proto-http/src/test/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutorTest.java index 2abbe5643..b86146606 100644 --- a/trpc-proto/trpc-proto-http/src/test/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutorTest.java +++ b/trpc-proto/trpc-proto-http/src/test/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutorTest.java @@ -13,16 +13,32 @@ import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.powermock.api.mockito.PowerMockito.doReturn; import static org.powermock.api.mockito.PowerMockito.mock; import static org.powermock.api.mockito.PowerMockito.when; import com.tencent.trpc.core.rpc.RpcInvocation; import com.tencent.trpc.core.rpc.common.RpcMethodInfo; +import com.tencent.trpc.core.rpc.def.DefRequest; +import com.tencent.trpc.proto.http.common.ErrorResponse; import com.tencent.trpc.proto.http.common.HttpConstants; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.junit.Test; import org.junit.runner.RunWith; +import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PowerMockIgnore; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; @@ -48,4 +64,121 @@ public void buildRpcInvocation_shouldSuccess() throws Exception { methodInfo); assertEquals(rpcInvocation.getFunc(), "/trpc.demo.server/hello"); } + + @Test + public void handleError_shouldCallHttpErrorReply_whenRespondedIsFalse() throws Exception { + // Arrange + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + DefRequest rpcRequest = mock(DefRequest.class); + AtomicBoolean responded = new AtomicBoolean(false); + CompletableFuture completionFuture = new CompletableFuture<>(); + Throwable throwable = new RuntimeException("test error"); + + AbstractHttpExecutor abstractHttpExecutor = mock(AbstractHttpExecutor.class); + + // Mock getOriginalRequest method + Map attachments = new HashMap<>(); + attachments.put(HttpConstants.TRPC_ATTACH_SERVLET_REQUEST, request); + when(rpcRequest.getAttachments()).thenReturn(attachments); + when(request.getRemoteAddr()).thenReturn("127.0.0.1"); + + // Mock httpErrorReply method + PowerMockito.doNothing().when(abstractHttpExecutor, "httpErrorReply", any(HttpServletRequest.class), + any(HttpServletResponse.class), any(ErrorResponse.class)); + + // Call real method + when(abstractHttpExecutor, "handleError", throwable, rpcRequest, response, responded, completionFuture) + .thenCallRealMethod(); + + // Act + Whitebox.invokeMethod(abstractHttpExecutor, "handleError", throwable, rpcRequest, response, + responded, completionFuture); + + // Assert + assertTrue("responded should be set to true", responded.get()); + assertTrue("completionFuture should be completed exceptionally", completionFuture.isCompletedExceptionally()); + + // Verify httpErrorReply was called + PowerMockito.verifyPrivate(abstractHttpExecutor, times(1)) + .invoke("httpErrorReply", eq(request), eq(response), any(ErrorResponse.class)); + } + + @Test + public void handleError_shouldNotCallHttpErrorReply_whenRespondedIsTrue() throws Exception { + // Arrange + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + DefRequest rpcRequest = mock(DefRequest.class); + AtomicBoolean responded = new AtomicBoolean(true); // Already responded + CompletableFuture completionFuture = new CompletableFuture<>(); + Throwable throwable = new RuntimeException("test error"); + + AbstractHttpExecutor abstractHttpExecutor = mock(AbstractHttpExecutor.class); + + // Mock httpErrorReply method + PowerMockito.doNothing().when(abstractHttpExecutor, "httpErrorReply", any(HttpServletRequest.class), + any(HttpServletResponse.class), any(ErrorResponse.class)); + + // Call real method + when(abstractHttpExecutor, "handleError", throwable, rpcRequest, response, responded, completionFuture) + .thenCallRealMethod(); + + // Act + Whitebox.invokeMethod(abstractHttpExecutor, "handleError", throwable, rpcRequest, response, + responded, completionFuture); + + // Assert + assertTrue("responded should remain true", responded.get()); + assertTrue("completionFuture should be completed exceptionally", completionFuture.isCompletedExceptionally()); + + // Verify httpErrorReply was NOT called + PowerMockito.verifyPrivate(abstractHttpExecutor, never()) + .invoke("httpErrorReply", any(HttpServletRequest.class), any(HttpServletResponse.class), + any(ErrorResponse.class)); + } + + @Test + public void handleError_shouldCompleteExceptionally_evenWhenHttpErrorReplyThrows() throws Exception { + // Arrange + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + DefRequest rpcRequest = mock(DefRequest.class); + AtomicBoolean responded = new AtomicBoolean(false); + CompletableFuture completionFuture = new CompletableFuture<>(); + Throwable throwable = new RuntimeException("test error"); + + AbstractHttpExecutor abstractHttpExecutor = mock(AbstractHttpExecutor.class); + + // Mock getOriginalRequest method + Map attachments = new HashMap<>(); + attachments.put(HttpConstants.TRPC_ATTACH_SERVLET_REQUEST, request); + when(rpcRequest.getAttachments()).thenReturn(attachments); + when(request.getRemoteAddr()).thenReturn("127.0.0.1"); + + // Mock httpErrorReply method to throw exception + PowerMockito.doThrow(new IOException("response error")).when(abstractHttpExecutor, "httpErrorReply", + any(HttpServletRequest.class), any(HttpServletResponse.class), any(ErrorResponse.class)); + + // Call real method + when(abstractHttpExecutor, "handleError", throwable, rpcRequest, response, responded, completionFuture) + .thenCallRealMethod(); + + // Act + Whitebox.invokeMethod(abstractHttpExecutor, "handleError", throwable, rpcRequest, response, + responded, completionFuture); + + // Assert + assertTrue("responded should be set to true", responded.get()); + assertTrue("completionFuture should be completed exceptionally", completionFuture.isCompletedExceptionally()); + + // Verify the original throwable is used for completing exceptionally + try { + completionFuture.get(); + fail("Should have thrown exception"); + } catch (ExecutionException e) { + assertEquals("Should complete with original throwable", throwable, e.getCause()); + } + } + } \ No newline at end of file From 3ebcc398f284b0449ea9f0ca7d3202b530dfe49d Mon Sep 17 00:00:00 2001 From: "yanzhenfd@163.com" Date: Sun, 12 Oct 2025 12:19:25 +0800 Subject: [PATCH 9/9] =?UTF-8?q?=E4=BF=AE=E6=94=B9user.name=E4=B8=BA?= =?UTF-8?q?=E9=82=AE=E7=AE=B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../tencent/trpc/proto/http/server/AbstractHttpExecutorTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/trpc-proto/trpc-proto-http/src/test/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutorTest.java b/trpc-proto/trpc-proto-http/src/test/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutorTest.java index b86146606..e0c0c19b2 100644 --- a/trpc-proto/trpc-proto-http/src/test/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutorTest.java +++ b/trpc-proto/trpc-proto-http/src/test/java/com/tencent/trpc/proto/http/server/AbstractHttpExecutorTest.java @@ -172,7 +172,6 @@ public void handleError_shouldCompleteExceptionally_evenWhenHttpErrorReplyThrows assertTrue("responded should be set to true", responded.get()); assertTrue("completionFuture should be completed exceptionally", completionFuture.isCompletedExceptionally()); - // Verify the original throwable is used for completing exceptionally try { completionFuture.get(); fail("Should have thrown exception");