Skip to content

Commit

Permalink
[FSSDK-9103] feat(ATS): fix retries for ODP segment and event API cal…
Browse files Browse the repository at this point in the history 
…ls. (#454)

Fix retries for ODP api access

- no retry on fetchQualifiedSegments failure
- max 3 retries on ODP event dispatch
  • Loading branch information
@jaeopt
jaeopt authored Apr 27, 2023
1 parent 59ad2e6 commit 8aeedd3
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 20 deletions.
9 changes: 4 additions & 5 deletions odp/src/androidTest/java/com/optimizely/ab/android/odp/ODPEventClientTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
import org.mockito.Matchers.any
import org.mockito.Matchers.anyInt
import org.mockito.Matchers.contains
import org.mockito.Matchers.eq
import org.mockito.Mockito.`when`
Expand Down Expand Up @@ -76,7 +75,7 @@ class ODPEventClientTest {

eventClient.dispatch(apiKey, apiEndpoint, payload)

verify(client).execute(captor.capture(), anyInt(), anyInt())
verify(client).execute(captor.capture(), eq(2), eq(3))
val received = captor.value.execute() as Boolean

assertFalse(received)
Expand All @@ -91,7 +90,7 @@ class ODPEventClientTest {

eventClient.dispatch(apiKey, apiEndpoint, payload)

verify(client).execute(captor.capture(), anyInt(), anyInt())
verify(client).execute(captor.capture(), eq(2), eq(3))
val received = captor.value.execute() as Boolean

assertFalse(received)
Expand All @@ -107,10 +106,10 @@ class ODPEventClientTest {
apiEndpoint = "invalid-url"
eventClient.dispatch(apiKey, apiEndpoint, payload)

verify(client).execute(captor.capture(), anyInt(), anyInt())
verify(client).execute(captor.capture(), eq(2), eq(3))
val received = captor.value.execute() as Boolean

assertFalse(received)
verify(logger).error(contains("Error making request"), any())
verify(logger).error(contains("Error making ODP event request"), any())
}
}
15 changes: 7 additions & 8 deletions odp/src/androidTest/java/com/optimizely/ab/android/odp/ODPSegmentClientTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
import org.mockito.Matchers.any
import org.mockito.Matchers.anyInt
import org.mockito.Matchers.contains
import org.mockito.Matchers.eq
import org.mockito.Mockito.`when`
Expand Down Expand Up @@ -59,7 +58,7 @@ class ODPSegmentClientTest {

segmentClient.fetchQualifiedSegments(apiKey, apiEndpoint, payload)

verify(client).execute(captor.capture(), eq(2), eq(2))
verify(client).execute(captor.capture(), eq(0), eq(0))
val received = captor.value.execute()

assert(received == response)
Expand All @@ -75,11 +74,11 @@ class ODPSegmentClientTest {

segmentClient.fetchQualifiedSegments(apiKey, apiEndpoint, payload)

verify(client).execute(captor.capture(), anyInt(), anyInt())
verify(client).execute(captor.capture(), eq(0), eq(0))
val received = captor.value.execute()

assertNull(received)
verify(logger).error("Unexpected response from event endpoint, status: 400")
verify(logger).error("Unexpected response from ODP segment endpoint, status: 400")
verify(urlConnection).disconnect()
}

Expand All @@ -89,11 +88,11 @@ class ODPSegmentClientTest {

segmentClient.fetchQualifiedSegments(apiKey, apiEndpoint, payload)

verify(client).execute(captor.capture(), anyInt(), anyInt())
verify(client).execute(captor.capture(), eq(0), eq(0))
val received = captor.value.execute()

assertNull(received)
verify(logger).error("Unexpected response from event endpoint, status: 500")
verify(logger).error("Unexpected response from ODP segment endpoint, status: 500")
verify(urlConnection).disconnect()
}

Expand All @@ -104,10 +103,10 @@ class ODPSegmentClientTest {
apiEndpoint = "invalid-url"
segmentClient.fetchQualifiedSegments(apiKey, apiEndpoint, payload)

verify(client).execute(captor.capture(), anyInt(), anyInt())
verify(client).execute(captor.capture(), eq(0), eq(0))
val received = captor.value.execute()

assertNull(received)
verify(logger).error(contains("Error making request"), any())
verify(logger).error(contains("Error making ODP segment request"), any())
}
}
6 changes: 5 additions & 1 deletion odp/src/main/java/com/optimizely/ab/android/odp/ODPEventClient.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ open class ODPEventClient(private val client: Client, private val logger: Logger
return@Request false
}
} catch (e: Exception) {
logger.error("Error making request", e)
logger.error("Error making ODP event request", e)
return@Request false
} finally {
if (urlConnection != null) {
Expand All @@ -90,6 +90,10 @@ open class ODPEventClient(private val client: Client, private val logger: Logger
var CONNECTION_TIMEOUT = 10 * 1000
var READ_TIMEOUT = 60 * 1000

// OdpEventManager (java-sdk core) is supposed to handle retries on failures.
// android-sdk returns success immediately for sendOdpEvent() from OdpEventManager and schedules it via WorkManager.
// so retries on failure are supported here in OdpEventClient for android-sdk.

// the numerical base for the exponential backoff
const val REQUEST_BACKOFF_TIMEOUT = 2
// power the number of retries
Expand Down
15 changes: 9 additions & 6 deletions odp/src/main/java/com/optimizely/ab/android/odp/ODPSegmentClient.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ open class ODPSegmentClient(private val client: Client, private val logger: Logg
val status = urlConnection.responseCode
if (status in 200..399) {
val json = client.readStream(urlConnection)
logger.debug("Successfully fetched segments: {}", json)
logger.debug("Successfully fetched ODP segments: {}", json)
return@Request json
} else {
logger.error("Unexpected response from event endpoint, status: $status")
logger.error("Unexpected response from ODP segment endpoint, status: $status")
return@Request null
}
} catch (e: Exception) {
logger.error("Error making request", e)
logger.error("Error making ODP segment request", e)
return@Request null
} finally {
if (urlConnection != null) {
Expand Down Expand Up @@ -92,9 +92,12 @@ open class ODPSegmentClient(private val client: Client, private val logger: Logg
var CONNECTION_TIMEOUT = 10 * 1000
var READ_TIMEOUT = 60 * 1000

// No retries on fetchQualifiedSegments() errors.
// We want to return failure immediately to callers.

// the numerical base for the exponential backoff
const val REQUEST_BACKOFF_TIMEOUT = 2
// power the number of retries (2 = retry once)
const val REQUEST_RETRIES_POWER = 2
const val REQUEST_BACKOFF_TIMEOUT = 0
// power the number of retries
const val REQUEST_RETRIES_POWER = 0
}
}
9 changes: 9 additions & 0 deletions shared/src/androidTest/java/com/optimizely/ab/android/shared/ClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,13 @@ public void testExpBackoffFailure() {
assertTrue(timeouts.contains(8));
assertTrue(timeouts.contains(16));
}

@Test
public void testExpBackoffFailure_noRetriesWhenBackoffSetToZero() {
Client.Request request = mock(Client.Request.class);
when(request.execute()).thenReturn(null);
assertNull(client.execute(request, 0, 0));
verify(logger, never()).info(eq("Request failed, waiting {} seconds to try again"), any(Integer.class));
}

}
3 changes: 3 additions & 0 deletions shared/src/main/java/com/optimizely/ab/android/shared/Client.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ public <T> T execute(Request<T> request, int timeout, int power) {
}

if (response == null || response == Boolean.FALSE) {
// retry is disabled when timeout set to 0
if (timeout == 0) break;

try {
logger.info("Request failed, waiting {} seconds to try again", timeout);
Thread.sleep(TimeUnit.MILLISECONDS.convert(timeout, TimeUnit.SECONDS));
Expand Down

0 comments on commit 8aeedd3

Please sign in to comment.