SONAR-7776 Catch aborted requests and display warning

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 6d39e6d1fc5066931872dd5bac9e66f72ee81213..a354c03c402c48437cae17cc29adbcc1b239e27b 100644 (file)
--- 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 b72623743d15f008d0f0b91e38286560c3617e92..bcf9dab7e10b2fede8ffeed1c3b2c5e328c030e6 100644 (file)
--- 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();
     }