Skip to content

Commit 0ba8c2c

Browse files
jkasten2jkasten2
committed
Retry on 404/410 within short window after create
In summary, this is fallback logic to opRepoPostCreateDelay. The opRepoPostCreateDelay is a balance between not waiting too long where the SDK isn't being response, but long enough where the likelihood of running into the backend replica delay issue is low. Given the above, we can further lower the chance of this issue by retrying requests that fail with 404 or 410 for Subscriptions. We don't want to do this with all 404/410 requests so we utilize the same NewRecordsState list, but added a new isInMissingRetryWindow method and opRepoPostCreateRetryUpTo value to drive the window value. Added tests to executors to ensure the new 404 logic is working. Some executors were missing 404 handling altogether so we filled in those gaps in this commit as well.
1 parent 59fd1df commit 0ba8c2c

File tree

11 files changed

+456
-30
lines changed

11 files changed

+456
-30
lines changed

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ class ConfigModel : Model() {
142142
}
143143

144144
/**
145-
* The number milliseconds to delay after an operation completes
145+
* The number of milliseconds to delay after an operation completes
146146
* that creates or changes ids.
147147
* This is a "cold down" period to avoid a caveat with OneSignal's backend
148148
* replication, where you may incorrectly get a 404 when attempting a GET
@@ -154,6 +154,19 @@ class ConfigModel : Model() {
154154
setLongProperty(::opRepoPostCreateDelay.name, value)
155155
}
156156

157+
/**
158+
* The number of milliseconds to retry operations for new models.
159+
* This is a fallback to opRepoPostCreateDelay, where it's delay may
160+
* not be enough. The server may be unusually overloaded so we will
161+
* retry these (back-off rules apply to all retries) as we only want
162+
* to re-create records as a last resort.
163+
*/
164+
var opRepoPostCreateRetryUpTo: Long
165+
get() = getLongProperty(::opRepoPostCreateRetryUpTo.name) { 60_000 }
166+
set(value) {
167+
setLongProperty(::opRepoPostCreateRetryUpTo.name, value)
168+
}
169+
157170
/**
158171
* The minimum number of milliseconds required to pass to allow the fetching of IAM to occur.
159172
*/

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@ import com.onesignal.user.internal.builduser.IRebuildUserService
1414
import com.onesignal.user.internal.identity.IdentityModelStore
1515
import com.onesignal.user.internal.operations.DeleteAliasOperation
1616
import com.onesignal.user.internal.operations.SetAliasOperation
17+
import com.onesignal.user.internal.operations.impl.states.NewRecordsState
1718

