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 2018/08/14 08:30:08 UTC

[incubator-servicecomb-java-chassis] 01/03: [SCB-827] add response decoding error log

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/incubator-servicecomb-java-chassis.git

commit 06ad3128b2d57840d57342f0bad8d4d1c0421076
Author: yaohaishi <ya...@huawei.com>
AuthorDate: Thu Aug 9 19:47:45 2018 +0800

    [SCB-827] add response decoding error log
---
 .../rest/client/http/DefaultHttpClientFilter.java  | 11 +++--
 .../client/http/TestDefaultHttpClientFilter.java   | 57 +++++++++++++++++++++-
 2 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/DefaultHttpClientFilter.java b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/DefaultHttpClientFilter.java
index bfaac00..659ba2a 100644
--- a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/DefaultHttpClientFilter.java
+++ b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/DefaultHttpClientFilter.java
@@ -33,8 +33,11 @@ import org.apache.servicecomb.swagger.invocation.Response;
 import org.apache.servicecomb.swagger.invocation.exception.ExceptionFactory;
 import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 import org.apache.servicecomb.swagger.invocation.response.ResponseMeta;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class DefaultHttpClientFilter implements HttpClientFilter {
+  private static final Logger LOGGER = LoggerFactory.getLogger(DefaultHttpClientFilter.class);
 
   @Override
   public int getOrder() {
@@ -61,7 +64,7 @@ public class DefaultHttpClientFilter implements HttpClientFilter {
     return restOperation.findProduceProcessor(contentTypeForFind);
   }
 
-  protected Object extractResult(Invocation invocation, HttpServletResponseEx responseEx) {
+  protected Object extractResponse(Invocation invocation, HttpServletResponseEx responseEx) {
     Object result = invocation.getHandlerContext().get(RestConst.READ_STREAM_PART);
     if (result != null) {
       return result;
@@ -79,19 +82,21 @@ public class DefaultHttpClientFilter implements HttpClientFilter {
               responseEx.getStatus(),
               responseEx.getStatusType().getReasonPhrase(),
               responseEx.getHeader(HttpHeaders.CONTENT_TYPE));
+      LOGGER.error(msg);
       return ExceptionFactory.createConsumerException(new InvocationException(responseEx.getStatus(), responseEx.getStatusType().getReasonPhrase(), msg));
     }
 
     try {
       return produceProcessor.decodeResponse(responseEx.getBodyBuffer(), responseMeta.getJavaType());
     } catch (Exception e) {
-      return ExceptionFactory.createConsumerException(e);
+      LOGGER.error("failed to decode response body", e);
+      throw ExceptionFactory.createConsumerException(e);
     }
   }
 
   @Override
   public Response afterReceiveResponse(Invocation invocation, HttpServletResponseEx responseEx) {
-    Object result = extractResult(invocation, responseEx);
+    Object result = extractResponse(invocation, responseEx);
 
     Response response =
         Response.create(responseEx.getStatusType(), result);
diff --git a/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestDefaultHttpClientFilter.java b/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestDefaultHttpClientFilter.java
index 542e82c..3088b40 100644
--- a/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestDefaultHttpClientFilter.java
+++ b/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestDefaultHttpClientFilter.java
@@ -17,7 +17,10 @@
 
 package org.apache.servicecomb.transport.rest.client.http;
 
+import static org.junit.Assert.fail;
+
 import java.util.Arrays;
+import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -25,6 +28,7 @@ import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Response.Status;
 
 import org.apache.servicecomb.common.rest.RestConst;
+import org.apache.servicecomb.common.rest.codec.produce.ProduceJsonProcessor;
 import org.apache.servicecomb.common.rest.codec.produce.ProduceProcessor;
 import org.apache.servicecomb.common.rest.definition.RestOperationMeta;
 import org.apache.servicecomb.core.Invocation;
@@ -37,10 +41,16 @@ import org.apache.servicecomb.swagger.invocation.response.ResponseMeta;
 import org.junit.Assert;
 import org.junit.Test;
 
+import com.fasterxml.jackson.core.JsonParseException;
+import com.fasterxml.jackson.databind.type.SimpleType;
+
 import io.vertx.core.MultiMap;
 import io.vertx.core.buffer.Buffer;
+import io.vertx.core.buffer.impl.BufferImpl;
 import io.vertx.core.http.CaseInsensitiveHeaders;
 import mockit.Expectations;
+import mockit.Mock;
+import mockit.MockUp;
 import mockit.Mocked;
 
 public class TestDefaultHttpClientFilter {
@@ -105,14 +115,57 @@ public class TestDefaultHttpClientFilter {
       }
     };
 
-    Assert.assertSame(part, filter.extractResult(invocation, null));
+    Assert.assertSame(part, filter.extractResponse(invocation, null));
+  }
+
+  @Test
+  public void extractResult_decodeError(@Mocked Invocation invocation, @Mocked ReadStreamPart part,
+      @Mocked OperationMeta operationMeta, @Mocked ResponseMeta responseMeta,
+      @Mocked RestOperationMeta swaggerRestOperation,
+      @Mocked HttpServletResponseEx responseEx) {
+    Map<String, Object> handlerContext = new HashMap<>();
+    new Expectations() {
+      {
+        invocation.getHandlerContext();
+        result = handlerContext;
+        invocation.getOperationMeta();
+        result = operationMeta;
+        operationMeta.findResponseMeta(200);
+        result = responseMeta;
+        operationMeta.getExtData(RestConst.SWAGGER_REST_OPERATION);
+        result = swaggerRestOperation;
+        responseMeta.getJavaType();
+        result = SimpleType.constructUnsafe(Date.class);
+        responseEx.getStatus();
+        result = 200;
+        responseEx.getBodyBuffer();
+        result = new BufferImpl().appendString("abc");
+      }
+    };
+    new MockUp<DefaultHttpClientFilter>() {
+      @Mock
+      ProduceProcessor findProduceProcessor(RestOperationMeta restOperation, HttpServletResponseEx responseEx) {
+        return new ProduceJsonProcessor();
+      }
+    };
+    try {
+      filter.extractResponse(invocation, responseEx);
+      fail("an exception is expected!");
+    } catch (Exception e) {
+      Assert.assertEquals(InvocationException.class, e.getClass());
+      Assert.assertEquals(JsonParseException.class, ((InvocationException) e).getErrorData().getClass());
+      JsonParseException jsonParseException = (JsonParseException) ((InvocationException) e).getErrorData();
+      Assert.assertEquals("Unrecognized token 'abc': was expecting ('true', 'false' or 'null')\n"
+              + " at [Source: (org.apache.servicecomb.foundation.vertx.stream.BufferInputStream); line: 1, column: 7]",
+          jsonParseException.getMessage());
+    }
   }
 
   @Test
   public void testAfterReceiveResponseNullProduceProcessor(@Mocked Invocation invocation,
       @Mocked HttpServletResponseEx responseEx,
       @Mocked OperationMeta operationMeta,
-      @Mocked RestOperationMeta swaggerRestOperation) throws Exception {
+      @Mocked RestOperationMeta swaggerRestOperation) {
     new Expectations() {
       {
         invocation.getOperationMeta();