From caf6e4ec27fdf626bef171fb686323d77ec7581b Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 5 Apr 2022 19:12:45 +1000 Subject: [PATCH] Fix StatisticsHandler in the case a Handler throws exception. Signed-off-by: Lachlan Roberts --- .../server/handler/StatisticsHandler.java | 32 +++++++++-- .../server/handler/StatisticsHandlerTest.java | 57 +++++++++++++++++++ 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java index a57f2ddf1613..39b7d55effbc 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java @@ -60,6 +60,7 @@ public class StatisticsHandler extends HandlerWrapper implements Graceful private final LongAdder _expires = new LongAdder(); private final LongAdder _errors = new LongAdder(); + private final LongAdder _responsesThrown = new LongAdder(); private final LongAdder _responses1xx = new LongAdder(); private final LongAdder _responses2xx = new LongAdder(); private final LongAdder _responses3xx = new LongAdder(); @@ -109,7 +110,7 @@ public void onComplete(AsyncEvent event) long numRequests = _requestStats.decrement(); _requestTimeStats.record(elapsed); - updateResponse(request); + updateResponse(request, false); _asyncWaitStats.decrement(); @@ -174,10 +175,16 @@ public void handle(String path, Request baseRequest, HttpServletRequest request, _asyncDispatches.increment(); } + boolean thrownError = false; try { handler.handle(path, baseRequest, request, response); } + catch (Throwable t) + { + thrownError = true; + throw t; + } finally { final long now = System.currentTimeMillis(); @@ -198,7 +205,7 @@ public void handle(String path, Request baseRequest, HttpServletRequest request, { numRequests = _requestStats.decrement(); _requestTimeStats.record(dispatched); - updateResponse(baseRequest); + updateResponse(baseRequest, thrownError); } } @@ -212,10 +219,14 @@ public void handle(String path, Request baseRequest, HttpServletRequest request, } } - protected void updateResponse(Request request) + protected void updateResponse(Request request, boolean thrownError) { Response response = request.getResponse(); - if (request.isHandled()) + if (thrownError) + { + _responsesThrown.increment(); + } + else if (request.isHandled()) { switch (response.getStatus() / 100) { @@ -548,6 +559,18 @@ public int getResponses5xx() return _responses5xx.intValue(); } + /** + * @return the number of requests that threw an exception during handling + * since {@link #statsReset()} was last called. These may have resulted in + * some error responses which were unrecorded by the {@link StatisticsHandler}. + */ + @ManagedAttribute("number of requests that threw an exception during handling") + public int getResponsesThrown() + { + return _responsesThrown.intValue(); + } + + /** * @return the milliseconds since the statistics were started with {@link #statsReset()}. */ @@ -601,6 +624,7 @@ public String toStatsHTML() sb.append("3xx responses: ").append(getResponses3xx()).append("
\n"); sb.append("4xx responses: ").append(getResponses4xx()).append("
\n"); sb.append("5xx responses: ").append(getResponses5xx()).append("
\n"); + sb.append("responses thrown: ").append(getResponsesThrown()).append("
\n"); sb.append("Bytes sent total: ").append(getResponsesBytesTotal()).append("
\n"); return sb.toString(); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java index f4a7d811fed3..32578523bb19 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java @@ -35,14 +35,17 @@ import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.io.ConnectionStatistics; +import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.util.log.StacklessLogging; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -425,6 +428,60 @@ public void handle(String path, Request request, HttpServletRequest httpRequest, barrier[3].await(); } + @Test + public void testThrownResponse() throws Exception + { + _statsHandler.setHandler(new AbstractHandler() + { + @Override + public void handle(String path, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws IOException + { + try + { + throw new IllegalStateException("expected"); + } + catch (IllegalStateException e) + { + throw e; + } + catch (Exception e) + { + throw new IOException(e); + } + } + }); + _server.start(); + + try (StacklessLogging ignored = new StacklessLogging(HttpChannel.class)) + { + String request = "GET / HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "\r\n"; + String response = _connector.getResponse(request); + assertThat(response, containsString("HTTP/1.1 500 Server Error")); + } + + assertEquals(1, _statsHandler.getRequests()); + assertEquals(0, _statsHandler.getRequestsActive()); + assertEquals(1, _statsHandler.getRequestsActiveMax()); + + assertEquals(1, _statsHandler.getDispatched()); + assertEquals(0, _statsHandler.getDispatchedActive()); + assertEquals(1, _statsHandler.getDispatchedActiveMax()); + + assertEquals(0, _statsHandler.getAsyncRequests()); + assertEquals(0, _statsHandler.getAsyncDispatches()); + assertEquals(0, _statsHandler.getExpires()); + + // We get no recorded status, but we get a recorded thrown response. + assertEquals(0, _statsHandler.getResponses1xx()); + assertEquals(0, _statsHandler.getResponses2xx()); + assertEquals(0, _statsHandler.getResponses3xx()); + assertEquals(0, _statsHandler.getResponses4xx()); + assertEquals(0, _statsHandler.getResponses5xx()); + assertEquals(1, _statsHandler.getResponsesThrown()); + } + @Test public void waitForSuspendedRequestTest() throws Exception {