Merge pull request #7953 from eclipse/jetty-10.0.x-statsHandlerThrows

Issue #7837 - Fix StatisticsHandler in the case a Handler throws exception.

jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java

@@ -55,6 +55,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();
@@ -91,7 +92,7 @@ public void onComplete(AsyncEvent event)
             long elapsed = System.currentTimeMillis() - request.getTimeStamp();
             _requestStats.decrement();
             _requestTimeStats.record(elapsed);
-            updateResponse(request);
+            updateResponse(request, false);
             _asyncWaitStats.decrement();
 
             if (_shutdown.isShutdown())
@@ -166,10 +167,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();
@@ -189,7 +196,7 @@ public void handle(String path, Request baseRequest, HttpServletRequest request,
                 {
                     _requestStats.decrement();
                     _requestTimeStats.record(dispatched);
-                    updateResponse(baseRequest);
+                    updateResponse(baseRequest, thrownError);
                 }
             }
 
@@ -198,10 +205,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)
             {
@@ -537,6 +548,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()}.
      */
@@ -590,6 +613,7 @@ public String toStatsHTML()
         sb.append("3xx responses: ").append(getResponses3xx()).append("<br />\n");
         sb.append("4xx responses: ").append(getResponses4xx()).append("<br />\n");
         sb.append("5xx responses: ").append(getResponses5xx()).append("<br />\n");
+        sb.append("responses thrown: ").append(getResponsesThrown()).append("<br />\n");
         sb.append("Bytes sent total: ").append(getResponsesBytesTotal()).append("<br />\n");
 
         return sb.toString();

jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java

@@ -30,6 +30,8 @@
 
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.io.ConnectionStatistics;
+import org.eclipse.jetty.logging.StacklessLogging;
+import org.eclipse.jetty.server.HttpChannel;
 import org.eclipse.jetty.server.LocalConnector;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Server;
@@ -38,6 +40,7 @@
 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;
@@ -420,6 +423,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
     {