Consider the configured unhandled exception content type as a default

... rather than an override.
quarkusio · Sep 21, 2021 · 02b30c8 · 02b30c8
1 parent 77a160d
commit 02b30c8
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 24 deletions.
diff --git a/...ons/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/HttpConfiguration.java b/...ons/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/HttpConfiguration.java
@@ -232,14 +232,18 @@ public class HttpConfiguration {
     public boolean enableDecompression;
 
     /**
-     * Provides a hint (optional) for the contents type of responses generated for
+     * Provides a hint (optional) for the default content type of responses generated for
      * the errors not handled by the application.
      * <p>
-     * When unset, Quarkus will decide which contents type to use based on request headers.
+     * If the client requested a supported content-type in request headers
+     * (e.g. "Accept: application/json", "Accept: text/html"),
+     * Quarkus will use that content type.
+     * <p>
+     * Otherwise, it will default to the content type configured here.
      * </p>
      */
     @ConfigItem
-    public Optional<PayloadHint> unhandledErrorContentsType;
+    public Optional<PayloadHint> unhandledErrorContentTypeDefault;
 
     public ProxyConfig proxy;
 

diff --git a/...s/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/QuarkusErrorHandler.java b/...s/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/QuarkusErrorHandler.java
@@ -40,11 +40,11 @@ public class QuarkusErrorHandler implements Handler<RoutingContext> {
     private static final AtomicLong ERROR_COUNT = new AtomicLong();
 
     private final boolean showStack;
-    private final Optional<HttpConfiguration.PayloadHint> contentTypeHint;
+    private final Optional<HttpConfiguration.PayloadHint> contentTypeDefault;
 
-    public QuarkusErrorHandler(boolean showStack, Optional<HttpConfiguration.PayloadHint> contentTypeHint) {
+    public QuarkusErrorHandler(boolean showStack, Optional<HttpConfiguration.PayloadHint> contentTypeDefault) {
         this.showStack = showStack;
-        this.contentTypeHint = contentTypeHint;
+        this.contentTypeDefault = contentTypeDefault;
     }
 
     @Override
@@ -139,17 +139,6 @@ public void accept(Throwable throwable) {
         if (responseContentType == null) {
             responseContentType = "";
         }
-        if (contentTypeHint.isPresent()) {
-            switch (contentTypeHint.get()) {
-                case JSON:
-                    jsonResponse(event, ContentTypes.APPLICATION_JSON, details, stack);
-                    break;
-                case HTML:
-                    htmlResponse(event, details, exception);
-                    break;
-            }
-            return;
-        }
         switch (responseContentType) {
             case ContentTypes.TEXT_HTML:
             case ContentTypes.APPLICATION_XHTML:
@@ -161,9 +150,16 @@ public void accept(Throwable throwable) {
             case ContentTypes.TEXT_JSON:
                 jsonResponse(event, responseContentType, details, stack);
                 break;
-            // We default to HTML representation
             default:
-                htmlResponse(event, details, exception);
+                // We default to HTML representation
+                switch (contentTypeDefault.orElse(HttpConfiguration.PayloadHint.HTML)) {
+                    case JSON:
+                        jsonResponse(event, ContentTypes.APPLICATION_JSON, details, stack);
+                        break;
+                    case HTML:
+                        htmlResponse(event, details, exception);
+                        break;
+                }
                 break;
         }
     }

diff --git a/...ons/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java b/...ons/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java
@@ -309,7 +309,7 @@ public void finalizeRouter(BeanContainer container, Consumer<Route> defaultRoute
         }
 
         httpRouteRouter.route().last().failureHandler(
-                new QuarkusErrorHandler(launchMode.isDevOrTest(), httpConfiguration.unhandledErrorContentsType));
+                new QuarkusErrorHandler(launchMode.isDevOrTest(), httpConfiguration.unhandledErrorContentTypeDefault));
 
         if (requireBodyHandler) {
             //if this is set then everything needs the body handler installed

diff --git a/...rtx-http/runtime/src/test/java/io/quarkus/vertx/http/runtime/QuarkusErrorHandlerTest.java b/...rtx-http/runtime/src/test/java/io/quarkus/vertx/http/runtime/QuarkusErrorHandlerTest.java
@@ -50,16 +50,27 @@ public void string_with_quotes_should_be_correctly_escaped() {
     }
 
     @Test
-    public void json_content_type_hint_should_be_respected() {
+    public void json_content_type_hint_should_be_respected_if_not_accepted() {
         QuarkusErrorHandler errorHandler = new QuarkusErrorHandler(false, Optional.of(HttpConfiguration.PayloadHint.JSON));
         Mockito.when(routingContext.failure()).thenReturn(testError);
         Mockito.when(routingContext.parsedHeaders().findBestUserAcceptedIn(any(), any()))
-                .thenReturn(new ParsableMIMEValue("text/html").forceParse());
+                .thenReturn(new ParsableMIMEValue("application/foo+json").forceParse());
         errorHandler.handle(routingContext);
         Mockito.verify(routingContext.response().headers()).set(HttpHeaderNames.CONTENT_TYPE,
                 "application/json; charset=utf-8");
     }
 
+    @Test
+    public void json_content_type_hint_should_be_ignored_if_accepted() {
+        QuarkusErrorHandler errorHandler = new QuarkusErrorHandler(false, Optional.of(HttpConfiguration.PayloadHint.JSON));
+        Mockito.when(routingContext.failure()).thenReturn(testError);
+        Mockito.when(routingContext.parsedHeaders().findBestUserAcceptedIn(any(), any()))
+                .thenReturn(new ParsableMIMEValue("text/html").forceParse());
+        errorHandler.handle(routingContext);
+        Mockito.verify(routingContext.response().headers()).set(HttpHeaderNames.CONTENT_TYPE,
+                "text/html; charset=utf-8");
+    }
+
     @Test
     public void content_type_should_default_to_json_if_accepted() {
         QuarkusErrorHandler errorHandler = new QuarkusErrorHandler(false, Optional.empty());
@@ -72,15 +83,26 @@ public void content_type_should_default_to_json_if_accepted() {
     }
 
     @Test
-    public void html_content_type_hint_should_be_respected() {
+    public void html_content_type_hint_should_be_respected_if_not_accepted() {
         QuarkusErrorHandler errorHandler = new QuarkusErrorHandler(false, Optional.of(HttpConfiguration.PayloadHint.HTML));
         Mockito.when(routingContext.failure()).thenReturn(testError);
         Mockito.when(routingContext.parsedHeaders().findBestUserAcceptedIn(any(), any()))
-                .thenReturn(new ParsableMIMEValue("application/json").forceParse());
+                .thenReturn(new ParsableMIMEValue("application/foo+json").forceParse());
         errorHandler.handle(routingContext);
         Mockito.verify(routingContext.response().headers()).set(HttpHeaderNames.CONTENT_TYPE, "text/html; charset=utf-8");
     }
 
+    @Test
+    public void html_content_type_hint_should_be_ignored_if_accepted() {
+        QuarkusErrorHandler errorHandler = new QuarkusErrorHandler(false, Optional.of(HttpConfiguration.PayloadHint.HTML));
+        Mockito.when(routingContext.failure()).thenReturn(testError);
+        Mockito.when(routingContext.parsedHeaders().findBestUserAcceptedIn(any(), any()))
+                .thenReturn(new ParsableMIMEValue("application/json").forceParse());
+        errorHandler.handle(routingContext);
+        Mockito.verify(routingContext.response().headers()).set(HttpHeaderNames.CONTENT_TYPE,
+                "application/json; charset=utf-8");
+    }
+
     @Test
     public void content_type_should_default_to_html_if_accepted() {
         QuarkusErrorHandler errorHandler = new QuarkusErrorHandler(false, Optional.empty());