[SCB-905] GlobalRestFailureHandler handle InvocationException properly

apache · Sep 17, 2018 · 4dbc83a · 4dbc83a
1 parent 3cd2dc2
commit 4dbc83a
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 2 deletions.
diff --git a/...t-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestRestServerConfigEdge.java b/...t-consumer/src/main/java/org/apache/servicecomb/it/testcase/TestRestServerConfigEdge.java
@@ -18,14 +18,18 @@
 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;
 import java.net.URL;
 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/...t-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java b/...t-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;