From c98c49efd5354c692dd58ba0600e1f8b38e7b272 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 590d24e0c5..af853b5417 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 cb478cd0fb..23a284ebcd 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 38fff1e35d..1c34da6f51 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
@@ -204,7 +204,7 @@ internal class LoginUserOperationExecutor(
                 NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
                     ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
                 else ->
-                    ExecutionResponse(ExecutionResult.FAIL_NORETRY)
+                    ExecutionResponse(ExecutionResult.FAIL_PAUSE_OPREPO)
             }
         }
     }