You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2020/02/04 07:40:13 UTC

[servicecomb-java-chassis] branch master updated: [SCB-1623]when timeout, return 408 error code and message is timeout

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

liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git


The following commit(s) were added to refs/heads/master by this push:
     new 0e59111  [SCB-1623]when timeout, return 408 error code and message is timeout
0e59111 is described below

commit 0e5911139083aa9bace4853b5db30fe4d17abc92
Author: liubao <bi...@qq.com>
AuthorDate: Tue Feb 4 11:01:10 2020 +0800

    [SCB-1623]when timeout, return 408 error code and message is timeout
---
 .../servicecomb/demo/jaxrs/client/JaxrsClient.java   |  8 ++++----
 .../it/edge/converter/CustomExceptionConverter.java  | 20 +++++++++++---------
 .../it/edge/handler/ExceptionConvertHandler.java     |  2 +-
 .../servicecomb/transport/highway/HighwayClient.java | 11 +++++++++++
 .../rest/client/http/RestClientInvocation.java       | 10 ++++++++++
 5 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/JaxrsClient.java b/demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/JaxrsClient.java
index 9779b13..971bcd6 100644
--- a/demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/JaxrsClient.java
+++ b/demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/JaxrsClient.java
@@ -514,10 +514,10 @@ public class JaxrsClient {
       template.postForObject(cseUrlPrefix + "add", params, Integer.class);
     } catch (InvocationException e) {
       isExcep = true;
-      TestMgr.check(490, e.getStatus().getStatusCode());
-      TestMgr.check(
-          "CommonExceptionData [message=Cse Internal Bad Request]",
-          e.getErrorData());
+      // implement timeout with same error code and message for rest and highway
+      TestMgr.check(408, e.getStatus().getStatusCode());
+      TestMgr.check(true,
+          e.getErrorData().toString().contains("CommonExceptionData [message=Request Timeout. Details:"));
     }
 
     TestMgr.check(true, isExcep);
diff --git a/integration-tests/it-edge/src/main/java/org/apache/servicecomb/it/edge/converter/CustomExceptionConverter.java b/integration-tests/it-edge/src/main/java/org/apache/servicecomb/it/edge/converter/CustomExceptionConverter.java
index 8678f4c..659984e 100644
--- a/integration-tests/it-edge/src/main/java/org/apache/servicecomb/it/edge/converter/CustomExceptionConverter.java
+++ b/integration-tests/it-edge/src/main/java/org/apache/servicecomb/it/edge/converter/CustomExceptionConverter.java
@@ -17,8 +17,6 @@
 
 package org.apache.servicecomb.it.edge.converter;
 
-import java.util.concurrent.TimeoutException;
-
 import javax.ws.rs.core.Response.Status;
 
 import org.apache.servicecomb.swagger.invocation.Response;
@@ -26,10 +24,10 @@ import org.apache.servicecomb.swagger.invocation.SwaggerInvocation;
 import org.apache.servicecomb.swagger.invocation.exception.ExceptionToProducerResponseConverter;
 import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 
