Fix StatisticsHandler in the case a Handler throws exception.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2022-04-05 19:12:45 +10:00
parent 282aad85ed
commit 39c478461b
2 changed files with 85 additions and 4 deletions

View File

@ -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 class StatisticsHandler extends HandlerWrapper implements Graceful
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 class StatisticsHandler extends HandlerWrapper implements Graceful
_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 class StatisticsHandler extends HandlerWrapper implements Graceful
{
_requestStats.decrement();
_requestTimeStats.record(dispatched);
updateResponse(baseRequest);
updateResponse(baseRequest, thrownError);
}
}
@ -198,10 +205,14 @@ public class StatisticsHandler extends HandlerWrapper implements Graceful
}
}
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 class StatisticsHandler extends HandlerWrapper implements Graceful
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 class StatisticsHandler extends HandlerWrapper implements Graceful
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();

View File

@ -30,14 +30,17 @@ import javax.servlet.http.HttpServletResponse;
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;
@ -420,6 +423,60 @@ public class StatisticsHandlerTest
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
{