From fd1f6574600ce33f556fc66dbff3ba8ffc127d21 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 15 Jul 2016 09:30:03 +0200 Subject: [PATCH] SONAR-7776 Catch aborted requests and display warning --- .../org/sonar/server/ws/WebServiceEngine.java | 30 ++++++++++++------- .../sonar/server/ws/WebServiceEngineTest.java | 30 ++++++++++++++++--- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java b/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java index 6d39e6d1fc5..a354c03c402 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java @@ -19,21 +19,13 @@ */ package org.sonar.server.ws; -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Strings.isNullOrEmpty; -import static java.lang.String.format; -import static org.apache.commons.lang.StringUtils.substring; -import static org.apache.commons.lang.StringUtils.substringAfterLast; -import static org.apache.commons.lang.StringUtils.substringBeforeLast; -import static org.sonar.server.ws.RequestVerifier.verifyRequest; -import static org.sonar.server.ws.ServletRequest.SUPPORTED_MEDIA_TYPES_BY_URL_SUFFIX; - import java.io.OutputStreamWriter; import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Locale; import javax.annotation.CheckForNull; import javax.annotation.Nullable; +import org.apache.catalina.connector.ClientAbortException; import org.picocontainer.Startable; import org.sonar.api.i18n.I18n; import org.sonar.api.server.ServerSide; @@ -42,6 +34,7 @@ import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.api.server.ws.internal.ValidatingRequest; +import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.api.utils.text.JsonWriter; import org.sonar.server.exceptions.BadRequestException; @@ -51,12 +44,23 @@ import org.sonar.server.exceptions.ServerException; import org.sonar.server.user.UserSession; import org.sonarqube.ws.MediaTypes; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Strings.isNullOrEmpty; +import static java.lang.String.format; +import static org.apache.commons.lang.StringUtils.substring; +import static org.apache.commons.lang.StringUtils.substringAfterLast; +import static org.apache.commons.lang.StringUtils.substringBeforeLast; +import static org.sonar.server.ws.RequestVerifier.verifyRequest; +import static org.sonar.server.ws.ServletRequest.SUPPORTED_MEDIA_TYPES_BY_URL_SUFFIX; + /** * @since 4.2 */ @ServerSide public class WebServiceEngine implements LocalConnector, Startable { + private static final Logger LOGGER = Loggers.get(WebServiceEngine.class); + private final WebService.Context context; private final I18n i18n; private final UserSession userSession; @@ -110,7 +114,13 @@ public class WebServiceEngine implements LocalConnector, Startable { } catch (ServerException e) { sendErrors(response, e.httpCode(), new Errors().add(Message.of(e.getMessage()))); } catch (Exception e) { - Loggers.get(getClass()).error("Fail to process request " + request, e); + Throwable cause = e.getCause(); + if (cause != null && cause instanceof ClientAbortException) { + // Request has been aborted by the client, nothing can been done as Tomcat has committed the response + LOGGER.warn("Request {} has been aborted by client, error is '{}'", request, e.getMessage()); + return; + } + LOGGER.error("Fail to process request " + request, e); sendErrors(response, 500, new Errors().add(Message.of(e.getMessage()))); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java index b72623743d1..bcf9dab7e10 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java @@ -19,12 +19,9 @@ */ package org.sonar.server.ws; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - import java.io.IOException; import java.util.Locale; +import org.apache.catalina.connector.ClientAbortException; import org.apache.commons.io.IOUtils; import org.junit.After; import org.junit.Before; @@ -36,14 +33,23 @@ import org.sonar.api.server.ws.RequestHandler; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.api.server.ws.internal.ValidatingRequest; +import org.sonar.api.utils.log.LogTester; +import org.sonar.api.utils.log.LoggerLevel; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.Errors; import org.sonar.server.exceptions.Message; import org.sonar.server.tester.UserSessionRule; import org.sonarqube.ws.MediaTypes; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + public class WebServiceEngineTest { + @Rule + public LogTester logTester = new LogTester(); + @Rule public UserSessionRule userSessionRule = UserSessionRule.standalone(); @@ -282,6 +288,16 @@ public class WebServiceEngineTest { assertThat(response.getHeader(name)).isEqualTo(value); } + @Test + public void does_not_fail_when_request_is_aborted() throws Exception { + ValidatingRequest request = new TestRequest().setMethod("GET").setPath("/api/system/fail_with_client_abort_exception"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); + + assertThat(response.stream().outputAsString()).isEmpty(); + assertThat(logTester.logs(LoggerLevel.WARN)).isNotEmpty(); + } + static class SystemWs implements WebService { @Override public void define(Context context) { @@ -380,6 +396,12 @@ public class WebServiceEngineTest { } } }); + + createNewDefaultAction(newController, "fail_with_client_abort_exception") + .setHandler((request, response) -> { + throw new IllegalStateException("fail!", new ClientAbortException()); + }); + newController.done(); } -- 2.39.5