Skip to content

Commit 6a6349e

Browse files
jkasten2jkasten2
authored
Merge pull request #2064 from OneSignal/improve/handle-http-response-header-retry-after
[Improve] Handle HTTP header Retry-After from responses from OneSignal
2 parents 4c8652b + 0a8684b commit 6a6349e

File tree

28 files changed

+342
-96
lines changed

28 files changed

+342
-96
lines changed

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/NetworkUtils.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ object NetworkUtils {
1717
401, 403 -> ResponseStatusType.UNAUTHORIZED
1818
404, 410 -> ResponseStatusType.MISSING
1919
409 -> ResponseStatusType.CONFLICT
20+
429 -> ResponseStatusType.RETRYABLE
2021
else -> ResponseStatusType.RETRYABLE
2122
}
2223
}

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/exceptions/BackendException.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,9 @@ class BackendException(
1212
* The response, if one exists.
1313
*/
1414
val response: String? = null,
15+
/**
16+
* Optional Integer value maybe returned from the backend.
17+
* The module handing this should delay any future requests by this time.
18+
*/
19+
val retryAfterSeconds: Int? = null,
1520
) : Exception()

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ internal class ParamsBackendService(
3434
val response = _http.get(paramsUrl, CacheKeys.REMOTE_PARAMS)
3535

3636
if (!response.isSuccess) {
37-
throw BackendException(response.statusCode, response.payload)
37+
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
3838
}
3939

4040
val responseJson = JSONObject(response.payload!!)

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,16 @@ class ConfigModel : Model() {
111111
setIntProperty(::httpGetTimeout.name, value)
112112
}
113113

114+
/**
115+
* The fallback Retry-After to use if the header is present, but the server
116+
* give us a format we can't parse.
117+
*/
118+
var httpRetryAfterParseFailFallback: Int
119+
get() = getIntProperty(::httpRetryAfterParseFailFallback.name) { 60 }
120+
set(value) {
121+
setIntProperty(::httpRetryAfterParseFailFallback.name, value)
122+
}
123+
114124
/**
115125
* Maximum time in milliseconds a user can spend out of focus before a new session is created.
116126
*/
@@ -167,6 +177,18 @@ class ConfigModel : Model() {
167177
setLongProperty(::opRepoPostCreateRetryUpTo.name, value)
168178
}
169179

180+
/**
181+
* The number of milliseconds times the number of times FAIL_RETRY
182+
* is returned from an executor for a specific operation. AKA this
183+
* backoff will increase each time we retry a specific operation
184+
* by this value.
185+
*/
186+
var opRepoDefaultFailRetryBackoff: Long
187+
get() = getLongProperty(::opRepoDefaultFailRetryBackoff.name) { 15_000 }
188+
set(value) {
189+
setLongProperty(::opRepoDefaultFailRetryBackoff.name, value)
190+
}
191+
170192
/**
171193
* The minimum number of milliseconds required to pass to allow the fetching of IAM to occur.
172194
*/

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/HttpResponse.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ class HttpResponse(
1818
* When non-null, the throwable that was thrown during processing.
1919
*/
2020
val throwable: Throwable? = null,
21+
/**
22+
* Optional Integer value maybe returned from the backend.
23+
* The module handing this should delay any future requests by this time.
24+
*/
25+
val retryAfterSeconds: Int? = null,
2126
) {
2227
/**
2328
* Whether the response is a successful one.

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

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@ import com.onesignal.core.internal.http.IHttpClient
1111
import com.onesignal.core.internal.preferences.IPreferencesService
1212
import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys
1313
import com.onesignal.core.internal.preferences.PreferenceStores
14+
import com.onesignal.core.internal.time.ITime
1415
import com.onesignal.debug.internal.logging.Logging
1516
import kotlinx.coroutines.DelicateCoroutinesApi
1617
import kotlinx.coroutines.Dispatchers
1718
import kotlinx.coroutines.GlobalScope
1819
import kotlinx.coroutines.TimeoutCancellationException
20+
import kotlinx.coroutines.delay
1921
import kotlinx.coroutines.launch
2022
import kotlinx.coroutines.withTimeout
2123
import org.json.JSONObject
@@ -29,7 +31,14 @@ internal class HttpClient(
2931
private val _connectionFactory: IHttpConnectionFactory,
3032
private val _prefs: IPreferencesService,
3133
private val _configModelStore: ConfigModelStore,
34+
private val _time: ITime,
3235
) : IHttpClient {
36+
/**
37+
* Delay making network requests until we reach this time.
38+
* Used when the OneSignal backend returns a Retry-After value.
39+
*/
40+
private var delayNewRequestsUntil = 0L
41+
3342
override suspend fun post(
3443
url: String,
3544
body: JSONObject,
@@ -77,6 +86,9 @@ internal class HttpClient(
7786
return HttpResponse(0, null, null)
7887
}
7988

89+
val delayUntil = delayNewRequestsUntil - _time.currentTimeMillis
90+
if (delayUntil > 0) delay(delayUntil)
91+
8092
try {
8193
return withTimeout(getThreadTimeout(timeout).toLong()) {
8294
return@withTimeout makeRequestIODispatcher(url, method, jsonBody, timeout, cacheKey)
@@ -171,6 +183,10 @@ internal class HttpClient(
171183
// Network request is made from getResponseCode()
172184
httpResponse = con.responseCode
173185

186+
val retryAfter = retryAfterFromResponse(con)
187+
val newDelayUntil = _time.currentTimeMillis + (retryAfter ?: 0) * 1_000
188+
if (newDelayUntil > delayNewRequestsUntil) delayNewRequestsUntil = newDelayUntil
189+
174190
when (httpResponse) {
175191
HttpURLConnection.HTTP_NOT_MODIFIED -> {
176192
val cachedResponse =
@@ -181,7 +197,7 @@ internal class HttpClient(
181197
Logging.debug("HttpClient: ${method ?: "GET"} $url - Using Cached response due to 304: " + cachedResponse)
182198

183199
// TODO: SHOULD RETURN OK INSTEAD OF NOT_MODIFIED TO MAKE TRANSPARENT?
184-
retVal = HttpResponse(httpResponse, cachedResponse)
200+
retVal = HttpResponse(httpResponse, cachedResponse, retryAfterSeconds = retryAfter)
185201
}
186202
HttpURLConnection.HTTP_ACCEPTED, HttpURLConnection.HTTP_CREATED, HttpURLConnection.HTTP_OK -> {
187203
val inputStream = con.inputStream
@@ -208,7 +224,7 @@ internal class HttpClient(
208224
}
209225
}
210226

211-
retVal = HttpResponse(httpResponse, json)
227+
retVal = HttpResponse(httpResponse, json, retryAfterSeconds = retryAfter)
212228
}
213229
else -> {
214230
Logging.debug("HttpClient: ${method ?: "GET"} $url - FAILED STATUS: $httpResponse")
@@ -229,7 +245,7 @@ internal class HttpClient(
229245
Logging.warn("HttpClient: $method HTTP Code: $httpResponse No response body!")
230246
}
231247

232-
retVal = HttpResponse(httpResponse, jsonResponse)
248+
retVal = HttpResponse(httpResponse, jsonResponse, retryAfterSeconds = retryAfter)
233249
}
234250
}
235251
} catch (t: Throwable) {
@@ -253,6 +269,22 @@ internal class HttpClient(
253269
return timeout + 5000
254270
}
255271

272+
/**
273+
* Reads the HTTP Retry-After from the response.
274+
* Only supports number format, not the date format.
275+
*/
276+
private fun retryAfterFromResponse(con: HttpURLConnection): Int? {
277+
val retryAfterStr = con.getHeaderField("Retry-After")
278+
return if (retryAfterStr != null) {
279+
Logging.debug("HttpClient: Response Retry-After: $retryAfterStr")
280+
retryAfterStr.toIntOrNull() ?: _configModelStore.model.httpRetryAfterParseFailFallback
281+
} else if (con.responseCode == 429) {
282+
_configModelStore.model.httpRetryAfterParseFailFallback
283+
} else {
284+
null
285+
}
286+
}
287+
256288
companion object {
257289
private const val OS_API_VERSION = "1"
258290
private const val OS_ACCEPT_HEADER = "application/vnd.onesignal.v$OS_API_VERSION+json"

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/IOperationExecutor.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ class ExecutionResponse(
3737
* When specified, any operations that should be prepended to the operation repo.
3838
*/
3939
val operations: List<Operation>? = null,
40+
/**
41+
* Optional Integer value maybe returned from the backend.
42+
* The module handing this should delay any future requests by this time.
43+
*/
44+
val retryAfterSeconds: Int? = null,
4045
)
4146

4247
enum class ExecutionResult {

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import kotlinx.coroutines.launch
1818
import kotlinx.coroutines.newSingleThreadContext
1919
import kotlinx.coroutines.withTimeoutOrNull
2020
import java.util.UUID
21+
import kotlin.math.max
2122
import kotlin.reflect.KClass
2223

2324
internal class OperationRepo(
@@ -221,6 +222,7 @@ internal class OperationRepo(
221222
}
222223
}
223224

225+
var highestRetries = 0
224226
when (response.result) {
225227
ExecutionResult.SUCCESS -> {
226228
// on success we remove the operation from the store and wake any waiters
@@ -248,7 +250,6 @@ internal class OperationRepo(
248250
ExecutionResult.FAIL_RETRY -> {
249251
Logging.error("Operation execution failed, retrying: $operations")
250252
// add back all operations to the front of the queue to be re-executed.
251-
var highestRetries = 0
252253
synchronized(queue) {
253254
ops.reversed().forEach {
254255
if (++it.retries > highestRetries) {
@@ -257,7 +258,6 @@ internal class OperationRepo(
257258
queue.add(0, it)
258259
}
259260
}
260-
delayBeforeRetry(highestRetries)
261261
}
262262
ExecutionResult.FAIL_PAUSE_OPREPO -> {
263263
Logging.error("Operation execution failed with eventual retry, pausing the operation repo: $operations")
@@ -282,6 +282,8 @@ internal class OperationRepo(
282282
}
283283
}
284284
}
285+
286+
delayBeforeNextExecution(highestRetries, response.retryAfterSeconds)
285287
} catch (e: Throwable) {
286288
Logging.log(LogLevel.ERROR, "Error attempting to execute operation: $ops", e)
287289

@@ -291,8 +293,18 @@ internal class OperationRepo(
291293
}
292294
}
293295

294-
suspend fun delayBeforeRetry(retries: Int) {
295-
val delayFor = retries * 15_000L
296+
/**
297+
* Wait which ever is longer, retryAfterSeconds returned by the server,
298+
* or based on the retry count.
299+
*/
300+
suspend fun delayBeforeNextExecution(
301+
retries: Int,
302+
retryAfterSeconds: Int?,
303+
) {
304+
Logging.debug("retryAfterSeconds: $retryAfterSeconds")
305+
val retryAfterSecondsNonNull = retryAfterSeconds?.toLong() ?: 0L
306+
val delayForOnRetries = retries * _configModelStore.model.opRepoDefaultFailRetryBackoff
307+
val delayFor = max(delayForOnRetries, retryAfterSecondsNonNull * 1_000)
296308
if (delayFor < 1) return
297309
Logging.error("Operations being delay for: $delayFor ms")
298310
delay(delayFor)

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsBackendService.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ internal class OutcomeEventsBackendService(private val _http: IHttpClient) :
4949
val response = _http.post("outcomes/measure", jsonObject)
5050

5151
if (!response.isSuccess) {
52-
throw BackendException(response.statusCode, response.payload)
52+
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
5353
}
5454
}
5555
}

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/backend/impl/IdentityBackendService.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ internal class IdentityBackendService(
2323
val response = _httpClient.patch("apps/$appId/users/by/$aliasLabel/$aliasValue/identity", requestJSONObject)
2424

2525
if (!response.isSuccess) {
26-
throw BackendException(response.statusCode, response.payload)
26+
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
2727
}
2828

2929
val responseJSON = JSONObject(response.payload!!)
@@ -40,7 +40,7 @@ internal class IdentityBackendService(
4040
val response = _httpClient.delete("apps/$appId/users/by/$aliasLabel/$aliasValue/identity/$aliasLabelToDelete")
4141

4242
if (!response.isSuccess) {
43-
throw BackendException(response.statusCode, response.payload)
43+
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
4444
}
4545
}
4646
}

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/backend/impl/SubscriptionBackendService.kt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ internal class SubscriptionBackendService(
2424
val response = _httpClient.post("apps/$appId/users/by/$aliasLabel/$aliasValue/subscriptions", requestJSON)
2525

2626
if (!response.isSuccess) {
27-
throw BackendException(response.statusCode, response.payload)
27+
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
2828
}
2929

3030
val responseJSON = JSONObject(response.payload!!)
@@ -48,7 +48,7 @@ internal class SubscriptionBackendService(
4848
val response = _httpClient.patch("apps/$appId/subscriptions/$subscriptionId", requestJSON)
4949

5050
if (!response.isSuccess) {
51-
throw BackendException(response.statusCode, response.payload)
51+
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
5252
}
5353
}
5454

@@ -59,7 +59,7 @@ internal class SubscriptionBackendService(
5959
val response = _httpClient.delete("apps/$appId/subscriptions/$subscriptionId")
6060

6161
if (!response.isSuccess) {
62-
throw BackendException(response.statusCode, response.payload)
62+
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
6363
}
6464
}
6565

@@ -76,7 +76,7 @@ internal class SubscriptionBackendService(
7676
val response = _httpClient.patch("apps/$appId/subscriptions/$subscriptionId/owner", requestJSON)
7777

7878
if (!response.isSuccess) {
79-
throw BackendException(response.statusCode, response.payload)
79+
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
8080
}
8181
}
8282

@@ -87,7 +87,7 @@ internal class SubscriptionBackendService(
8787
val response = _httpClient.get("apps/$appId/subscriptions/$subscriptionId/user/identity")
8888

8989
if (!response.isSuccess) {
90-
throw BackendException(response.statusCode, response.payload)
90+
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
9191
}
9292

9393
val responseJSON = JSONObject(response.payload!!)

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/backend/impl/UserBackendService.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ internal class UserBackendService(
3939
val response = _httpClient.post("apps/$appId/users", requestJSON)
4040

4141
if (!response.isSuccess) {
42-
throw BackendException(response.statusCode, response.payload)
42+
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
4343
}
4444

4545
return JSONConverter.convertToCreateUserResponse(JSONObject(response.payload!!))
@@ -68,7 +68,7 @@ internal class UserBackendService(
6868
val response = _httpClient.patch("apps/$appId/users/by/$aliasLabel/$aliasValue", jsonObject)
6969

7070
if (!response.isSuccess) {
71-
throw BackendException(response.statusCode, response.payload)
71+
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
7272
}
7373
}
7474

@@ -80,7 +80,7 @@ internal class UserBackendService(
8080
val response = _httpClient.get("apps/$appId/users/by/$aliasLabel/$aliasValue")
8181

8282
if (!response.isSuccess) {
83-
throw BackendException(response.statusCode, response.payload)
83+
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
8484
}
8585

8686
return JSONConverter.convertToCreateUserResponse(JSONObject(response.payload))

0 commit comments

Comments
 (0)