Merge pull request #659 from DwayneJengSage/develop

BRIDGE-3321 move ClientAbortException ("broken pipe") from error to warning

src/main/java/org/sagebionetworks/bridge/spring/handlers/BridgeExceptionHandler.java

@@ -16,6 +16,7 @@
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.common.collect.ImmutableSet;
 
+import org.apache.catalina.connector.ClientAbortException;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -69,6 +70,13 @@ private void logException(HttpServletRequest request, Throwable throwable) {
             }
         }
 
+        // BRIDGE-3321 - ClientAbortException aka "broken pipe" is not a problem, it's just the client. Log a warning
+        // instead of an error.
+        if (throwable instanceof ClientAbortException) {
+            log.warn(msg, throwable);
+            return;
+        }
+
         log.error(msg, throwable);
     }
 

src/test/java/org/sagebionetworks/bridge/spring/handlers/BridgeExceptionHandlerTest.java

@@ -23,6 +23,7 @@
 import com.google.common.collect.Maps;
 
 import org.aopalliance.intercept.MethodInvocation;
+import org.apache.catalina.connector.ClientAbortException;
 import org.hibernate.QueryParameterException;
 import org.mockito.ArgumentCaptor;
 import org.mockito.InjectMocks;
@@ -219,7 +220,15 @@ public void ddbThrottlingReportedAs500() throws Throwable {
         // Verify log.
         verify(logger).error(contains(ex.getMessage()), same(ex));
     }
-
+
+    @Test
+    public void clientAbortException() throws Exception {
+        // Execute. The output is not important. We just want to verify that we log a warning instead of an error.
+        ClientAbortException ex = new ClientAbortException("dummy exception message");
+        handler.handleException(mockRequest, ex);
+        verify(logger).warn(contains(ex.getMessage()), same(ex));
+    }
+
     // If you do not wrap a RuntimeException in BridgeServiceException, it's still reported as a 500 response, 
     // but the JSON will be based on that object, so e.g. the type will be the type of the exception. That and 
     // usually other details are internal to the system and will not make sense to an API caller.