Merge pull request #2064 from OneSignal/improve/handle-http-response-…

…header-retry-after

[Improve] Handle HTTP header Retry-After from responses from OneSignal

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/NetworkUtils.kt

@@ -17,6 +17,7 @@ object NetworkUtils {
             401, 403 -> ResponseStatusType.UNAUTHORIZED
             404, 410 -> ResponseStatusType.MISSING
             409 -> ResponseStatusType.CONFLICT
+            429 -> ResponseStatusType.RETRYABLE
             else -> ResponseStatusType.RETRYABLE
         }
     }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/exceptions/BackendException.kt

@@ -12,4 +12,9 @@ class BackendException(
      * The response, if one exists.
      */
     val response: String? = null,
+    /**
+     * Optional Integer value maybe returned from the backend.
+     * The module handing this should delay any future requests by this time.
+     */
+    val retryAfterSeconds: Int? = null,
 ) : Exception()

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt

@@ -34,7 +34,7 @@ internal class ParamsBackendService(
         val response = _http.get(paramsUrl, CacheKeys.REMOTE_PARAMS)
 
         if (!response.isSuccess) {
-            throw BackendException(response.statusCode, response.payload)
+            throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
         }
 
         val responseJson = JSONObject(response.payload!!)

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt

@@ -111,6 +111,16 @@ class ConfigModel : Model() {
             setIntProperty(::httpGetTimeout.name, value)
         }
 
+    /**
+     * The fallback Retry-After to use if the header is present, but the server
+     * give us a format we can't parse.
+     */
+    var httpRetryAfterParseFailFallback: Int
+        get() = getIntProperty(::httpRetryAfterParseFailFallback.name) { 60 }
+        set(value) {
+            setIntProperty(::httpRetryAfterParseFailFallback.name, value)
+        }
+
     /**
      * Maximum time in milliseconds a user can spend out of focus before a new session is created.
      */
@@ -167,6 +177,18 @@ class ConfigModel : Model() {
             setLongProperty(::opRepoPostCreateRetryUpTo.name, value)
         }
 
+    /**
+     * The number of milliseconds times the number of times FAIL_RETRY
+     * is returned from an executor for a specific operation. AKA this
+     * backoff will increase each time we retry a specific operation
+     * by this value.
+     */
+    var opRepoDefaultFailRetryBackoff: Long
+        get() = getLongProperty(::opRepoDefaultFailRetryBackoff.name) { 15_000 }
+        set(value) {
+            setLongProperty(::opRepoDefaultFailRetryBackoff.name, value)
+        }
+
     /**
      * The minimum number of milliseconds required to pass to allow the fetching of IAM to occur.
      */

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/HttpResponse.kt

@@ -18,6 +18,11 @@ class HttpResponse(
      * When non-null, the throwable that was thrown during processing.
      */
     val throwable: Throwable? = null,
+    /**
+     * Optional Integer value maybe returned from the backend.
+     * The module handing this should delay any future requests by this time.
+     */
+    val retryAfterSeconds: Int? = null,
 ) {
     /**
      * Whether the response is a successful one.

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt

@@ -11,11 +11,13 @@ import com.onesignal.core.internal.http.IHttpClient
 import com.onesignal.core.internal.preferences.IPreferencesService
 import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys
 import com.onesignal.core.internal.preferences.PreferenceStores
+import com.onesignal.core.internal.time.ITime
 import com.onesignal.debug.internal.logging.Logging
 import kotlinx.coroutines.DelicateCoroutinesApi
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.GlobalScope
 import kotlinx.coroutines.TimeoutCancellationException
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withTimeout
 import org.json.JSONObject
@@ -29,7 +31,14 @@ internal class HttpClient(
     private val _connectionFactory: IHttpConnectionFactory,
     private val _prefs: IPreferencesService,
     private val _configModelStore: ConfigModelStore,
+    private val _time: ITime,
 ) : IHttpClient {
+    /**
+     * Delay making network requests until we reach this time.
+     * Used when the OneSignal backend returns a Retry-After value.
+     */
+    private var delayNewRequestsUntil = 0L
+
     override suspend fun post(
         url: String,
         body: JSONObject,
@@ -77,6 +86,9 @@ internal class HttpClient(
             return HttpResponse(0, null, null)
         }
 
+        val delayUntil = delayNewRequestsUntil - _time.currentTimeMillis
+        if (delayUntil > 0) delay(delayUntil)
+
         try {
             return withTimeout(getThreadTimeout(timeout).toLong()) {
                 return@withTimeout makeRequestIODispatcher(url, method, jsonBody, timeout, cacheKey)
@@ -171,6 +183,10 @@ internal class HttpClient(
                     // Network request is made from getResponseCode()
                     httpResponse = con.responseCode
 
+                    val retryAfter = retryAfterFromResponse(con)
+                    val newDelayUntil = _time.currentTimeMillis + (retryAfter ?: 0) * 1_000
+                    if (newDelayUntil > delayNewRequestsUntil) delayNewRequestsUntil = newDelayUntil
+
                     when (httpResponse) {
                         HttpURLConnection.HTTP_NOT_MODIFIED -> {
                             val cachedResponse =
@@ -181,7 +197,7 @@ internal class HttpClient(
                             Logging.debug("HttpClient: ${method ?: "GET"} $url - Using Cached response due to 304: " + cachedResponse)
 
                             // TODO: SHOULD RETURN OK INSTEAD OF NOT_MODIFIED TO MAKE TRANSPARENT?
-                            retVal = HttpResponse(httpResponse, cachedResponse)
+                            retVal = HttpResponse(httpResponse, cachedResponse, retryAfterSeconds = retryAfter)
                         }
                         HttpURLConnection.HTTP_ACCEPTED, HttpURLConnection.HTTP_CREATED, HttpURLConnection.HTTP_OK -> {
                             val inputStream = con.inputStream
@@ -208,7 +224,7 @@ internal class HttpClient(
                                 }
                             }
 
-                            retVal = HttpResponse(httpResponse, json)
+                            retVal = HttpResponse(httpResponse, json, retryAfterSeconds = retryAfter)
                         }
                         else -> {
                             Logging.debug("HttpClient: ${method ?: "GET"} $url - FAILED STATUS: $httpResponse")
@@ -229,7 +245,7 @@ internal class HttpClient(
                                 Logging.warn("HttpClient: $method HTTP Code: $httpResponse No response body!")
                             }
 
-                            retVal = HttpResponse(httpResponse, jsonResponse)
+                            retVal = HttpResponse(httpResponse, jsonResponse, retryAfterSeconds = retryAfter)
                         }
                     }
                 } catch (t: Throwable) {
@@ -253,6 +269,22 @@ internal class HttpClient(
         return timeout + 5000
     }
 
+    /**
+     * Reads the HTTP Retry-After from the response.
+     * Only supports number format, not the date format.
+     */
+    private fun retryAfterFromResponse(con: HttpURLConnection): Int? {
+        val retryAfterStr = con.getHeaderField("Retry-After")
+        return if (retryAfterStr != null) {
+            Logging.debug("HttpClient: Response Retry-After: $retryAfterStr")
+            retryAfterStr.toIntOrNull() ?: _configModelStore.model.httpRetryAfterParseFailFallback
+        } else if (con.responseCode == 429) {
+            _configModelStore.model.httpRetryAfterParseFailFallback
+        } else {
+            null
+        }
+    }
+
     companion object {
         private const val OS_API_VERSION = "1"
         private const val OS_ACCEPT_HEADER = "application/vnd.onesignal.v$OS_API_VERSION+json"

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/IOperationExecutor.kt

@@ -37,6 +37,11 @@ class ExecutionResponse(
      * When specified, any operations that should be prepended to the operation repo.
      */
     val operations: List<Operation>? = null,
+    /**
+     * Optional Integer value maybe returned from the backend.
+     * The module handing this should delay any future requests by this time.
+     */
+    val retryAfterSeconds: Int? = null,
 )
 
 enum class ExecutionResult {

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt

@@ -18,6 +18,7 @@ import kotlinx.coroutines.launch
 import kotlinx.coroutines.newSingleThreadContext
 import kotlinx.coroutines.withTimeoutOrNull
 import java.util.UUID
+import kotlin.math.max
 import kotlin.reflect.KClass
 
 internal class OperationRepo(
@@ -221,6 +222,7 @@ internal class OperationRepo(
                 }
             }
 
+            var highestRetries = 0
             when (response.result) {
                 ExecutionResult.SUCCESS -> {
                     // on success we remove the operation from the store and wake any waiters
@@ -248,7 +250,6 @@ internal class OperationRepo(
                 ExecutionResult.FAIL_RETRY -> {
                     Logging.error("Operation execution failed, retrying: $operations")
                     // add back all operations to the front of the queue to be re-executed.
-                    var highestRetries = 0
                     synchronized(queue) {
                         ops.reversed().forEach {
                             if (++it.retries > highestRetries) {
@@ -257,7 +258,6 @@ internal class OperationRepo(
                             queue.add(0, it)
                         }
                     }
-                    delayBeforeRetry(highestRetries)
                 }
                 ExecutionResult.FAIL_PAUSE_OPREPO -> {
                     Logging.error("Operation execution failed with eventual retry, pausing the operation repo: $operations")
@@ -282,6 +282,8 @@ internal class OperationRepo(
                     }
                 }
             }
+
+            delayBeforeNextExecution(highestRetries, response.retryAfterSeconds)
         } catch (e: Throwable) {
             Logging.log(LogLevel.ERROR, "Error attempting to execute operation: $ops", e)
 
@@ -291,8 +293,18 @@ internal class OperationRepo(
         }
     }
 
-    suspend fun delayBeforeRetry(retries: Int) {
-        val delayFor = retries * 15_000L
+    /**
+     * Wait which ever is longer, retryAfterSeconds returned by the server,
+     * or based on the retry count.
+     */
+    suspend fun delayBeforeNextExecution(
+        retries: Int,
+        retryAfterSeconds: Int?,
+    ) {
+        Logging.debug("retryAfterSeconds: $retryAfterSeconds")
+        val retryAfterSecondsNonNull = retryAfterSeconds?.toLong() ?: 0L
+        val delayForOnRetries = retries * _configModelStore.model.opRepoDefaultFailRetryBackoff
+        val delayFor = max(delayForOnRetries, retryAfterSecondsNonNull * 1_000)
         if (delayFor < 1) return
         Logging.error("Operations being delay for: $delayFor ms")
         delay(delayFor)

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsBackendService.kt

@@ -49,7 +49,7 @@ internal class OutcomeEventsBackendService(private val _http: IHttpClient) :
         val response = _http.post("outcomes/measure", jsonObject)
 
         if (!response.isSuccess) {
-            throw BackendException(response.statusCode, response.payload)
+            throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
         }
     }
 }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/backend/impl/IdentityBackendService.kt

@@ -23,7 +23,7 @@ internal class IdentityBackendService(
         val response = _httpClient.patch("apps/$appId/users/by/$aliasLabel/$aliasValue/identity", requestJSONObject)
 
         if (!response.isSuccess) {
-            throw BackendException(response.statusCode, response.payload)
+            throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
         }
 
         val responseJSON = JSONObject(response.payload!!)
@@ -40,7 +40,7 @@ internal class IdentityBackendService(
         val response = _httpClient.delete("apps/$appId/users/by/$aliasLabel/$aliasValue/identity/$aliasLabelToDelete")
 
         if (!response.isSuccess) {
-            throw BackendException(response.statusCode, response.payload)
+            throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
         }
     }
 }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/backend/impl/SubscriptionBackendService.kt

@@ -24,7 +24,7 @@ internal class SubscriptionBackendService(
         val response = _httpClient.post("apps/$appId/users/by/$aliasLabel/$aliasValue/subscriptions", requestJSON)
 
         if (!response.isSuccess) {
-            throw BackendException(response.statusCode, response.payload)
+            throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
         }
 
         val responseJSON = JSONObject(response.payload!!)
@@ -48,7 +48,7 @@ internal class SubscriptionBackendService(
         val response = _httpClient.patch("apps/$appId/subscriptions/$subscriptionId", requestJSON)
 
         if (!response.isSuccess) {
-            throw BackendException(response.statusCode, response.payload)
+            throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
         }
     }
 
@@ -59,7 +59,7 @@ internal class SubscriptionBackendService(
         val response = _httpClient.delete("apps/$appId/subscriptions/$subscriptionId")
 
         if (!response.isSuccess) {
-            throw BackendException(response.statusCode, response.payload)
+            throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
         }
     }
 
@@ -76,7 +76,7 @@ internal class SubscriptionBackendService(
         val response = _httpClient.patch("apps/$appId/subscriptions/$subscriptionId/owner", requestJSON)
 
         if (!response.isSuccess) {
-            throw BackendException(response.statusCode, response.payload)
+            throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
         }
     }
 
@@ -87,7 +87,7 @@ internal class SubscriptionBackendService(
         val response = _httpClient.get("apps/$appId/subscriptions/$subscriptionId/user/identity")
 
         if (!response.isSuccess) {
-            throw BackendException(response.statusCode, response.payload)
+            throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
         }
 
         val responseJSON = JSONObject(response.payload!!)

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/backend/impl/UserBackendService.kt

@@ -39,7 +39,7 @@ internal class UserBackendService(
         val response = _httpClient.post("apps/$appId/users", requestJSON)
 
         if (!response.isSuccess) {
-            throw BackendException(response.statusCode, response.payload)
+            throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
         }
 
         return JSONConverter.convertToCreateUserResponse(JSONObject(response.payload!!))
@@ -68,7 +68,7 @@ internal class UserBackendService(
         val response = _httpClient.patch("apps/$appId/users/by/$aliasLabel/$aliasValue", jsonObject)
 
         if (!response.isSuccess) {
-            throw BackendException(response.statusCode, response.payload)
+            throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
         }
     }
 
@@ -80,7 +80,7 @@ internal class UserBackendService(
         val response = _httpClient.get("apps/$appId/users/by/$aliasLabel/$aliasValue")
 
         if (!response.isSuccess) {
-            throw BackendException(response.statusCode, response.payload)
+            throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
         }
 
         return JSONConverter.convertToCreateUserResponse(JSONObject(response.payload))