[
https://issues.apache.org/jira/browse/SCB-827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16579435#comment-16579435
]
ASF GitHub Bot commented on SCB-827:
------------------------------------
liubao68 closed pull request #865: [SCB-827] Add response body decoding error
log
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/865
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/CodeFirstSpringmvcIntf.java
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/CodeFirstSpringmvcIntf.java
index c2f5cc50d..509e15c7a 100644
---
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/CodeFirstSpringmvcIntf.java
+++
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/CodeFirstSpringmvcIntf.java
@@ -27,6 +27,7 @@
import org.apache.servicecomb.demo.compute.GenericParam;
import org.apache.servicecomb.demo.compute.Person;
import org.apache.servicecomb.demo.server.User;
+import org.apache.servicecomb.demo.springmvc.decoderesponse.DecodeTestResponse;
import org.apache.servicecomb.swagger.invocation.Response;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
@@ -72,4 +73,6 @@ String checkQueryGenericString(String str,
GenericParam<Person> requestBody, lon
String testDelay();
String testAbort();
+
+ DecodeTestResponse testDecodeResponseError();
}
diff --git
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestResponse.java
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestResponse.java
index e243a2f50..0cd60284f 100644
---
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestResponse.java
+++
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestResponse.java
@@ -17,7 +17,9 @@
package org.apache.servicecomb.demo.springmvc.client;
import java.util.Date;
+import java.util.Objects;
+import org.apache.servicecomb.core.exception.CseException;
import org.apache.servicecomb.demo.TestMgr;
import org.apache.servicecomb.demo.compute.GenericParam;
import org.apache.servicecomb.demo.compute.Person;
@@ -27,6 +29,8 @@
import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
import org.springframework.http.ResponseEntity;
+import com.fasterxml.jackson.databind.exc.InvalidFormatException;
+
public class TestResponse {
private CodeFirstSpringmvcIntf intf;
@@ -39,6 +43,7 @@ public void runRest() {
checkQueryGenericString();
testDelay();
testAbort();
+ testDecodeResponseError();
}
public void runHighway() {
@@ -130,4 +135,32 @@ private void testAbort() {
+ "OK|InvocationException: code=421;msg=CommonExceptionData
[message=aborted by fault inject]|",
result.toString());
}
+
+ private void testDecodeResponseError() {
+ InvocationException exception = null;
+ try {
+ intf.testDecodeResponseError();
+ } catch (InvocationException e) {
+ // 1. InvocationException: wrapper exception
+ exception = e;
+ }
+ Objects.requireNonNull(exception);
+ // 2. CseException: bizKeeper exception
+ Throwable wrappedException = exception.getCause();
+ TestMgr.check(CseException.class, wrappedException.getClass());
+ // 3. InvocationException: decoder wrapping exception
+ wrappedException = wrappedException.getCause();
+ TestMgr.check(InvocationException.class, wrappedException.getClass());
+ TestMgr.check("InvocationException: code=490;msg=CommonExceptionData
[message=Cse Internal Bad Request]",
+ wrappedException.getMessage());
+ // 4. InvalidFormatException: decode exception
+ Object cause = wrappedException.getCause();
+ TestMgr.check(InvalidFormatException.class, cause.getClass());
+ TestMgr.check(
+ "Cannot deserialize value of type `java.util.Date` from String
\"returnOK\": not a valid representation "
+ + "(error: Failed to parse Date value 'returnOK': Failed to parse
date \"returnOK\": Invalid number: retu)\n"
+ + " at [Source:
(org.apache.servicecomb.foundation.vertx.stream.BufferInputStream); line: 1,
column: 12] "
+ + "(through reference chain:
org.apache.servicecomb.demo.springmvc.decoderesponse.DecodeTestResponse[\"content\"])",
+ ((InvalidFormatException) cause).getMessage());
+ }
}
diff --git
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/decoderesponse/DecodeTestResponse.java
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/decoderesponse/DecodeTestResponse.java
new file mode 100644
index 000000000..f27c15d00
--- /dev/null
+++
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/decoderesponse/DecodeTestResponse.java
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.servicecomb.demo.springmvc.decoderesponse;
+
+import java.util.Date;
+
+public class DecodeTestResponse {
+ private Date content;
+
+ public Date getContent() {
+ return content;
+ }
+
+ public void setContent(Date content) {
+ this.content = content;
+ }
+
+ @Override
+ public String toString() {
+ final StringBuilder sb = new StringBuilder("DecodeTestResponse{");
+ sb.append("content=").append(content);
+ sb.append('}');
+ return sb.toString();
+ }
+}
diff --git
a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/decoderesponse/DecodeTestResponse.java
b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/decoderesponse/DecodeTestResponse.java
new file mode 100644
index 000000000..9a19dc190
--- /dev/null
+++
b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/decoderesponse/DecodeTestResponse.java
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.servicecomb.demo.springmvc.decoderesponse;
+
+public class DecodeTestResponse {
+ private String content;
+
+ public String getContent() {
+ return content;
+ }
+
+ public void setContent(String content) {
+ this.content = content;
+ }
+
+ @Override
+ public String toString() {
+ final StringBuilder sb = new StringBuilder("DecodeTestResponse{");
+ sb.append("content='").append(content).append('\'');
+ sb.append('}');
+ return sb.toString();
+ }
+}
diff --git
a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/CodeFirstSpringmvc.java
b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/CodeFirstSpringmvc.java
index 01a21587f..953cb158b 100644
---
a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/CodeFirstSpringmvc.java
+++
b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/CodeFirstSpringmvc.java
@@ -42,6 +42,7 @@
import org.apache.servicecomb.demo.ignore.OutputModelForTestIgnore;
import org.apache.servicecomb.demo.jaxbbean.JAXBPerson;
import org.apache.servicecomb.demo.server.User;
+import org.apache.servicecomb.demo.springmvc.decoderesponse.DecodeTestResponse;
import org.apache.servicecomb.provider.rest.common.RestSchema;
import org.apache.servicecomb.swagger.extend.annotations.RawJsonRequestBody;
import org.apache.servicecomb.swagger.extend.annotations.ResponseHeaders;
@@ -512,4 +513,11 @@ public String testAbort() {
LOGGER.info("testAbort() is called!");
return "OK";
}
+
+ @GetMapping(path = "/testDecodeResponseError")
+ public DecodeTestResponse testDecodeResponseError() {
+ DecodeTestResponse response = new DecodeTestResponse();
+ response.setContent("returnOK");
+ return response;
+ }
}
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 bfaac0041..7d540784a 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
@@ -29,12 +29,14 @@
import org.apache.servicecomb.core.definition.OperationMeta;
import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx;
+import org.apache.servicecomb.swagger.invocation.InvocationType;
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,10 +63,10 @@ protected ProduceProcessor
findProduceProcessor(RestOperationMeta restOperation,
return restOperation.findProduceProcessor(contentTypeForFind);
}
- protected Object extractResult(Invocation invocation, HttpServletResponseEx
responseEx) {
+ protected Response extractResponse(Invocation invocation,
HttpServletResponseEx responseEx) {
Object result =
invocation.getHandlerContext().get(RestConst.READ_STREAM_PART);
if (result != null) {
- return result;
+ return Response.create(responseEx.getStatusType(), result);
}
OperationMeta operationMeta = invocation.getOperationMeta();
@@ -79,22 +81,23 @@ protected Object extractResult(Invocation invocation,
HttpServletResponseEx resp
responseEx.getStatus(),
responseEx.getStatusType().getReasonPhrase(),
responseEx.getHeader(HttpHeaders.CONTENT_TYPE));
- return ExceptionFactory.createConsumerException(new
InvocationException(responseEx.getStatus(),
responseEx.getStatusType().getReasonPhrase(), msg));
+ LOGGER.error(msg);
+ return Response.createFail(InvocationType.CONSUMER, msg);
}
try {
- return produceProcessor.decodeResponse(responseEx.getBodyBuffer(),
responseMeta.getJavaType());
+ result = produceProcessor.decodeResponse(responseEx.getBodyBuffer(),
responseMeta.getJavaType());
+ return Response.create(responseEx.getStatusType(), result);
} catch (Exception e) {
- return ExceptionFactory.createConsumerException(e);
+ LOGGER.error("failed to decode response body, exception type is [{}]",
e.getClass());
+ return Response.createConsumerFail(e);
}
}
@Override
public Response afterReceiveResponse(Invocation invocation,
HttpServletResponseEx responseEx) {
- Object result = extractResult(invocation, responseEx);
+ Response response = extractResponse(invocation, responseEx);
- Response response =
- Response.create(responseEx.getStatusType(), result);
for (String headerName : responseEx.getHeaderNames()) {
Collection<String> headerValues = responseEx.getHeaders(headerName);
for (String headerValue : headerValues) {
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 542e82c55..3279ab393 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
@@ -18,6 +18,7 @@
package org.apache.servicecomb.transport.rest.client.http;
import java.util.Arrays;
+import java.util.Date;
import java.util.HashMap;
import java.util.Map;
@@ -25,6 +26,7 @@
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;
@@ -32,15 +34,22 @@
import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx;
import org.apache.servicecomb.foundation.vertx.http.ReadStreamPart;
import org.apache.servicecomb.swagger.invocation.Response;
+import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData;
+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.junit.Assert;
import org.junit.Test;
+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 {
@@ -95,24 +104,75 @@ public void
testFindProduceProcessorJsonWithCharset(@Mocked RestOperationMeta re
}
@Test
- public void extractResult_readStreamPart(@Mocked Invocation invocation,
@Mocked ReadStreamPart part) {
+ public void extractResult_readStreamPart(@Mocked Invocation invocation,
@Mocked ReadStreamPart part,
+ @Mocked HttpServletResponseEx responseEx) {
Map<String, Object> handlerContext = new HashMap<>();
handlerContext.put(RestConst.READ_STREAM_PART, part);
new Expectations() {
{
invocation.getHandlerContext();
result = handlerContext;
+ responseEx.getStatusType();
+ result = Status.OK;
+ }
+ };
+
+ Response response = filter.extractResponse(invocation, responseEx);
+ Assert.assertSame(part, response.getResult());
+ Assert.assertEquals(Status.OK, response.getStatus());
+ }
+
+ @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();
}
};
- Assert.assertSame(part, filter.extractResult(invocation, null));
+ Response response = filter.extractResponse(invocation, responseEx);
+ Assert.assertEquals(490, response.getStatusCode());
+ Assert.assertEquals(ExceptionFactory.CONSUMER_INNER_REASON_PHRASE,
response.getReasonPhrase());
+ Assert.assertEquals(InvocationException.class,
response.<InvocationException>getResult().getClass());
+ InvocationException invocationException = response.getResult();
+ Assert.assertEquals("InvocationException: code=490;msg=CommonExceptionData
[message=Cse Internal Bad Request]",
+ invocationException.getMessage());
+ 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]",
+ invocationException.getCause().getMessage());
+ Assert.assertEquals(CommonExceptionData.class,
invocationException.getErrorData().getClass());
+ CommonExceptionData commonExceptionData = (CommonExceptionData)
invocationException.getErrorData();
+ Assert.assertEquals("Cse Internal Bad Request",
commonExceptionData.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();
@@ -120,18 +180,22 @@ public void
testAfterReceiveResponseNullProduceProcessor(@Mocked Invocation invo
operationMeta.getExtData(RestConst.SWAGGER_REST_OPERATION);
result = swaggerRestOperation;
responseEx.getStatus();
- result = 401;
+ result = 200;
}
};
Response response = filter.afterReceiveResponse(invocation, responseEx);
- InvocationException exception = response.getResult();
+ Assert.assertEquals(490, response.getStatusCode());
+ Assert.assertEquals(ExceptionFactory.CONSUMER_INNER_REASON_PHRASE,
response.getReasonPhrase());
+ Assert.assertEquals(InvocationException.class,
response.<InvocationException>getResult().getClass());
+ InvocationException invocationException = response.getResult();
Assert.assertEquals(
- 401,
- exception.getStatusCode());
+ 490,
+ invocationException.getStatusCode());
Assert.assertEquals(
- "method null, path null, statusCode 401, reasonPhrase null, response
content-type null is not supported",
- exception.getErrorData());
+ "CommonExceptionData [message=method null, path null, statusCode 200,
reasonPhrase null, "
+ + "response content-type null is not supported]",
+ invocationException.getErrorData().toString());
}
@Test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Add response decode error log
> -----------------------------
>
> Key: SCB-827
> URL: https://issues.apache.org/jira/browse/SCB-827
> Project: Apache ServiceComb
> Issue Type: Improvement
> Reporter: YaoHaishi
> Assignee: YaoHaishi
> Priority: Major
>
> In DefaultHttpClientFilter.extractResult(), once response decode error
> occurs, it will be caught and set into an InvocationException, and the
> InvocationException will be treated as response body, which will cause
> confusing result.
> We need to print some logs to help developers locate such problem.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
