#504 DatafeedLoop now retries on 500 server errors

symphony-bdk-core/src/test/java/com/symphony/bdk/core/retry/resilience4j/Resilience4jRetryWithRecoveryTest.java

@@ -93,7 +93,7 @@ void testSupplierWithExceptionAndNoRetryShouldFailWithException() throws Throwab
         ofMinimalInterval(), supplier,
         (t) -> false, Collections.emptyList());
 
-    assertThrows(ApiException.class, () -> r.execute());
+    assertThrows(ApiException.class, r::execute);
     verify(supplier, times(1)).get();
   }
 
@@ -108,7 +108,7 @@ void testMaxAttemptsReachedShouldFailWithException() throws ApiException {
         retryConfig, supplier, (t) -> true,
         Collections.emptyList());
 
-    assertThrows(ApiException.class, () -> r.execute());
+    assertThrows(ApiException.class, r::execute);
     verify(supplier, times(retryConfig.getMaxAttempts())).get();
   }
 
@@ -122,7 +122,7 @@ void testExceptionNotMatchingRetryPredicateShouldBeForwarded() throws ApiExcepti
         (t) -> t instanceof ApiException && ((ApiException) t).isServerError(),
         Collections.emptyList());
 
-    assertThrows(ApiException.class, () -> r.execute());
+    assertThrows(ApiException.class, r::execute);
     verify(supplier, times(1)).get();
   }
 
@@ -166,13 +166,13 @@ void testNonMatchingExceptionShouldNotTriggerRecoveryAndRetry() throws Throwable
     final String value = "string";
 
     SupplierWithApiException<String> supplier = mock(ConcreteSupplier.class);
-    when(supplier.get()).thenThrow(new ApiException(500, "error")).thenReturn(value);
+    when(supplier.get()).thenThrow(new ApiException(400, "error")).thenReturn(value);
 
     ConcreteConsumer consumer = mock(ConcreteConsumer.class);
 
     Resilience4jRetryWithRecovery<String> r = new Resilience4jRetryWithRecovery<>("name", "localhost.symphony.com",
         ofMinimalInterval(), supplier, (t) -> true,
-        Collections.singletonList(new RecoveryStrategy(ApiException.class, e -> e.isClientError(), consumer)));
+        Collections.singletonList(new RecoveryStrategy(ApiException.class, ApiException::isServerError, consumer)));
 
     assertEquals(value, r.execute());
     verify(supplier, times(2)).get();
@@ -195,7 +195,7 @@ void testThrowableInRecoveryAndNotMatchingRetryPredicateShouldBeForwarded() thro
         (t) -> t instanceof ApiException,
         Collections.singletonList(new RecoveryStrategy(ApiException.class, ApiException::isClientError, consumer)));
 
-    assertThrows(IndexOutOfBoundsException.class, () -> r.execute());
+    assertThrows(IndexOutOfBoundsException.class, r::execute);
 
     InOrder inOrder = inOrder(supplier, consumer);
     inOrder.verify(supplier).get();
@@ -241,7 +241,7 @@ void testExecuteAndRetrySucceeds() throws Throwable {
   @Test
   void testExecuteAndRetryShouldConvertApiExceptionIntoApiRuntimeException() throws Throwable {
     SupplierWithApiException<String> supplier = mock(ConcreteSupplier.class);
-    when(supplier.get()).thenThrow(new ApiException(500, ""));
+    when(supplier.get()).thenThrow(new ApiException(400, ""));
 
     assertThrows(ApiRuntimeException.class,
         () -> Resilience4jRetryWithRecovery.executeAndRetry(new RetryWithRecoveryBuilder<String>(), "test", "serviceName", supplier));

symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedLoopV1Test.java

@@ -259,7 +259,7 @@ void startTestRecreateDatafeedError() throws ApiException {
     when(datafeedApi.v4DatafeedIdReadGet("persisted-id", "1234", "1234", null))
         .thenThrow(new ApiException(400, "expired DF id"));
     when(datafeedApi.v4DatafeedCreatePost("1234", "1234"))
-        .thenThrow(new ApiException(500, "unhandled exception"));
+        .thenThrow(new ApiException(404, "unhandled exception"));
 
     assertThrows(NestedRetryException.class, () -> this.datafeedService.start());
 

symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedLoopV2Test.java

@@ -531,7 +531,7 @@ void testStartInternalServerErrorReadDatafeedShouldNotBeRetried() throws ApiExce
     AckId ackId = datafeedService.getAckId();
     when(datafeedApi.listDatafeed("1234", "1234", "tibot")).thenReturn(
         Collections.singletonList(new V5Datafeed().id("test-id")));
-    when(datafeedApi.readDatafeed("test-id", "1234", "1234", ackId)).thenThrow(new ApiException(500, "client-error"));
+    when(datafeedApi.readDatafeed("test-id", "1234", "1234", ackId)).thenThrow(new ApiException(404, "client-error"));
 
     assertThrows(ApiException.class, this.datafeedService::start);
     verify(datafeedApi, times(1)).listDatafeed("1234", "1234", "tibot");

symphony-bdk-http/symphony-bdk-http-api/src/main/java/com/symphony/bdk/http/api/ApiException.java

@@ -81,7 +81,7 @@ public boolean isClientError() {
      * @return true if response status strictly greater than 500, false otherwise
      */
     public boolean isServerError() {
-        return this.code > HttpURLConnection.HTTP_INTERNAL_ERROR;
+        return this.code >= HttpURLConnection.HTTP_INTERNAL_ERROR;
     }
 
     /**

symphony-bdk-http/symphony-bdk-http-api/src/test/java/com/symphony/bdk/http/api/ApiExceptionTest.java

@@ -0,0 +1,32 @@
+package com.symphony.bdk.http.api;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.junit.jupiter.api.Test;
+
+class ApiExceptionTest {
+
+  @Test
+  void isServerError() {
+    assertFalse(new ApiException(499, "An error").isServerError());
+    assertTrue(new ApiException(500, "Internal Server Error").isServerError());
+    assertTrue(new ApiException(502, "Bad Gateway").isServerError());
+    assertTrue(new ApiException(503, "Service Unavailable").isServerError());
+  }
+
+  @Test
+  void isUnauthorized() {
+    assertTrue(new ApiException(401, "Unauthorized").isUnauthorized());
+  }
+
+  @Test
+  void isClientError() {
+    assertTrue(new ApiException(400, "Bad Request").isClientError());
+  }
+
+  @Test
+  void isTooManyRequestsError() {
+    assertTrue(new ApiException(429, "Too Many Requests").isTooManyRequestsError());
+  }
+}