-public class CustomExceptionConverter implements ExceptionToProducerResponseConverter<TimeoutException> {
+public class CustomExceptionConverter implements ExceptionToProducerResponseConverter<InvocationException> {
   @Override
-  public Class<TimeoutException> getExceptionClass() {
-    return TimeoutException.class;
+  public Class<InvocationException> getExceptionClass() {
+    return InvocationException.class;
   }
 
   @Override
@@ -38,9 +36,13 @@ public class CustomExceptionConverter implements ExceptionToProducerResponseConv
   }
 
   @Override
-  public Response convert(SwaggerInvocation swaggerInvocation, TimeoutException e) {
-    CustomException customException = new CustomException("change the response", 777);
-    InvocationException stt = new InvocationException(Status.EXPECTATION_FAILED, customException);
-    return Response.failResp(stt);
+  public Response convert(SwaggerInvocation swaggerInvocation, InvocationException e) {
+    if (e.getStatusCode() == 408) {
+      CustomException customException = new CustomException("change the response", 777);
+      InvocationException stt = new InvocationException(Status.EXPECTATION_FAILED, customException);
+      return Response.failResp(stt);
+    } else {
+      return Response.failResp(e);
+    }
   }
 }
diff --git a/integration-tests/it-edge/src/main/java/org/apache/servicecomb/it/edge/handler/ExceptionConvertHandler.java b/integration-tests/it-edge/src/main/java/org/apache/servicecomb/it/edge/handler/ExceptionConvertHandler.java
index 7eda483..c03fc65 100644
--- a/integration-tests/it-edge/src/main/java/org/apache/servicecomb/it/edge/handler/ExceptionConvertHandler.java
+++ b/integration-tests/it-edge/src/main/java/org/apache/servicecomb/it/edge/handler/ExceptionConvertHandler.java
@@ -33,7 +33,7 @@ public class ExceptionConvertHandler implements Handler {
     invocation.next(response -> {
       if (response.isFailed()) {
         Throwable e = response.getResult();
-        if (e instanceof TimeoutException || e.getCause() instanceof TimeoutException) {
+        if (e instanceof InvocationException && ((InvocationException)e).getStatusCode() == 408) {
           CustomException customException = new CustomException("change the response", 777);
           InvocationException stt = new InvocationException(Status.EXPECTATION_FAILED, customException);
           response.setResult(stt);
diff --git a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClient.java b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClient.java
index d8193c9..56c61e2 100644
--- a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClient.java
+++ b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClient.java
@@ -17,6 +17,10 @@
 
 package org.apache.servicecomb.transport.highway;
 
+import java.util.concurrent.TimeoutException;
+
+import javax.ws.rs.core.Response.Status;
+
 import org.apache.servicecomb.codec.protobuf.definition.OperationProtobuf;
 import org.apache.servicecomb.codec.protobuf.definition.ProtobufManager;
 import org.apache.servicecomb.core.Invocation;
@@ -31,6 +35,7 @@ import org.apache.servicecomb.foundation.vertx.client.ClientVerticle;
 import org.apache.servicecomb.foundation.vertx.client.tcp.TcpClientConfig;
 import org.apache.servicecomb.swagger.invocation.AsyncResponse;
 import org.apache.servicecomb.swagger.invocation.Response;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -109,6 +114,12 @@ public class HighwayClient {
         if (ar.failed()) {
           // 只会是本地异常
           invocation.getInvocationStageTrace().finishClientFiltersResponse();
+          if (ar.cause() instanceof TimeoutException) {
+            // give an accurate cause for timeout exception
+            asyncResp.consumerFail(new InvocationException(Status.REQUEST_TIMEOUT,
+                String.format("Request Timeout. Details: %s", ar.cause().getMessage())));
+            return;
+          }
           asyncResp.consumerFail(ar.cause());
           return;
         }
diff --git a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java
index 1b62b4e..98cc7ec 100644
--- a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java
+++ b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java
@@ -18,6 +18,9 @@
 package org.apache.servicecomb.transport.rest.client.http;
 
 import java.util.List;
+import java.util.concurrent.TimeoutException;
+
+import javax.ws.rs.core.Response.Status;
 
 import org.apache.servicecomb.common.rest.RestConst;
 import org.apache.servicecomb.common.rest.codec.param.RestClientRequestImpl;
@@ -41,6 +44,7 @@ import org.apache.servicecomb.foundation.vertx.metrics.metric.DefaultHttpSocketM
 import org.apache.servicecomb.serviceregistry.api.Const;
 import org.apache.servicecomb.swagger.invocation.AsyncResponse;
 import org.apache.servicecomb.swagger.invocation.Response;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.util.StringUtils;
@@ -277,6 +281,12 @@ public class RestClientInvocation {
     stageTrace.finishClientFiltersResponse();
 
     try {
+      if (e instanceof TimeoutException) {
+        // give an accurate cause for timeout exception
+        asyncResp.consumerFail(new InvocationException(Status.REQUEST_TIMEOUT,
+            String.format("Request Timeout. Details: %s", e.getMessage())));
+        return;
+      }
       asyncResp.fail(invocation.getInvocationType(), e);
     } catch (Throwable e1) {
       LOGGER.error(invocation.getMarker(), "failed to invoke asyncResp.fail.", e1);