Merge pull request #1327 from embrace-io/fix-session-retry-logic

Retry session delivery where possible

embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/comms/api/ApiResponse.kt

@@ -12,6 +12,8 @@ public sealed class ApiResponse {
         get() = when (this) {
             is TooManyRequests -> true
             is Incomplete -> true
+            is None -> true
+            is Failure -> code in 500..599
             else -> false
         }
 
@@ -44,4 +46,9 @@ public sealed class ApiResponse {
      * Represents an exception thrown while making the API call.
      */
     public data class Incomplete(val exception: Throwable) : ApiResponse()
+
+    /**
+     * No response was received
+     */
+    public object None : ApiResponse()
 }

embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/comms/api/ApiService.kt

@@ -35,5 +35,5 @@ public interface ApiService : RemoteConfigSource, NetworkConnectivityListener {
      * Sends a session to the API. This can be either a v1 or v2 session - the implementation
      * is responsible for routing the payload correctly.
      */
-    public fun sendSession(action: SerializationAction, onFinish: ((successful: Boolean) -> Unit)?): Future<*>?
+    public fun sendSession(action: SerializationAction, onFinish: ((response: ApiResponse) -> Unit)): Future<*>?
 }

embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/comms/api/EmbraceApiService.kt

@@ -77,7 +77,7 @@ internal class EmbraceApiService(
                 null
             }
 
-            is ApiResponse.Failure -> {
+            is ApiResponse.Failure, ApiResponse.None -> {
                 logger.logInfo("Failed to fetch config (no response).")
                 null
             }
@@ -131,15 +131,15 @@ internal class EmbraceApiService(
         post(eventMessage, mapper::eventMessageRequest)
     }
 
-    override fun sendSession(action: SerializationAction, onFinish: ((successful: Boolean) -> Unit)?): Future<*> {
+    override fun sendSession(action: SerializationAction, onFinish: ((response: ApiResponse) -> Unit)): Future<*> {
         return postOnWorker(action, mapper.sessionRequest(), onFinish)
     }
 
     private inline fun <reified T> post(
         payload: T,
         mapper: (T) -> ApiRequest,
         type: ParameterizedType? = null,
-        noinline onComplete: ((successful: Boolean) -> Unit)? = null,
+        noinline onComplete: ((response: ApiResponse) -> Unit) = {}
     ): Future<*> {
         val request: ApiRequest = mapper(payload)
         val action: SerializationAction = { stream ->
@@ -162,20 +162,20 @@ internal class EmbraceApiService(
     private fun postOnWorker(
         action: SerializationAction,
         request: ApiRequest,
-        onComplete: ((successful: Boolean) -> Any)?,
+        onComplete: ((response: ApiResponse) -> Unit),
     ): Future<*> {
         val priority = when (request.isSessionRequest()) {
             true -> TaskPriority.CRITICAL
             else -> TaskPriority.NORMAL
         }
         return backgroundWorker.submit(priority) {
-            var successfullySent = false
+            var response: ApiResponse = ApiResponse.None
             try {
-                successfullySent = handleApiRequest(request, action)
+                response = handleApiRequest(request, action)
             } catch (e: Exception) {
                 logger.logWarning("API call failed.", e)
             } finally {
-                onComplete?.invoke(successfullySent)
+                onComplete(response)
             }
         }
     }
@@ -184,7 +184,7 @@ internal class EmbraceApiService(
      * Handles an API request by executing it if the device is online and the endpoint is not rate limited.
      * Otherwise, the API call is saved to be sent later.
      */
-    private fun handleApiRequest(request: ApiRequest, action: SerializationAction): Boolean {
+    private fun handleApiRequest(request: ApiRequest, action: SerializationAction): ApiResponse {
         val url = EmbraceUrl.create(request.url.url)
         val endpoint = url.endpoint()
 
@@ -196,19 +196,12 @@ internal class EmbraceApiService(
                 pendingApiCallsSender.savePendingApiCall(request, action)
                 pendingApiCallsSender.scheduleRetry(response)
             }
-
-            if (response !is ApiResponse.Success) {
-                // If the API call failed, propagate the error to the caller.
-                error("Failed to post Embrace API call. $response")
-            } else {
-                return true
-            }
+            return response
         } else {
             // Otherwise, save the API call to send it once the rate limit is lifted or the device is online again.
             pendingApiCallsSender.savePendingApiCall(request, action)
         }
-
-        return false
+        return ApiResponse.None
     }
 
     /**

embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/comms/delivery/EmbraceDeliveryService.kt

@@ -57,8 +57,8 @@ internal class EmbraceDeliveryService(
                         serializer.toJson(envelope, Envelope.sessionEnvelopeType)
                     }
                 }
-            val future = apiService.sendSession(action) { successful ->
-                if (!successful) {
+            val future = apiService.sendSession(action) { response ->
+                if (!response.shouldRetry) {
                     val message =
                         "Session deleted without request being sent: ID $sessionId"
                     logger.logWarning(message, SessionPurgeException(message))
@@ -144,8 +144,8 @@ internal class EmbraceDeliveryService(
                 val sessionId = cachedSession.sessionId
                 val action = cacheManager.loadSessionAsAction(sessionId)
                 if (action != null) {
-                    apiService.sendSession(action) { successful ->
-                        if (!successful) {
+                    apiService.sendSession(action) { response ->
+                        if (!response.shouldRetry) {
                             val message = "Cached session deleted without request being sent. File name: ${cachedSession.filename}"
                             logger.logWarning(message, SessionPurgeException(message))
                         }

embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/comms/api/ApiResponseTest.kt

@@ -0,0 +1,21 @@
+package io.embrace.android.embracesdk.internal.comms.api
+
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+internal class ApiResponseTest {
+
+    @Test
+    fun `test retry logic`() {
+        assertFalse(ApiResponse.Success(null, null).shouldRetry)
+        assertFalse(ApiResponse.NotModified.shouldRetry)
+        assertFalse(ApiResponse.PayloadTooLarge.shouldRetry)
+        assertFalse(ApiResponse.Failure(400, null).shouldRetry)
+
+        assertTrue(ApiResponse.TooManyRequests(Endpoint.EVENTS, null).shouldRetry)
+        assertTrue(ApiResponse.Incomplete(RuntimeException()).shouldRetry)
+        assertTrue(ApiResponse.None.shouldRetry)
+        assertTrue(ApiResponse.Failure(500, null).shouldRetry)
+    }
+}

embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeApiService.kt

@@ -1,5 +1,6 @@
 package io.embrace.android.embracesdk.fakes
 
+import io.embrace.android.embracesdk.internal.comms.api.ApiResponse
 import io.embrace.android.embracesdk.internal.comms.api.ApiService
 import io.embrace.android.embracesdk.internal.comms.api.CachedConfig
 import io.embrace.android.embracesdk.internal.comms.delivery.NetworkStatus
@@ -49,15 +50,15 @@ public class FakeApiService : ApiService {
         eventRequests.add(eventMessage)
     }
 
-    override fun sendSession(action: SerializationAction, onFinish: ((successful: Boolean) -> Unit)?): Future<*> {
+    override fun sendSession(action: SerializationAction, onFinish: ((response: ApiResponse) -> Unit)): Future<*> {
         if (throwExceptionSendSession) {
             error("FakeApiService.sendSession")
         }
         val stream = ByteArrayOutputStream()
         action(stream)
         val obj = readBodyAsSessionEnvelope(stream.toByteArray().inputStream())
         sessionRequests.add(obj)
-        onFinish?.invoke(true)
+        onFinish(ApiResponse.None)
         return ObservableFutureTask { }
     }