1819
internal class IdentityOperationExecutor(
1920
private val _identityBackend: IIdentityBackendService,
2021
private val _identityModelStore: IdentityModelStore,
2122
private val _buildUserService: IRebuildUserService,
23+
private val _newRecordState: NewRecordsState,
2224
) : IOperationExecutor {
2325
override val operations: List<String>
2426
get() = listOf(SET_ALIAS, DELETE_ALIAS)
@@ -67,11 +69,15 @@ internal class IdentityOperationExecutor(
6769
NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
6870
ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
6971
NetworkUtils.ResponseStatusType.MISSING -> {
70-
val operations = _buildUserService.getRebuildOperationsIfCurrentUser(lastOperation.appId, lastOperation.onesignalId)
71-
if (operations == null) {
72+
if (_newRecordState.isInMissingRetryWindow(lastOperation.onesignalId)) {
73+
return ExecutionResponse(ExecutionResult.FAIL_RETRY)
74+
}
75+
76+
val rebuildOps = _buildUserService.getRebuildOperationsIfCurrentUser(lastOperation.appId, lastOperation.onesignalId)
77+
if (rebuildOps == null) {
7278
return ExecutionResponse(ExecutionResult.FAIL_NORETRY)
7379
} else {
74-
return ExecutionResponse(ExecutionResult.FAIL_RETRY, operations = operations)
80+
return ExecutionResponse(ExecutionResult.FAIL_RETRY, operations = rebuildOps)
7581
}
7682
}
7783
}
@@ -103,10 +109,14 @@ internal class IdentityOperationExecutor(
103109
NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
104110
ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
105111
NetworkUtils.ResponseStatusType.MISSING -> {
106-
// This means either the User or the Alias was already
107-
// deleted, either way the end state is the same, the
108-
// alias no longer exists on that User.
109-
ExecutionResponse(ExecutionResult.SUCCESS)
112+
return if (_newRecordState.isInMissingRetryWindow(lastOperation.onesignalId)) {
113+
ExecutionResponse(ExecutionResult.FAIL_RETRY)
114+
} else {
115+
// This means either the User or the Alias was already
116+
// deleted, either way the end state is the same, the
117+
// alias no longer exists on that User.
118+
ExecutionResponse(ExecutionResult.SUCCESS)
119+
}
110120
}
111121
}
112122
}

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import com.onesignal.user.internal.builduser.IRebuildUserService
1717
import com.onesignal.user.internal.identity.IdentityModel
1818
import com.onesignal.user.internal.identity.IdentityModelStore
1919
import com.onesignal.user.internal.operations.RefreshUserOperation
20+
import com.onesignal.user.internal.operations.impl.states.NewRecordsState
2021
import com.onesignal.user.internal.properties.PropertiesModel
2122
import com.onesignal.user.internal.properties.PropertiesModelStore
2223
import com.onesignal.user.internal.subscriptions.SubscriptionModel
@@ -31,6 +32,7 @@ internal class RefreshUserOperationExecutor(
3132
private val _subscriptionsModelStore: SubscriptionModelStore,
3233
private val _configModelStore: ConfigModelStore,
3334
private val _buildUserService: IRebuildUserService,
35+
private val _newRecordState: NewRecordsState,
3436
) : IOperationExecutor {
3537
override val operations: List<String>
3638
get() = listOf(REFRESH_USER)
@@ -135,6 +137,9 @@ internal class RefreshUserOperationExecutor(
135137
NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
136138
ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
137139
NetworkUtils.ResponseStatusType.MISSING -> {
140+
if (_newRecordState.isInMissingRetryWindow(op.onesignalId)) {
141+
return ExecutionResponse(ExecutionResult.FAIL_RETRY)
142+
}
138143
val operations = _buildUserService.getRebuildOperationsIfCurrentUser(op.appId, op.onesignalId)
139144
if (operations == null) {
140145
return ExecutionResponse(ExecutionResult.FAIL_NORETRY)

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/SubscriptionOperationExecutor.kt‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import com.onesignal.user.internal.operations.CreateSubscriptionOperation
2626
import com.onesignal.user.internal.operations.DeleteSubscriptionOperation
2727
import com.onesignal.user.internal.operations.TransferSubscriptionOperation
2828
import com.onesignal.user.internal.operations.UpdateSubscriptionOperation
29+
import com.onesignal.user.internal.operations.impl.states.NewRecordsState
2930
import com.onesignal.user.internal.subscriptions.SubscriptionModel
3031
import com.onesignal.user.internal.subscriptions.SubscriptionModelStore
3132
import com.onesignal.user.internal.subscriptions.SubscriptionType
@@ -37,6 +38,7 @@ internal class SubscriptionOperationExecutor(
3738
private val _subscriptionModelStore: SubscriptionModelStore,
3839
private val _configModelStore: ConfigModelStore,
3940
private val _buildUserService: IRebuildUserService,
41+
private val _newRecordState: NewRecordsState,
4042
) : IOperationExecutor {
4143
override val operations: List<String>
4244
get() = listOf(CREATE_SUBSCRIPTION, UPDATE_SUBSCRIPTION, DELETE_SUBSCRIPTION, TRANSFER_SUBSCRIPTION)
@@ -136,6 +138,9 @@ internal class SubscriptionOperationExecutor(
136138
NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
137139
ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
138140
NetworkUtils.ResponseStatusType.MISSING -> {
141+
if (_newRecordState.isInMissingRetryWindow(createOperation.onesignalId)) {
142+
return ExecutionResponse(ExecutionResult.FAIL_RETRY)
143+
}
139144
val operations = _buildUserService.getRebuildOperationsIfCurrentUser(createOperation.appId, createOperation.onesignalId)
140145
if (operations == null) {
141146
return ExecutionResponse(ExecutionResult.FAIL_NORETRY)
@@ -177,23 +182,27 @@ internal class SubscriptionOperationExecutor(
177182
return when (responseType) {
178183
NetworkUtils.ResponseStatusType.RETRYABLE ->
179184
ExecutionResponse(ExecutionResult.FAIL_RETRY)
180-
NetworkUtils.ResponseStatusType.MISSING ->
185+
NetworkUtils.ResponseStatusType.MISSING -> {
186+
if (listOf(lastOperation.onesignalId, lastOperation.subscriptionId).any { _newRecordState.isInMissingRetryWindow(it) }) {
187+
return ExecutionResponse(ExecutionResult.FAIL_RETRY)
188+
}
181189
// toss this, but create an identical CreateSubscriptionOperation to re-create the subscription being updated.
182190
ExecutionResponse(
183191
ExecutionResult.FAIL_NORETRY,
184192
operations =
185-
listOf(
186-
CreateSubscriptionOperation(
187-
lastOperation.appId,
188-
lastOperation.onesignalId,
189-
lastOperation.subscriptionId,
190-
lastOperation.type,
191-
lastOperation.enabled,
192-
lastOperation.address,
193-
lastOperation.status,
194-
),
193+
listOf(
194+
CreateSubscriptionOperation(
195+
lastOperation.appId,
196+
lastOperation.onesignalId,
197+
lastOperation.subscriptionId,
198+
lastOperation.type,
199+
lastOperation.enabled,
200+
lastOperation.address,
201+
lastOperation.status,
195202
),
203+
),
196204
)
205+
}
197206
else ->
198207
ExecutionResponse(ExecutionResult.FAIL_NORETRY)
199208
}
@@ -248,9 +257,14 @@ internal class SubscriptionOperationExecutor(
248257
val responseType = NetworkUtils.getResponseStatusType(ex.statusCode)
249258

250259
return when (responseType) {
251-
NetworkUtils.ResponseStatusType.MISSING ->
252-
// if the subscription is missing, we are good!
253-
ExecutionResponse(ExecutionResult.SUCCESS)
260+
NetworkUtils.ResponseStatusType.MISSING -> {
261+
if (listOf(op.onesignalId, op.subscriptionId).any { _newRecordState.isInMissingRetryWindow(it) }) {
262+
ExecutionResponse(ExecutionResult.FAIL_RETRY)
263+
} else {
264+
// if the subscription is missing, we are good!
265+
ExecutionResponse(ExecutionResult.SUCCESS)
266+
}
267+
}
254268
NetworkUtils.ResponseStatusType.RETRYABLE ->
255269
ExecutionResponse(ExecutionResult.FAIL_RETRY)
256270
else ->

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/UpdateUserOperationExecutor.kt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@ import com.onesignal.user.internal.operations.SetTagOperation
2222
import com.onesignal.user.internal.operations.TrackPurchaseOperation
2323
import com.onesignal.user.internal.operations.TrackSessionEndOperation
2424
import com.onesignal.user.internal.operations.TrackSessionStartOperation
25+
import com.onesignal.user.internal.operations.impl.states.NewRecordsState
2526
import com.onesignal.user.internal.properties.PropertiesModelStore
2627

2728
internal class UpdateUserOperationExecutor(
2829
private val _userBackend: IUserBackendService,
2930
private val _identityModelStore: IdentityModelStore,
3031
private val _propertiesModelStore: PropertiesModelStore,
3132
private val _buildUserService: IRebuildUserService,
33+
private val _newRecordState: NewRecordsState,
3234
) : IOperationExecutor {
3335
override val operations: List<String>
3436
get() = listOf(SET_TAG, DELETE_TAG, SET_PROPERTY, TRACK_SESSION_START, TRACK_SESSION_END, TRACK_PURCHASE)
@@ -163,6 +165,9 @@ internal class UpdateUserOperationExecutor(
163165
NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
164166
ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
165167
NetworkUtils.ResponseStatusType.MISSING -> {
168+
if (_newRecordState.isInMissingRetryWindow(onesignalId)) {
169+
return ExecutionResponse(ExecutionResult.FAIL_RETRY)
170+
}
166171
val operations = _buildUserService.getRebuildOperationsIfCurrentUser(appId, onesignalId)
167172
if (operations == null) {
168173
return ExecutionResponse(ExecutionResult.FAIL_NORETRY)

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/states/NewRecordsState.kt‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import com.onesignal.core.internal.time.ITime
66
/**
77
* Purpose: Keeps track of ids that were just created on the backend.
88
* This list gets used to delay network calls to ensure upcoming
9-
* requests are ready to be accepted by the backend.
9+
* requests are ready to be accepted by the backend. Also used for retries
10+
* as a fallback if the server is under extra load.
1011
*/
1112
class NewRecordsState(
1213
private val _time: ITime,
@@ -24,4 +25,9 @@ class NewRecordsState(
2425
val timeLastMovedOrCreated = records[key] ?: return true
2526
return _time.currentTimeMillis - timeLastMovedOrCreated > _configModelStore.model.opRepoPostCreateDelay
2627
}
28+
29+
fun isInMissingRetryWindow(key: String): Boolean {
30+
val timeLastMovedOrCreated = records[key] ?: return false
31+
return _time.currentTimeMillis - timeLastMovedOrCreated < _configModelStore.model.opRepoPostCreateRetryUpTo
32+
}
2733
}

0 commit comments

Comments
 (0)