fix: [SDK-5535] retry failed operations without blocking the queue

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt

@@ -16,9 +16,11 @@ import com.onesignal.core.internal.config.impl.FeatureFlagsRefreshService
 import com.onesignal.core.internal.config.impl.IdentityVerificationService
 import com.onesignal.core.internal.database.IDatabaseProvider
 import com.onesignal.core.internal.database.impl.DatabaseProvider
+import com.onesignal.core.internal.device.IConnectivityMonitor
 import com.onesignal.core.internal.device.IDeviceService
 import com.onesignal.core.internal.device.IFidEnv
 import com.onesignal.core.internal.device.IInstallIdService
+import com.onesignal.core.internal.device.impl.ConnectivityMonitor
 import com.onesignal.core.internal.device.impl.DeviceService
 import com.onesignal.core.internal.device.impl.FidEnvService
 import com.onesignal.core.internal.device.impl.InstallIdService
@@ -34,6 +36,7 @@ import com.onesignal.core.internal.language.impl.LanguageContext
 import com.onesignal.core.internal.operations.IOperationRepo
 import com.onesignal.core.internal.operations.impl.OperationModelStore
 import com.onesignal.core.internal.operations.impl.OperationRepo
+import com.onesignal.core.internal.operations.impl.OperationRetryTrigger
 import com.onesignal.core.internal.permissions.IRequestPermissionService
 import com.onesignal.core.internal.permissions.impl.RequestPermissionService
 import com.onesignal.core.internal.preferences.IPreferencesService
