delay OneSignal network requests if Retry-After

When we get a Retry-After we delay all future requests until after this
time as passed. To be safe we assume the server's Retry-After value
means all traffic should be delayed until then. If we wanted finer
grain control we could work with the the backend to decide which
endpoints should be affected.

Author: jkasten2

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)
@@ -172,6 +184,8 @@ internal class HttpClient(
                     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 -> {

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/http/HttpClientTests.kt

@@ -2,6 +2,7 @@ package com.onesignal.core.internal.http
 
 import com.onesignal.common.OneSignalUtils
 import com.onesignal.core.internal.http.impl.HttpClient
+import com.onesignal.core.internal.time.impl.Time
 import com.onesignal.debug.LogLevel
 import com.onesignal.debug.internal.logging.Logging
 import com.onesignal.mocks.MockHelper
@@ -12,14 +13,15 @@ import io.kotest.matchers.shouldBe
 import io.kotest.matchers.shouldNotBe
 import io.kotest.matchers.types.beInstanceOf
 import kotlinx.coroutines.TimeoutCancellationException
+import kotlinx.coroutines.withTimeoutOrNull
 import org.json.JSONObject
 
 class Mocks {
     internal val mockConfigModel = MockHelper.configModelStore()
     internal val response = MockHttpConnectionFactory.MockResponse()
     internal val factory = MockHttpConnectionFactory(response)
     internal val httpClient by lazy {
-        HttpClient(factory, MockPreferencesService(), mockConfigModel)
+        HttpClient(factory, MockPreferencesService(), mockConfigModel, Time())
     }
 }
 
@@ -208,4 +210,43 @@ class HttpClientTests : FunSpec({
         // Then
         response.retryAfterSeconds shouldBe 60
     }
+
+    // If the OneSignal server ever responses with a Retry-After we want to
+    // make sure we delay any future requests until that delay has past. To
+    // be safe we assume the server's Retry-After value means all traffic
+    // should be delayed until then. If we wanted finer grain control we
+    // could work with the the backend to decide which endpoints should be
+    // affected.
+    test("ensure next request is delayed by the Retry-After value") {
+        // Given
+        val mocks = Mocks()
+
+        val mockRetryAfterResponse = MockHttpConnectionFactory.MockResponse()
+        mockRetryAfterResponse.status = 429
+        mockRetryAfterResponse.mockProps["Retry-After"] = "1"
+        mockRetryAfterResponse.errorResponseBody = "{}"
+
+        val mockSuccessfulResponse = MockHttpConnectionFactory.MockResponse()
+        mockSuccessfulResponse.status = 200
+        mockSuccessfulResponse.responseBody = "{}"
+
+        // When
+        mocks.factory.mockResponse = mockRetryAfterResponse
+        mocks.httpClient.post("URL", JSONObject())
+
+        mocks.factory.mockResponse = mockSuccessfulResponse
+        val response2 =
+            withTimeoutOrNull(999) {
+                mocks.httpClient.post("URL", JSONObject())
+            }
+
+        val response3 =
+            withTimeoutOrNull(100) {
+                mocks.httpClient.post("URL", JSONObject())
+            }
+
+        // Then
+        response2 shouldBe null
+        response3 shouldNotBe null
+    }
 })