Convert ApiResponse into a seal class.

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/ApiClient.kt

@@ -7,13 +7,13 @@ internal interface ApiClient {
     /**
      * Executes [ApiRequest] as a GET, returning the response as a [ApiResponse]
      */
-    fun executeGet(request: ApiRequest): ApiResponse<String>
+    fun executeGet(request: ApiRequest): ApiResponse
 
     /**
      * Executes [ApiRequest] as a POST with the given body defined by [payloadToCompress], returning the response as a [ApiResponse].
      * The body will be gzip compressed.
      */
-    fun executePost(request: ApiRequest, payloadToCompress: ByteArray): ApiResponse<String>
+    fun executePost(request: ApiRequest, payloadToCompress: ByteArray): ApiResponse
 
     companion object {
         /**
@@ -23,6 +23,8 @@ internal interface ApiClient {
 
         const val NO_HTTP_RESPONSE = -1
 
+        const val TOO_MANY_REQUESTS = 429
+
         const val defaultTimeoutMs = 60 * 1000
     }
 }

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/ApiClientImpl.kt

@@ -1,13 +1,16 @@
 package io.embrace.android.embracesdk.comms.api
 
 import io.embrace.android.embracesdk.comms.api.ApiClient.Companion.NO_HTTP_RESPONSE
+import io.embrace.android.embracesdk.comms.api.ApiClient.Companion.TOO_MANY_REQUESTS
 import io.embrace.android.embracesdk.comms.api.ApiClient.Companion.defaultTimeoutMs
 import io.embrace.android.embracesdk.logging.InternalEmbraceLogger
 import java.io.ByteArrayOutputStream
 import java.io.IOException
 import java.io.InputStream
 import java.io.InputStreamReader
-import java.net.HttpURLConnection
+import java.net.HttpURLConnection.HTTP_ENTITY_TOO_LARGE
+import java.net.HttpURLConnection.HTTP_NOT_MODIFIED
+import java.net.HttpURLConnection.HTTP_OK
 import java.util.zip.GZIPOutputStream
 
 /**
@@ -24,30 +27,31 @@
     private val logger: InternalEmbraceLogger
 ) : ApiClient {
 
-    override fun executeGet(request: ApiRequest): ApiResponse<String> {
+    override fun executeGet(request: ApiRequest): ApiResponse {
         var connection: EmbraceConnection? = null
 
-        try {
+        return try {
             connection = request.toConnection()
             setTimeouts(connection)
             connection.connect()
-            return executeHttpRequest(connection)
+            val response = executeHttpRequest(connection)
+            response
         } catch (ex: Throwable) {
-            throw IllegalStateException(ex.localizedMessage ?: "", ex)
+            ApiResponse.Incomplete(IllegalStateException(ex.localizedMessage ?: "", ex))
         } finally {
             runCatching {
                 connection?.inputStream?.close()
             }
         }
     }
 
-    override fun executePost(request: ApiRequest, payloadToCompress: ByteArray): ApiResponse<String> =
+    override fun executePost(request: ApiRequest, payloadToCompress: ByteArray): ApiResponse =
         executeRawPost(request, gzip(payloadToCompress))
 
     /**
      * Posts a payload according to the ApiRequest parameter. The payload will not be gzip compressed.
      */
-    private fun executeRawPost(request: ApiRequest, payload: ByteArray?): ApiResponse<String> {
+    private fun executeRawPost(request: ApiRequest, payload: ByteArray?): ApiResponse {
         logger.logDeveloper("ApiClient", request.httpMethod.toString() + " " + request.url)
         logger.logDeveloper("ApiClient", "Request details: $request")
 
@@ -60,13 +64,10 @@
                 connection.outputStream?.write(payload)
                 connection.connect()
             }
-
             val response = executeHttpRequest(connection)
-            // pre-existing behavior. handle this better in future.
-            check(response.statusCode == HttpURLConnection.HTTP_OK) { "Failed to retrieve from Embrace server." }
             response
         } catch (ex: Throwable) {
-            throw IllegalStateException(ex.localizedMessage ?: "", ex)
+            ApiResponse.Incomplete(IllegalStateException(ex.localizedMessage ?: "", ex))
         } finally {
             runCatching {
                 connection?.inputStream?.close()
@@ -83,17 +84,35 @@
      * Executes a HTTP call using the specified connection, returning the response from the
      * server as a string.
      */
-    private fun executeHttpRequest(connection: EmbraceConnection): ApiResponse<String> {
-        try {
+    private fun executeHttpRequest(connection: EmbraceConnection): ApiResponse {
+        return try {
             val responseCode = readHttpResponseCode(connection)
-            val headers = readHttpResponseHeaders(connection)
-            return ApiResponse(
-                responseCode,
-                headers,
-                readResponseBodyAsString(connection.inputStream)
-            )
+            val responseHeaders = readHttpResponseHeaders(connection)
+
+            return when (responseCode) {
+                HTTP_OK -> {
+                    val responseBody = readResponseBodyAsString(connection.inputStream)
+                    ApiResponse.Success(responseBody, responseHeaders)
+                }
+                HTTP_NOT_MODIFIED -> {
+                    ApiResponse.NotModified
+                }
+                HTTP_ENTITY_TOO_LARGE -> {
+                    ApiResponse.PayloadTooLarge
+                }
+                TOO_MANY_REQUESTS -> {
+                    val retryAfter = responseHeaders["Retry-After"]?.toLongOrNull()
+                    ApiResponse.TooManyRequests(retryAfter)
+                }
+                NO_HTTP_RESPONSE -> {
+                    ApiResponse.Incomplete(IllegalStateException("Connection failed or unexpected response code"))
+                }
+                else -> {
+                    ApiResponse.Failure(responseCode, responseHeaders)
+                }
+            }
         } catch (exc: Throwable) {
-            throw IllegalStateException("Error occurred during HTTP request execution", exc)
+            ApiResponse.Incomplete(IllegalStateException("Error occurred during HTTP request execution", exc))
         }
     }
 

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/ApiResponse.kt

@@ -1,7 +1,36 @@
 package io.embrace.android.embracesdk.comms.api
 
-internal data class ApiResponse<T>(
-    val statusCode: Int?,
-    val headers: Map<String, String>,
-    val body: T?
-)
+/**
+ * ApiResponse is a sealed class that represents the result of an API call.
+ */
+internal sealed class ApiResponse {
+    /**
+     * Represents an API call that returned a 200 OK status code.
+     */
+    data class Success(val body: String?, val headers: Map<String, String>?) : ApiResponse()
+
+    /**
+     * Represents an API call that returned a 304 Not Modified status code.
+     */
+    object NotModified : ApiResponse()
+
+    /**
+     * Represents an API call that returned a 413 Payload Too Large status code.
+     */
+    object PayloadTooLarge : ApiResponse()
+
+    /**
+     * Represents an API call that returned a 429 Too Many Requests status code.
+     */
+    data class TooManyRequests(val retryAfter: Long?) : ApiResponse()
+
+    /**
+     * Represents a failed API call. (status code 400-499 or 500-599 except 413 and 429)
+     */
+    data class Failure(val code: Int, val headers: Map<String, String>?) : ApiResponse()
+
+    /**
+     * Represents an exception thrown while making the API call.
+     */
+    data class Incomplete(val exception: Throwable) : ApiResponse()
+}

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/EmbraceApiService.kt

@@ -15,7 +15,6 @@
 import io.embrace.android.embracesdk.payload.EventMessage
 import io.embrace.android.embracesdk.payload.NetworkEvent
 import java.io.StringReader
-import java.net.HttpURLConnection
 import java.util.concurrent.Future
 import java.util.concurrent.ScheduledExecutorService
 import java.util.concurrent.TimeUnit
@@ -53,33 +52,39 @@
      * @return a future containing the configuration.
      */
     @Throws(IllegalStateException::class)
+    @Suppress("UseCheckOrError")
     override fun getConfig(): RemoteConfig? {
         var request = prepareConfigRequest(configUrl)
         val cachedResponse = cachedConfigProvider(configUrl, request)
         if (cachedResponse.isValid()) { // only bother if we have a useful response.
             request = request.copy(eTag = cachedResponse.eTag)
         }
-        val response = apiClient.executeGet(request)
 
-        return when (response.statusCode) {
-            HttpURLConnection.HTTP_OK -> {
+        return when (val response = apiClient.executeGet(request)) {
+            is ApiResponse.Success -> {
                 logger.logInfo("Fetched new config successfully.")
                 val jsonReader = JsonReader(StringReader(response.body))
                 serializer.loadObject(jsonReader, RemoteConfig::class.java)
             }
-
-            HttpURLConnection.HTTP_NOT_MODIFIED -> {
+            is ApiResponse.NotModified -> {
                 logger.logInfo("Confirmed config has not been modified.")
                 cachedResponse.remoteConfig
             }
-
-            ApiClient.NO_HTTP_RESPONSE -> {
+            is ApiResponse.TooManyRequests -> {
+                // TODO: We should retry after the retryAfter time or 3 seconds and apply exponential backoff.
+                logger.logWarning("Too many requests. ")
+                null
+            }
+            is ApiResponse.Failure -> {
                 logger.logInfo("Failed to fetch config (no response).")
                 null
             }
-
-            else -> {
-                logger.logWarning("Unexpected status code when fetching config: ${response.statusCode}")
+            is ApiResponse.Incomplete -> {
+                logger.logWarning("Failed to fetch config.", response.exception)
+                throw response.exception
+            }
+            ApiResponse.PayloadTooLarge -> {
+                // Not expected to receive a 413 response for a GET request.
                 null
             }
         }
@@ -209,8 +214,32 @@
         }
     }
 
+    @Suppress("UseCheckOrError")
     private fun executePost(request: ApiRequest, payload: ByteArray) {
-        apiClient.executePost(request, payload)
+        when (val response = apiClient.executePost(request, payload)) {
+            is ApiResponse.Success -> {}
+            is ApiResponse.TooManyRequests -> {
+                // Temporarily, we just throw an exception. In the future,
+                // we will use the retryAfter to schedule a retry.
+                val retryAfter = response.retryAfter ?: 3
+                throw IllegalStateException("Too many requests. Will retry after $retryAfter seconds.")
+            }
+            is ApiResponse.PayloadTooLarge -> {
+                // We don't want to retry on PayloadTooLarge error, so we just log the error.
+                logger.logError("Payload too large. Dropping event.")
+            }
+            is ApiResponse.Failure -> {
+                // We don't want to retry on 4xx or 5xx errors, so we just log the error.
+                logger.logError("Failed to retrieve from Embrace server. Status code: ${response.code}")
+            }
+            is ApiResponse.Incomplete -> {
+                // Retry on incomplete response.
+                throw IllegalStateException("Failed to retrieve from Embrace server.")
+            }
+            is ApiResponse.NotModified -> {
+                // Not expected to receive a 304 response for a POST request.
+            }
+        }
     }
 
     companion object {