[
https://issues.apache.org/jira/browse/SCB-905?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16617161#comment-16617161
]
ASF GitHub Bot commented on SCB-905:
------------------------------------
liubao68 closed pull request #909: [SCB-905] GlobalRestFailureHandler handle
InvocationException properly
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/909
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/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestRestServerConfigEdge.java
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestRestServerConfigEdge.java
index 29800040e..dbdf50871 100644
---
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestRestServerConfigEdge.java
+++
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestRestServerConfigEdge.java
@@ -18,6 +18,7 @@
package org.apache.servicecomb.it.testcase;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
import java.io.IOException;
import java.net.HttpURLConnection;
@@ -25,7 +26,10 @@
import java.util.Scanner;
import org.apache.servicecomb.it.extend.engine.GateRestTemplate;
+import org.junit.Assert;
import org.junit.Test;
+import org.springframework.web.client.HttpClientErrorException;
+import org.springframework.web.client.RestClientException;
public class TestRestServerConfigEdge {
static GateRestTemplate rt = (GateRestTemplate)
GateRestTemplate.createEdgeRestTemplate("dataTypeJaxrs");
@@ -53,4 +57,17 @@ public void testIllegalPathParam() throws IOException {
assertEquals("Internal Server Error", responseMessage);
assertEquals("{\"message\":\"unknown error\"}", errorBody);
}
+
+ @Test
+ public void test404ThrownByServicCombNotConvertedTo500() {
+ String notFoundRequestUri = rt.getUrlPrefix() + "/intPath2/123";
+
+ try {
+ rt.getForEntity(notFoundRequestUri, int.class);
+ fail("an exception is expected!");
+ } catch (RestClientException e) {
+ Assert.assertEquals(404, ((HttpClientErrorException)
e).getRawStatusCode());
+ Assert.assertEquals("CommonExceptionData [message=Not Found]",
((HttpClientErrorException) e).getStatusText());
+ }
+ }
}
diff --git
a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java
b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java
index c3944e64d..caeaa1bb3 100644
---
a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java
+++
b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java
@@ -39,6 +39,7 @@
import org.apache.servicecomb.foundation.vertx.TransportType;
import org.apache.servicecomb.foundation.vertx.VertxTLSBuilder;
import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
import
org.apache.servicecomb.transport.rest.vertx.accesslog.AccessLogConfiguration;
import
org.apache.servicecomb.transport.rest.vertx.accesslog.impl.AccessLogHandler;
import org.slf4j.Logger;
@@ -55,6 +56,7 @@
import io.vertx.core.http.HttpMethod;
import io.vertx.core.http.HttpServer;
import io.vertx.core.http.HttpServerOptions;
+import io.vertx.core.http.HttpServerResponse;
import io.vertx.ext.web.Router;
import io.vertx.ext.web.RoutingContext;
import io.vertx.ext.web.handler.CorsHandler;
@@ -130,13 +132,29 @@ private void mountGlobalRestFailureHandler(Router
mainRouter) {
SPIServiceUtils.getPriorityHighestService(GlobalRestFailureHandler.class);
Handler<RoutingContext> failureHandler = null == globalRestFailureHandler ?
ctx -> {
+ if (ctx.response().closed()) {
+ // response has been sent, do nothing
+ LOGGER.error("get a failure with closed response", ctx.failure());
+ ctx.next();
+ }
+ HttpServerResponse response = ctx.response();
+ if (ctx.failure() instanceof InvocationException) {
+ // ServiceComb defined exception
+ InvocationException exception = (InvocationException)
ctx.failure();
+ response.setStatusCode(exception.getStatusCode());
+ response.setStatusMessage(exception.getErrorData().toString());
+ response.end();
+ return;
+ }
+
LOGGER.error("unexpected failure happened", ctx.failure());
- CommonExceptionData unknownError = new CommonExceptionData("unknown
error");
try {
+ // unknown exception
+ CommonExceptionData unknownError = new
CommonExceptionData("unknown error");
ctx.response().setStatusCode(500).putHeader(HttpHeaders.CONTENT_TYPE,
MediaType.APPLICATION_JSON)
.end(RestObjectMapperFactory.getRestObjectMapper().writeValueAsString(unknownError));
} catch (Exception e) {
- LOGGER.error("failed to send error response!");
+ LOGGER.error("failed to send error response!", e);
}
}
: globalRestFailureHandler;
----------------------------------------------------------------
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]
> Request connection is hang up when request path contains illegal string
> -----------------------------------------------------------------------
>
> Key: SCB-905
> URL: https://issues.apache.org/jira/browse/SCB-905
> Project: Apache ServiceComb
> Issue Type: Bug
> Reporter: YaoHaishi
> Assignee: YaoHaishi
> Priority: Major
>
> When provider receives a request that contains illegal path params like
> "%%E", an Exception
> java.lang.NumberFormatException: For input string: "%E"
> is thrown and no response is sent. On consumer side it seems like that the
> request connection is hang up until the request timed out.
> The root cause is that there is no failure handler to handle the
> NumberFormatException. As a result, the request process is interrupted and no
> response is returned.
> Usually it only happens in EdgeService because for
> Router.routeWithRegex(String) the % encoded string is decoded like above,
> while for Router.route() the path is not processed in such way. In
> EdgeService, routeWithRegex() is usually used, and in normal provide, we use
> route() instead.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
