From 9c553639dd19035c4936c4f9ec89be16dac78d6f Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Fri, 27 Aug 2021 11:55:08 +1000 Subject: [PATCH] Fix "Response head already sent" Fixes #19621 --- .../http/runtime/QuarkusErrorHandler.java | 82 +++++++++++-------- .../web/mutiny/NdjsonMultiRouteTest.java | 2 +- .../vertx/web/mutiny/SSEMultiRouteTest.java | 2 +- 3 files changed, 52 insertions(+), 34 deletions(-) diff --git a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/QuarkusErrorHandler.java b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/QuarkusErrorHandler.java index 04fae63b0f396..6484de2a3f34d 100644 --- a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/QuarkusErrorHandler.java +++ b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/QuarkusErrorHandler.java @@ -41,40 +41,49 @@ public QuarkusErrorHandler(boolean showStack) { @Override public void handle(RoutingContext event) { - if (event.failure() == null) { - event.response().setStatusCode(event.statusCode()); - event.response().end(); - return; - } - //this can happen if there is no auth mechanisms - if (event.failure() instanceof UnauthorizedException) { - HttpAuthenticator authenticator = event.get(HttpAuthenticator.class.getName()); - if (authenticator != null) { - authenticator.sendChallenge(event).subscribe().with(new Consumer() { - @Override - public void accept(Boolean aBoolean) { - event.response().end(); - } - }, new Consumer() { - @Override - public void accept(Throwable throwable) { - event.fail(throwable); - } - }); - } else { + try { + if (event.failure() == null) { + event.response().setStatusCode(event.statusCode()); + event.response().end(); + return; + } + //this can happen if there is no auth mechanisms + if (event.failure() instanceof UnauthorizedException) { + HttpAuthenticator authenticator = event.get(HttpAuthenticator.class.getName()); + if (authenticator != null) { + authenticator.sendChallenge(event).subscribe().with(new Consumer() { + @Override + public void accept(Boolean aBoolean) { + event.response().end(); + } + }, new Consumer() { + @Override + public void accept(Throwable throwable) { + event.fail(throwable); + } + }); + } else { + event.response().setStatusCode(HttpResponseStatus.UNAUTHORIZED.code()).end(); + } + return; + } + if (event.failure() instanceof ForbiddenException) { + event.response().setStatusCode(HttpResponseStatus.FORBIDDEN.code()).end(); + return; + } + if (event.failure() instanceof AuthenticationFailedException) { + //generally this should be handled elsewhere + //but if we get to this point bad things have happened + //so it is better to send a response than to hang event.response().setStatusCode(HttpResponseStatus.UNAUTHORIZED.code()).end(); + return; + } + } catch (IllegalStateException e) { + //ignore this if the response is already started + if (!event.response().ended()) { + //could be that just the head is committed + event.response().end(); } - return; - } - if (event.failure() instanceof ForbiddenException) { - event.response().setStatusCode(HttpResponseStatus.FORBIDDEN.code()).end(); - return; - } - if (event.failure() instanceof AuthenticationFailedException) { - //generally this should be handled elsewhere - //but if we get to this point bad things have happened - //so it is better to send a response than to hang - event.response().setStatusCode(HttpResponseStatus.UNAUTHORIZED.code()).end(); return; } @@ -100,6 +109,15 @@ public void accept(Throwable throwable) { } else { log.errorf(exception, "HTTP Request to %s failed, error id: %s", event.request().uri(), uuid); } + //we have logged the error + //now lets see if we can actually send a response + //if not we just return + if (event.response().ended()) { + return; + } else if (event.response().headWritten()) { + event.response().end(); + return; + } String accept = event.request().getHeader("Accept"); if (accept != null && accept.contains("application/json")) { event.response().headers().set(HttpHeaderNames.CONTENT_TYPE, "application/json; charset=utf-8"); diff --git a/extensions/vertx-web/deployment/src/test/java/io/quarkus/vertx/web/mutiny/NdjsonMultiRouteTest.java b/extensions/vertx-web/deployment/src/test/java/io/quarkus/vertx/web/mutiny/NdjsonMultiRouteTest.java index 97e31d960907c..8c2d652896705 100644 --- a/extensions/vertx-web/deployment/src/test/java/io/quarkus/vertx/web/mutiny/NdjsonMultiRouteTest.java +++ b/extensions/vertx-web/deployment/src/test/java/io/quarkus/vertx/web/mutiny/NdjsonMultiRouteTest.java @@ -47,7 +47,7 @@ public void testNdjsonMultiRoute() { // We get the item followed by the exception when().get("/hello-and-fail").then().statusCode(200) .body(containsString("\"Hello\"")) - .body(containsString("boom")); + .body(not(containsString("boom"))); when().get("/void").then().statusCode(204).body(hasLength(0)); diff --git a/extensions/vertx-web/deployment/src/test/java/io/quarkus/vertx/web/mutiny/SSEMultiRouteTest.java b/extensions/vertx-web/deployment/src/test/java/io/quarkus/vertx/web/mutiny/SSEMultiRouteTest.java index 6c998dbf1a106..3dd1656631f1e 100644 --- a/extensions/vertx-web/deployment/src/test/java/io/quarkus/vertx/web/mutiny/SSEMultiRouteTest.java +++ b/extensions/vertx-web/deployment/src/test/java/io/quarkus/vertx/web/mutiny/SSEMultiRouteTest.java @@ -44,7 +44,7 @@ public void testSSEMultiRoute() { // We get the item followed by the exception when().get("/hello-and-fail").then().statusCode(200) .body(containsString("id: 0")) - .body(containsString("boom")); + .body(not(containsString("boom"))); when().get("/buffer").then().statusCode(200) .body(is("data: Buffer\nid: 0\n\n"))