@@ -69,6 +72,7 @@ internal class CoreModule : IModule {
             .provides<IApplicationService>()
         builder.register<DeviceService>().provides<IDeviceService>()
         builder.register<FidEnvService>().provides<IFidEnv>()
+        builder.register<ConnectivityMonitor>().provides<IConnectivityMonitor>()
         builder.register<Time>().provides<ITime>()
         builder.register<DatabaseProvider>().provides<IDatabaseProvider>()
         builder.register<InstallIdService>().provides<IInstallIdService>()
@@ -91,6 +95,7 @@ internal class CoreModule : IModule {
         builder.register<OperationRepo>()
             .provides<IOperationRepo>()
             .provides<IStartableService>()
+        builder.register<OperationRetryTrigger>().provides<IStartableService>()
 
         // Permissions
         builder.register<RequestPermissionService>()

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

@@ -180,15 +180,31 @@ class ConfigModel : Model() {
         }
 
     /**
-     * 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.
+     * The first retry backoff after FAIL_RETRY, in milliseconds. It doubles with each further
+     * failure of the same operation, up to [opRepoMaxFailRetryBackoff], and is then jittered.
      */
-    var opRepoDefaultFailRetryBackoff: Long
-        get() = getLongProperty(::opRepoDefaultFailRetryBackoff.name) { 15_000 }
+    var opRepoBaseFailRetryBackoff: Long
+        get() = getLongProperty(::opRepoBaseFailRetryBackoff.name) { 5_000 }
         set(value) {
-            setLongProperty(::opRepoDefaultFailRetryBackoff.name, value)
+            setLongProperty(::opRepoBaseFailRetryBackoff.name, value)
+        }
+
+    /**
+     * The longest retry backoff, in milliseconds, before jitter.
+     */
+    var opRepoMaxFailRetryBackoff: Long
+        get() = getLongProperty(::opRepoMaxFailRetryBackoff.name) { 300_000 }
+        set(value) {
+            setLongProperty(::opRepoMaxFailRetryBackoff.name, value)
+        }
+
+    /**
+     * The longest a server Retry-After is honored, in milliseconds.
+     */
+    var maxRetryAfter: Long
+        get() = getLongProperty(::maxRetryAfter.name) { 3_600_000 }
+        set(value) {
+            setLongProperty(::maxRetryAfter.name, value)
         }
 
     /**

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/IConnectivityMonitor.kt

@@ -0,0 +1,9 @@
+package com.onesignal.core.internal.device
+
+internal interface IConnectivityMonitor {
+    /**
+     * Calls [onOnline] each time the device goes from having no internet-capable network to
+     * having one. Not called for the state at registration.
+     */
+    fun addOnlineHandler(onOnline: () -> Unit)
+}

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/ConnectivityMonitor.kt

@@ -0,0 +1,61 @@
+package com.onesignal.core.internal.device.impl
+
+import android.content.Context
+import android.net.ConnectivityManager
+import android.net.Network
+import android.net.NetworkCapabilities
+import android.net.NetworkRequest
+import com.onesignal.core.internal.application.IApplicationService
+import com.onesignal.core.internal.device.IConnectivityMonitor
+import com.onesignal.debug.internal.logging.Logging
+
+internal class ConnectivityMonitor(
+    private val applicationService: IApplicationService,
+) : IConnectivityMonitor {
+    @Suppress("TooGenericExceptionCaught")
+    override fun addOnlineHandler(onOnline: () -> Unit) {
+        try {
+            val connectivityManager =
+                applicationService.appContext.getSystemService(Context.CONNECTIVITY_SERVICE) as? ConnectivityManager
+                    ?: return
+            val request =
+                NetworkRequest.Builder()
+                    .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
+                    .build()
+            connectivityManager.registerNetworkCallback(request, OnlineCallback(isOnline(connectivityManager), onOnline))
+        } catch (t: Throwable) {
+            // Can throw, e.g. when the app already holds the maximum number of network callbacks.
+            Logging.warn("ConnectivityMonitor: unable to register network callback", t)
+        }
+    }
+
+    @Suppress("DEPRECATION")
+    private fun isOnline(connectivityManager: ConnectivityManager) = connectivityManager.activeNetworkInfo?.isConnected == true
+
+    // onAvailable also fires for networks already up at registration. Seeding with the current
+    // state keeps those from counting as coming online.
+    private class OnlineCallback(
+        private var online: Boolean,
+        private val onOnline: () -> Unit,
+    ) : ConnectivityManager.NetworkCallback() {
+        private val networks = mutableSetOf<Network>()
+
+        override fun onAvailable(network: Network) {
+            val cameOnline =
+                synchronized(this) {
+                    networks.add(network)
+                    val wasOnline = online
+                    online = true
+                    !wasOnline
+                }
+            if (cameOnline) onOnline()
+        }
+
+        override fun onLost(network: Network) {
+            synchronized(this) {
+                networks.remove(network)
+                if (networks.isEmpty()) online = false
+            }
+        }
+    }
+}

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

@@ -26,7 +26,10 @@ import java.net.HttpURLConnection
 import java.net.URL
 import java.net.UnknownHostException
 import java.util.Scanner
+import java.util.concurrent.TimeUnit
 import javax.net.ssl.HttpsURLConnection
+import kotlin.math.ceil
+import kotlin.math.min
 
 internal const val HTTP_SDK_VERSION_HEADER_KEY = "SDK-Version"
 internal val HTTP_SDK_VERSION_HEADER_VALUE = "onesignal/android/${OneSignalUtils.sdkVersion}"
@@ -40,9 +43,10 @@ internal class HttpClient(
     private val _fidEnv: IFidEnv,
 ) : IHttpClient {
     /**
-     * Delay making network requests until we reach this time.
+     * Delay making network requests until we reach this time, on [ITime.elapsedRealtimeMillis].
      * Used when the OneSignal backend returns a Retry-After value.
      */
+    @Volatile
     private var delayNewRequestsUntil = 0L
 
     override suspend fun post(
@@ -88,7 +92,7 @@ internal class HttpClient(
             return HttpResponse(0, null, null)
         }
 
-        val delayUntil = delayNewRequestsUntil - _time.currentTimeMillis
+        val delayUntil = delayNewRequestsUntil - _time.elapsedRealtimeMillis
         if (delayUntil > 0) delay(delayUntil)
 
         try {
@@ -218,7 +222,8 @@ internal class HttpClient(
 
                     val retryAfter = retryAfterFromResponse(con)
                     val retryLimit = retryLimitFromResponse(con)
-                    val newDelayUntil = _time.currentTimeMillis + (retryAfter ?: 0) * 1_000
+                    val retryAfterMs = min(TimeUnit.SECONDS.toMillis((retryAfter ?: 0).toLong()), _configModelStore.model.maxRetryAfter)
+                    val newDelayUntil = _time.elapsedRealtimeMillis + retryAfterMs
                     if (newDelayUntil > delayNewRequestsUntil) delayNewRequestsUntil = newDelayUntil
 
                     when (httpResponse) {
@@ -316,7 +321,9 @@ internal class HttpClient(
         val retryAfterStr = con.getHeaderField("Retry-After")
         return if (retryAfterStr != null) {
             Logging.debug("HttpClient: Response Retry-After: $retryAfterStr")
-            retryAfterStr.toIntOrNull() ?: _configModelStore.model.httpRetryAfterParseFailFallback
+            val maxSeconds = TimeUnit.MILLISECONDS.toSeconds(_configModelStore.model.maxRetryAfter).toDouble()
+            retryAfterStr.trim().toDoubleOrNull()?.takeIf { it.isFinite() && it >= 0 }?.let { ceil(min(it, maxSeconds)).toInt() }
+                ?: _configModelStore.model.httpRetryAfterParseFailFallback
         } else if (con.responseCode == 429) {
             _configModelStore.model.httpRetryAfterParseFailFallback
         } else {