From 0cf1eb9969511e23e2301f4abb66bc96db59799b Mon Sep 17 00:00:00 2001
From: Nan <nanftw@gmail.com>
Date: Fri, 3 Nov 2023 20:22:52 -0700
Subject: [PATCH] pause operation repo and retry on failed user create

Successful user creation is vital to the functioning of the SDK.

Problem:
If the create user request fails with an un-retryable error such as a 400 response, the SDK would not retry and stay in an unrecoverable error state with no onesignal_id, and no subscription_id (potentially). Therefore, it would never register, send data, or receive notifications. The only way out was to uninstall the app.

Solution:
Let's give the SDK a chance to recover from failed user creation, similar to the behavior of the iOS SDK. When met with this error, we will pause the operation repo from executing any more operations as it is impossible to do anything without a onesignal_id.

Then, on new sessions or new cold starts, we will retry the still-cached operation, in the hopes that perhaps it can succeed at this later date.
---
 .../internal/operations/IOperationExecutor.kt     |  7 +++++++
 .../internal/operations/impl/OperationRepo.kt     | 15 +++++++++++++++
 .../impl/executors/LoginUserOperationExecutor.kt  |  2 +-
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/IOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/IOperationExecutor.kt
index 590d24e0c..af853b541 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/IOperationExecutor.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/IOperationExecutor.kt
@@ -71,4 +71,11 @@ enum class ExecutionResult {
      * The operation failed due to a conflict and can be handled.
      */
     FAIL_CONFLICT,
+
+    /**
+     * Used in special create user case.
+     * The operation failed due to a non-retryable error. Pause the operation repo
+     * and retry on a new session, giving the SDK a chance to recover from the failed user create.
+     */
+    FAIL_PAUSE_OPREPO,
 }
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt
index cb478cd0f..23a284ebc 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt
@@ -30,6 +30,7 @@ internal class OperationRepo(
     private val executorsMap: Map<String, IOperationExecutor>
     private val queue = mutableListOf<OperationQueueItem>()
     private val waiter = WaiterWithValue<Boolean>()
+    private var paused = false
 
     init {
         val executorsMap: MutableMap<String, IOperationExecutor> = mutableMapOf()
@@ -47,6 +48,7 @@ internal class OperationRepo(
     }
 
     override fun start() {
+        paused = false
         suspendifyOnThread(name = "OpRepo") {
             processQueueForever()
         }
@@ -99,6 +101,10 @@ internal class OperationRepo(
 
         // This runs forever, until the application is destroyed.
         while (true) {
+            if (paused) {
+                Logging.debug("OperationRepo is paused")
+                return
+            }
             try {
                 var ops: List<OperationQueueItem>? = null
 
@@ -199,6 +205,15 @@ internal class OperationRepo(
                         ops.reversed().forEach { queue.add(0, it) }
                     }
                 }
+                ExecutionResult.FAIL_PAUSE_OPREPO -> {
+                    Logging.error("Operation execution failed with eventual retry, pausing the operation repo: $operations")
+                    // keep the failed operation and pause the operation repo from executing
+                    paused = true
+                    // add back all operations to the front of the queue to be re-executed.
+                    synchronized(queue) {
+                        ops.reversed().forEach { queue.add(0, it) }
+                    }
+                }
             }
 
             // if there are operations provided on the result, we need to enqueue them at the
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt
index 159ffa2a3..3f29b9097 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt
@@ -205,7 +205,7 @@ internal class LoginUserOperationExecutor(
                 NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
                     ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
                 else ->
-                    ExecutionResponse(ExecutionResult.FAIL_NORETRY)
+                    ExecutionResponse(ExecutionResult.FAIL_PAUSE_OPREPO)
             }
         }
     }