From abda35b34f3cd014762ba70a17d7afa19f026637 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Tue, 28 Oct 2025 21:08:14 +0700 Subject: [PATCH 01/20] chore: implement retryOnException --- .../android/internal/remote/ApiClientImpl.kt | 123 +++++++++++++----- 1 file changed, 87 insertions(+), 36 deletions(-) diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt index 894cb597..5acbeb29 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt @@ -2,6 +2,7 @@ package io.bucketeer.sdk.android.internal.remote import com.squareup.moshi.JsonAdapter import com.squareup.moshi.Moshi +import io.bucketeer.sdk.android.BKTException import io.bucketeer.sdk.android.internal.logd import io.bucketeer.sdk.android.internal.model.Event import io.bucketeer.sdk.android.internal.model.SourceId @@ -20,6 +21,9 @@ import okhttp3.Request import okhttp3.RequestBody import okhttp3.RequestBody.Companion.toRequestBody import okhttp3.Response +import java.util.concurrent.Executors +import java.util.concurrent.Future +import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.TimeUnit internal const val DEFAULT_REQUEST_TIMEOUT_MILLIS: Long = 30_000 @@ -49,11 +53,36 @@ internal class ApiClientImpl( moshi.adapter(ErrorResponse::class.java) } + private val getEvaluationExecutor = Executors.newSingleThreadScheduledExecutor() override fun getEvaluations( user: User, userEvaluationsId: String, timeoutMillis: Long?, condition: UserEvaluationCondition, + ): GetEvaluationsResult { + return retryOnException( + executor = getEvaluationExecutor, + maxRetries = 3, + delayMillis = 1000, + exceptionCheck = { e -> + val bktException = e as? BKTException + bktException is BKTException.ClientClosedRequestException + }, + ) { + getEvaluationsInternal( + user = user, + userEvaluationsId = userEvaluationsId, + timeoutMillis = timeoutMillis, + condition = condition, + ) + }.get() + } + + private fun getEvaluationsInternal( + user: User, + userEvaluationsId: String, + timeoutMillis: Long?, + condition: UserEvaluationCondition, ): GetEvaluationsResult { val body = GetEvaluationsRequest( @@ -91,50 +120,46 @@ internal class ApiClientImpl( } var responseStatusCode = 0 - val result = - actualClient.newCall(request).runCatching { - logd { "--> Fetch Evaluation\n$body" } + try { + val call = actualClient.newCall(request) - val (millis, data) = - measureTimeMillisWithResult { - val rawResponse = execute() - responseStatusCode = rawResponse.code + logd { "--> Fetch Evaluation\n$body" } - if (!rawResponse.isSuccessful) { - throw rawResponse.toBKTException(errorResponseJsonAdapter) - } + val (millis, data) = + measureTimeMillisWithResult { + val rawResponse = call.execute() + responseStatusCode = rawResponse.code - val response = - requireNotNull(rawResponse.fromJson()) { "failed to parse GetEvaluationsResponse" } - - response to (rawResponse.body?.contentLength() ?: -1).toInt() + if (!rawResponse.isSuccessful) { + throw rawResponse.toBKTException(errorResponseJsonAdapter) } - val (response, contentLength) = data + val response = + requireNotNull(rawResponse.fromJson()) { "failed to parse GetEvaluationsResponse" } - logd { "--> END Fetch Evaluation" } - logd { "<-- Fetch Evaluation\n$response\n<-- END Evaluation response" } + response to (rawResponse.body?.contentLength() ?: -1).toInt() + } - GetEvaluationsResult.Success( - value = response, - seconds = millis / 1000.0, - sizeByte = contentLength, - featureTag = featureTag, - ) - } + val (response, contentLength) = data - return result.fold( - onSuccess = { res -> res }, - onFailure = { e -> - GetEvaluationsResult.Failure( - e.toBKTException( - requestTimeoutMillis = client.callTimeoutMillis.toLong(), - statusCode = responseStatusCode, - ), - featureTag, - ) - }, - ) + logd { "--> END Fetch Evaluation" } + logd { "<-- Fetch Evaluation\n$response\n<-- END Evaluation response" } + + return GetEvaluationsResult.Success( + value = response, + seconds = millis / 1000.0, + sizeByte = contentLength, + featureTag = featureTag, + ) + } catch (e: Exception) { + return GetEvaluationsResult.Failure( + e.toBKTException( + requestTimeoutMillis = client.callTimeoutMillis.toLong(), + statusCode = responseStatusCode, + ), + featureTag, + ) + } } override fun registerEvents(events: List): RegisterEventsResult { @@ -222,3 +247,29 @@ private class FixJsonContentTypeInterceptor : Interceptor { return chain.proceed(fixed) } } + +fun retryOnException( + executor: ScheduledExecutorService, + maxRetries: Int, + delayMillis: Long = 1000, + exceptionCheck: (Throwable) -> Boolean, + block: () -> T, +): Future { + return executor.submit { + var lastException: Throwable? = null + repeat(maxRetries + 1) { attempt -> + try { + return@submit block() + } catch (e: Throwable) { + lastException = e + if (!exceptionCheck(e) || attempt >= maxRetries) { + throw e + } + Thread.sleep(delayMillis * (attempt + 1)) + } + } + // Tell the compiler that this function can make sure to return T or throw + // This code below is never reached + throw lastException!! + } +} From 8838f6363d44a1810cbed261cb9af618d1ddc715 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Tue, 28 Oct 2025 21:24:25 +0700 Subject: [PATCH 02/20] fix: lint fail --- .../bucketeer/sdk/android/internal/remote/ApiClientImpl.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt index 5acbeb29..fe36427a 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt @@ -54,13 +54,14 @@ internal class ApiClientImpl( } private val getEvaluationExecutor = Executors.newSingleThreadScheduledExecutor() + override fun getEvaluations( user: User, userEvaluationsId: String, timeoutMillis: Long?, condition: UserEvaluationCondition, - ): GetEvaluationsResult { - return retryOnException( + ): GetEvaluationsResult = + retryOnException( executor = getEvaluationExecutor, maxRetries = 3, delayMillis = 1000, @@ -76,7 +77,6 @@ internal class ApiClientImpl( condition = condition, ) }.get() - } private fun getEvaluationsInternal( user: User, From f06b1ae4f1d8b257be0fafcf893a97fb34e7006a Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Fri, 31 Oct 2025 14:52:19 +0700 Subject: [PATCH 03/20] feat: retry on 499 when register events --- .../android/internal/remote/ApiClientImpl.kt | 21 ++++++++++++++++--- .../internal/remote/ApiClientImplTest.kt | 15 +++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt index fe36427a..185290c7 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt @@ -162,7 +162,22 @@ internal class ApiClientImpl( } } - override fun registerEvents(events: List): RegisterEventsResult { + override fun registerEvents(events: List): RegisterEventsResult = + retryOnException( + executor = getEvaluationExecutor, + maxRetries = 3, + delayMillis = 1000, + exceptionCheck = { e -> + val bktException = e as? BKTException + bktException is BKTException.ClientClosedRequestException + }, + ) { + registerEventsInternal( + events + ) + }.get() + + private fun registerEventsInternal(events: List): RegisterEventsResult { val body = RegisterEventsRequest(events = events, sourceId = sourceId, sdkVersion = sdkVersion) val request = @@ -248,9 +263,9 @@ private class FixJsonContentTypeInterceptor : Interceptor { } } -fun retryOnException( +internal fun retryOnException( executor: ScheduledExecutorService, - maxRetries: Int, + maxRetries: Int = 3, delayMillis: Long = 1000, exceptionCheck: (Throwable) -> Boolean, block: () -> T, diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt index a6e0977f..bf946784 100644 --- a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt @@ -756,4 +756,19 @@ internal class ApiClientImplTest { assertThat(error.statusCode).isEqualTo(case.code) } } + + @Test() + fun `should be retry when got 499 status code at least 3 times before throw error`() { + + } + + @Test() + fun `should not be retry when got 3xx, 4xx, 5xx error`(){ + + } + + @Test() + fun `should stop retry when got other status code != 499`(){ + + } } From cb847554e3df5559d639b2f36080077a50059e66 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Fri, 31 Oct 2025 17:07:52 +0700 Subject: [PATCH 04/20] fix: lint fail --- .../bucketeer/sdk/android/internal/remote/ApiClientImpl.kt | 2 +- .../sdk/android/internal/remote/ApiClientImplTest.kt | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt index 185290c7..6ee7d6ab 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt @@ -173,7 +173,7 @@ internal class ApiClientImpl( }, ) { registerEventsInternal( - events + events, ) }.get() diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt index bf946784..c224fc13 100644 --- a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt @@ -759,16 +759,13 @@ internal class ApiClientImplTest { @Test() fun `should be retry when got 499 status code at least 3 times before throw error`() { - } @Test() - fun `should not be retry when got 3xx, 4xx, 5xx error`(){ - + fun `should not be retry when got 3xx, 4xx, 5xx error`() { } @Test() - fun `should stop retry when got other status code != 499`(){ - + fun `should stop retry when got other status code != 499`() { } } From 97890ddd8d352b4e7d1d31e3cf25cafe2e91f528 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Fri, 31 Oct 2025 18:05:03 +0700 Subject: [PATCH 05/20] Refactor retryOnException and update API client usage Moved retryOnException to a separate file and refactored it to return the result directly instead of a Future. Updated ApiClientImpl to use the new retryOnException implementation and simplified the getEvaluations and registerEvents methods accordingly. --- .../android/internal/remote/ApiClientImpl.kt | 153 ++++++------------ .../internal/remote/RetryOnException.kt | 45 ++++++ 2 files changed, 96 insertions(+), 102 deletions(-) create mode 100644 bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt index 6ee7d6ab..97a0b18c 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt @@ -2,7 +2,6 @@ package io.bucketeer.sdk.android.internal.remote import com.squareup.moshi.JsonAdapter import com.squareup.moshi.Moshi -import io.bucketeer.sdk.android.BKTException import io.bucketeer.sdk.android.internal.logd import io.bucketeer.sdk.android.internal.model.Event import io.bucketeer.sdk.android.internal.model.SourceId @@ -22,8 +21,6 @@ import okhttp3.RequestBody import okhttp3.RequestBody.Companion.toRequestBody import okhttp3.Response import java.util.concurrent.Executors -import java.util.concurrent.Future -import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.TimeUnit internal const val DEFAULT_REQUEST_TIMEOUT_MILLIS: Long = 30_000 @@ -54,35 +51,13 @@ internal class ApiClientImpl( } private val getEvaluationExecutor = Executors.newSingleThreadScheduledExecutor() + private val registerExecutor = Executors.newSingleThreadScheduledExecutor() override fun getEvaluations( user: User, userEvaluationsId: String, timeoutMillis: Long?, condition: UserEvaluationCondition, - ): GetEvaluationsResult = - retryOnException( - executor = getEvaluationExecutor, - maxRetries = 3, - delayMillis = 1000, - exceptionCheck = { e -> - val bktException = e as? BKTException - bktException is BKTException.ClientClosedRequestException - }, - ) { - getEvaluationsInternal( - user = user, - userEvaluationsId = userEvaluationsId, - timeoutMillis = timeoutMillis, - condition = condition, - ) - }.get() - - private fun getEvaluationsInternal( - user: User, - userEvaluationsId: String, - timeoutMillis: Long?, - condition: UserEvaluationCondition, ): GetEvaluationsResult { val body = GetEvaluationsRequest( @@ -120,64 +95,64 @@ internal class ApiClientImpl( } var responseStatusCode = 0 - try { - val call = actualClient.newCall(request) - - logd { "--> Fetch Evaluation\n$body" } - - val (millis, data) = - measureTimeMillisWithResult { - val rawResponse = call.execute() - responseStatusCode = rawResponse.code - - if (!rawResponse.isSuccessful) { - throw rawResponse.toBKTException(errorResponseJsonAdapter) + val result = runCatching { + retryOnException( + executor = getEvaluationExecutor, + maxRetries = 0, + delayMillis = 1000L, + exceptionCheck = { +// it is BKTException.ClientClosedRequestException + false + }, + ) { + val call = + actualClient.newCall(request) + logd { "--> Fetch Evaluation\n$body" } + + val (millis, data) = + measureTimeMillisWithResult { + val rawResponse = call.execute() + responseStatusCode = rawResponse.code + + if (!rawResponse.isSuccessful) { + throw rawResponse.toBKTException(errorResponseJsonAdapter) + } + + val response = + requireNotNull(rawResponse.fromJson()) { "failed to parse GetEvaluationsResponse" } + + response to (rawResponse.body?.contentLength() ?: -1).toInt() } - val response = - requireNotNull(rawResponse.fromJson()) { "failed to parse GetEvaluationsResponse" } - - response to (rawResponse.body?.contentLength() ?: -1).toInt() - } - - val (response, contentLength) = data + val (response, contentLength) = data - logd { "--> END Fetch Evaluation" } - logd { "<-- Fetch Evaluation\n$response\n<-- END Evaluation response" } + logd { "--> END Fetch Evaluation" } + logd { "<-- Fetch Evaluation\n$response\n<-- END Evaluation response" } - return GetEvaluationsResult.Success( - value = response, - seconds = millis / 1000.0, - sizeByte = contentLength, - featureTag = featureTag, - ) - } catch (e: Exception) { - return GetEvaluationsResult.Failure( - e.toBKTException( - requestTimeoutMillis = client.callTimeoutMillis.toLong(), - statusCode = responseStatusCode, - ), - featureTag, - ) + GetEvaluationsResult.Success( + value = response, + seconds = millis / 1000.0, + sizeByte = contentLength, + featureTag = featureTag, + ) + } } - } - override fun registerEvents(events: List): RegisterEventsResult = - retryOnException( - executor = getEvaluationExecutor, - maxRetries = 3, - delayMillis = 1000, - exceptionCheck = { e -> - val bktException = e as? BKTException - bktException is BKTException.ClientClosedRequestException + return result.fold( + onSuccess = { res -> res }, + onFailure = { e -> + GetEvaluationsResult.Failure( + e.toBKTException( + requestTimeoutMillis = client.callTimeoutMillis.toLong(), + statusCode = responseStatusCode, + ), + featureTag, + ) }, - ) { - registerEventsInternal( - events, - ) - }.get() + ) + } - private fun registerEventsInternal(events: List): RegisterEventsResult { + override fun registerEvents(events: List): RegisterEventsResult { val body = RegisterEventsRequest(events = events, sourceId = sourceId, sdkVersion = sdkVersion) val request = @@ -262,29 +237,3 @@ private class FixJsonContentTypeInterceptor : Interceptor { return chain.proceed(fixed) } } - -internal fun retryOnException( - executor: ScheduledExecutorService, - maxRetries: Int = 3, - delayMillis: Long = 1000, - exceptionCheck: (Throwable) -> Boolean, - block: () -> T, -): Future { - return executor.submit { - var lastException: Throwable? = null - repeat(maxRetries + 1) { attempt -> - try { - return@submit block() - } catch (e: Throwable) { - lastException = e - if (!exceptionCheck(e) || attempt >= maxRetries) { - throw e - } - Thread.sleep(delayMillis * (attempt + 1)) - } - } - // Tell the compiler that this function can make sure to return T or throw - // This code below is never reached - throw lastException!! - } -} diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt new file mode 100644 index 00000000..3cc90d5c --- /dev/null +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt @@ -0,0 +1,45 @@ +package io.bucketeer.sdk.android.internal.remote + +import java.util.concurrent.Callable +import java.util.concurrent.ExecutionException +import java.util.concurrent.Future +import java.util.concurrent.FutureTask +import java.util.concurrent.ScheduledExecutorService + +internal fun retryOnException( + executor: ScheduledExecutorService, + maxRetries: Int = 3, + delayMillis: Long = 1000, + exceptionCheck: (Throwable) -> Boolean, + block: () -> T, +): T { + val futureTask = object : FutureTask( + Callable { + var lastException: Throwable? = null + repeat(maxRetries + 1) { attempt -> + try { + return@Callable block() + } catch (e: Throwable) { + lastException = e + if (!exceptionCheck(e) || attempt >= maxRetries) { + throw e + } + Thread.sleep(delayMillis * (attempt + 1)) + } + } + throw lastException!! + } + ) {} + + executor.execute(futureTask) + return futureTask.getOrThrow() +} + +// Unwrap ExecutionException and throw the cause directly +fun Future.getOrThrow(): T { + return try { + get() + } catch (e: ExecutionException) { + throw e.cause ?: e + } +} From c95b2235d174c95a831554a95a4402f5a263c87b Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Fri, 31 Oct 2025 21:55:30 +0700 Subject: [PATCH 06/20] feat: use retry on register event --- .../android/internal/remote/ApiClientImpl.kt | 116 ++++++++++-------- .../internal/remote/RetryOnException.kt | 36 +++--- 2 files changed, 81 insertions(+), 71 deletions(-) diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt index 97a0b18c..a8e15604 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt @@ -51,7 +51,7 @@ internal class ApiClientImpl( } private val getEvaluationExecutor = Executors.newSingleThreadScheduledExecutor() - private val registerExecutor = Executors.newSingleThreadScheduledExecutor() + private val registerEventExecutor = Executors.newSingleThreadScheduledExecutor() override fun getEvaluations( user: User, @@ -95,48 +95,48 @@ internal class ApiClientImpl( } var responseStatusCode = 0 - val result = runCatching { - retryOnException( - executor = getEvaluationExecutor, - maxRetries = 0, - delayMillis = 1000L, - exceptionCheck = { -// it is BKTException.ClientClosedRequestException - false - }, - ) { - val call = - actualClient.newCall(request) - logd { "--> Fetch Evaluation\n$body" } - - val (millis, data) = - measureTimeMillisWithResult { - val rawResponse = call.execute() - responseStatusCode = rawResponse.code - - if (!rawResponse.isSuccessful) { - throw rawResponse.toBKTException(errorResponseJsonAdapter) + val result = + runCatching { + retryOnException( + executor = getEvaluationExecutor, + maxRetries = 0, + delayMillis = 1000L, + exceptionCheck = { + false + }, + ) { + val call = + actualClient.newCall(request) + logd { "--> Fetch Evaluation\n$body" } + + val (millis, data) = + measureTimeMillisWithResult { + val rawResponse = call.execute() + responseStatusCode = rawResponse.code + + if (!rawResponse.isSuccessful) { + throw rawResponse.toBKTException(errorResponseJsonAdapter) + } + + val response = + requireNotNull(rawResponse.fromJson()) { "failed to parse GetEvaluationsResponse" } + + response to (rawResponse.body?.contentLength() ?: -1).toInt() } - val response = - requireNotNull(rawResponse.fromJson()) { "failed to parse GetEvaluationsResponse" } - - response to (rawResponse.body?.contentLength() ?: -1).toInt() - } + val (response, contentLength) = data - val (response, contentLength) = data + logd { "--> END Fetch Evaluation" } + logd { "<-- Fetch Evaluation\n$response\n<-- END Evaluation response" } - logd { "--> END Fetch Evaluation" } - logd { "<-- Fetch Evaluation\n$response\n<-- END Evaluation response" } - - GetEvaluationsResult.Success( - value = response, - seconds = millis / 1000.0, - sizeByte = contentLength, - featureTag = featureTag, - ) + GetEvaluationsResult.Success( + value = response, + seconds = millis / 1000.0, + sizeByte = contentLength, + featureTag = featureTag, + ) + } } - } return result.fold( onSuccess = { res -> res }, @@ -169,24 +169,34 @@ internal class ApiClientImpl( var responseStatusCode = 0 val result = - client.newCall(request).runCatching { - logd { "--> Register events\n$body" } - val response = execute() - responseStatusCode = response.code - - if (!response.isSuccessful) { - val e = response.toBKTException(errorResponseJsonAdapter) - logd(throwable = e) { "<-- Register events error" } - throw e - } + runCatching { + retryOnException( + executor = registerEventExecutor, + maxRetries = 0, + delayMillis = 1000L, + exceptionCheck = { + false + }, + ) { + val call = client.newCall(request) + logd { "--> Register events\n$body" } + val response = call.execute() + responseStatusCode = response.code + + if (!response.isSuccessful) { + val e = response.toBKTException(errorResponseJsonAdapter) + logd(throwable = e) { "<-- Register events error" } + throw e + } - val result = - requireNotNull(response.fromJson()) { "failed to parse RegisterEventsResponse" } + val result = + requireNotNull(response.fromJson()) { "failed to parse RegisterEventsResponse" } - logd { "--> END Register events" } - logd { "<-- Register events\n$result\n<-- END Register events" } + logd { "--> END Register events" } + logd { "<-- Register events\n$result\n<-- END Register events" } - RegisterEventsResult.Success(value = result) + RegisterEventsResult.Success(value = result) + } } return result.fold( diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt index 3cc90d5c..99ba38de 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt @@ -13,33 +13,33 @@ internal fun retryOnException( exceptionCheck: (Throwable) -> Boolean, block: () -> T, ): T { - val futureTask = object : FutureTask( - Callable { - var lastException: Throwable? = null - repeat(maxRetries + 1) { attempt -> - try { - return@Callable block() - } catch (e: Throwable) { - lastException = e - if (!exceptionCheck(e) || attempt >= maxRetries) { - throw e + val futureTask = + object : FutureTask( + Callable { + var lastException: Throwable? = null + repeat(maxRetries + 1) { attempt -> + try { + return@Callable block() + } catch (e: Throwable) { + lastException = e + if (!exceptionCheck(e) || attempt >= maxRetries) { + throw e + } + Thread.sleep(delayMillis * (attempt + 1)) } - Thread.sleep(delayMillis * (attempt + 1)) } - } - throw lastException!! - } - ) {} + throw lastException!! + }, + ) {} executor.execute(futureTask) return futureTask.getOrThrow() } // Unwrap ExecutionException and throw the cause directly -fun Future.getOrThrow(): T { - return try { +fun Future.getOrThrow(): T = + try { get() } catch (e: ExecutionException) { throw e.cause ?: e } -} From 67978f2a271ddd6131df23e76f776321a73932cf Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Mon, 3 Nov 2025 10:36:53 +0700 Subject: [PATCH 07/20] test: add test-case for retry logic --- .../android/internal/remote/ApiClientImpl.kt | 9 +- .../bucketeer/sdk/android/RetryOnException.kt | 192 ++++++++++++++++++ 2 files changed, 197 insertions(+), 4 deletions(-) create mode 100644 bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt index a8e15604..1e26fd59 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt @@ -2,6 +2,7 @@ package io.bucketeer.sdk.android.internal.remote import com.squareup.moshi.JsonAdapter import com.squareup.moshi.Moshi +import io.bucketeer.sdk.android.BKTException import io.bucketeer.sdk.android.internal.logd import io.bucketeer.sdk.android.internal.model.Event import io.bucketeer.sdk.android.internal.model.SourceId @@ -99,10 +100,10 @@ internal class ApiClientImpl( runCatching { retryOnException( executor = getEvaluationExecutor, - maxRetries = 0, + maxRetries = 3, delayMillis = 1000L, - exceptionCheck = { - false + exceptionCheck = { ex -> + ex is BKTException.ClientClosedRequestException }, ) { val call = @@ -174,7 +175,7 @@ internal class ApiClientImpl( executor = registerEventExecutor, maxRetries = 0, delayMillis = 1000L, - exceptionCheck = { + exceptionCheck = { ex -> false }, ) { diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt new file mode 100644 index 00000000..78396d4e --- /dev/null +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt @@ -0,0 +1,192 @@ +package io.bucketeer.sdk.android + +import com.google.common.truth.Truth.assertThat +import io.bucketeer.sdk.android.internal.remote.retryOnException +import org.junit.Assert.assertThrows +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.util.concurrent.Executors +import java.util.concurrent.ScheduledExecutorService + +@RunWith(RobolectricTestRunner::class) +class RetryOnExceptionTest { + + private val executor: ScheduledExecutorService = Executors.newSingleThreadScheduledExecutor() + + @Test + fun `success on first attempt`() { + var attempts = 0 + val result = retryOnException( + executor = executor, + maxRetries = 3, + delayMillis = 10, + exceptionCheck = { true }, + ) { + attempts++ + "success" + } + + assertThat(result).isEqualTo("success") + assertThat(attempts).isEqualTo(1) + } + + @Test + fun `success after retries`() { + var attempts = 0 + val result = retryOnException( + executor = executor, + maxRetries = 3, + delayMillis = 10, + exceptionCheck = { true }, + ) { + attempts++ + if (attempts < 3) throw RuntimeException("fail") else "success" + } + + assertThat(result).isEqualTo("success") + assertThat(attempts).isEqualTo(3) + } + + @Test + fun `failure after max retries`() { + var attempts = 0 + val exception = assertThrows(RuntimeException::class.java) { + retryOnException( + executor = executor, + maxRetries = 2, + delayMillis = 10, + exceptionCheck = { true }, + ) { + attempts++ + throw RuntimeException("fail") + } + } + + assertThat(exception.message).isEqualTo("fail") + assertThat(attempts).isEqualTo(3) // 1 initial + 2 retries + } + + @Test + fun `no retry on non-retriable exception`() { + var attempts = 0 + val exception = assertThrows(IllegalArgumentException::class.java) { + retryOnException( + executor = executor, + maxRetries = 3, + delayMillis = 10, + exceptionCheck = { it !is IllegalArgumentException }, + ) { + attempts++ + throw IllegalArgumentException("non-retriable") + } + } + + assertThat(exception.message).isEqualTo("non-retriable") + assertThat(attempts).isEqualTo(1) + } + + @Test + fun `retry stops on first success after failures`() { + var attempts = 0 + val result = retryOnException( + executor = executor, + maxRetries = 5, + delayMillis = 10, + exceptionCheck = { true }, + ) { + attempts++ + if (attempts < 2) throw RuntimeException("fail") else "success" + } + + assertThat(result).isEqualTo("success") + assertThat(attempts).isEqualTo(2) + } + + @Test + fun `returns correct value type`() { + val intResult = retryOnException( + executor = executor, + maxRetries = 1, + delayMillis = 10, + exceptionCheck = { true }, + ) { + 42 + } + + assertThat(intResult).isEqualTo(42) + + val listResult = retryOnException( + executor = executor, + maxRetries = 1, + delayMillis = 10, + exceptionCheck = { true }, + ) { + listOf("a", "b", "c") + } + + assertThat(listResult).containsExactly("a", "b", "c") + } + + @Test + fun `exception check with specific exception types`() { + var attempts = 0 + val exception = assertThrows(IllegalStateException::class.java) { + retryOnException( + executor = executor, + maxRetries = 3, + delayMillis = 10, + exceptionCheck = { it is RuntimeException && it !is IllegalStateException }, + ) { + attempts++ + throw IllegalStateException("not retriable") + } + } + + assertThat(exception.message).isEqualTo("not retriable") + assertThat(attempts).isEqualTo(1) + } + + @Test + fun `mixed exceptions - retry on retriable then fail on non-retriable`() { + var attempts = 0 + val exception = assertThrows(IllegalArgumentException::class.java) { + retryOnException( + executor = executor, + maxRetries = 5, + delayMillis = 10, + exceptionCheck = { it is RuntimeException && it !is IllegalArgumentException }, + ) { + attempts++ + if (attempts < 3) { + throw RuntimeException("retriable") + } else { + throw IllegalArgumentException("non-retriable") + } + } + } + + assertThat(exception.message).isEqualTo("non-retriable") + assertThat(attempts).isEqualTo(3) + } + + @Test + fun `zero maxRetries means one attempt only`() { + var attempts = 0 + val exception = assertThrows(RuntimeException::class.java) { + retryOnException( + executor = executor, + maxRetries = 0, + delayMillis = 10, + exceptionCheck = { true }, + ) { + attempts++ + throw RuntimeException("fail") + } + } + + assertThat(exception.message).isEqualTo("fail") + assertThat(attempts).isEqualTo(1) + } +} + From 64b85dfccba79ef6f89253dad331281536c87e55 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Tue, 4 Nov 2025 20:50:02 +0700 Subject: [PATCH 08/20] fix: can not retry --- .../android/internal/remote/ApiClientImpl.kt | 132 ++++++++-------- .../internal/remote/RetryOnException.kt | 58 ++++--- .../bucketeer/sdk/android/RetryOnException.kt | 39 ++--- .../internal/remote/ApiClientImplErrorTest.kt | 143 ++++++++++++++++++ 4 files changed, 257 insertions(+), 115 deletions(-) create mode 100644 bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplErrorTest.kt diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt index 1e26fd59..c0fae7ab 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt @@ -21,7 +21,6 @@ import okhttp3.Request import okhttp3.RequestBody import okhttp3.RequestBody.Companion.toRequestBody import okhttp3.Response -import java.util.concurrent.Executors import java.util.concurrent.TimeUnit internal const val DEFAULT_REQUEST_TIMEOUT_MILLIS: Long = 30_000 @@ -50,15 +49,35 @@ internal class ApiClientImpl( private val errorResponseJsonAdapter: JsonAdapter by lazy { moshi.adapter(ErrorResponse::class.java) } - - private val getEvaluationExecutor = Executors.newSingleThreadScheduledExecutor() - private val registerEventExecutor = Executors.newSingleThreadScheduledExecutor() - override fun getEvaluations( user: User, userEvaluationsId: String, timeoutMillis: Long?, condition: UserEvaluationCondition, + ): GetEvaluationsResult = retryOnExceptionSync( + maxRetries = 3, + delayMillis = 1000L, + exceptionCheck = { it is BKTException.ClientClosedRequestException }, + block = { + val result = + getEvaluationsInternal( + user = user, + userEvaluationsId = userEvaluationsId, + timeoutMillis = timeoutMillis, + condition = condition, + ) + if (result is GetEvaluationsResult.Failure) { + throw result.error + } + return@retryOnExceptionSync result + }, + ) + + fun getEvaluationsInternal( + user: User, + userEvaluationsId: String, + timeoutMillis: Long?, + condition: UserEvaluationCondition, ): GetEvaluationsResult { val body = GetEvaluationsRequest( @@ -94,49 +113,38 @@ internal class ApiClientImpl( .writeTimeout(timeoutMillis, TimeUnit.MILLISECONDS) .build() } - + actualClient.connectionPool.evictAll() var responseStatusCode = 0 val result = - runCatching { - retryOnException( - executor = getEvaluationExecutor, - maxRetries = 3, - delayMillis = 1000L, - exceptionCheck = { ex -> - ex is BKTException.ClientClosedRequestException - }, - ) { - val call = - actualClient.newCall(request) - logd { "--> Fetch Evaluation\n$body" } - - val (millis, data) = - measureTimeMillisWithResult { - val rawResponse = call.execute() - responseStatusCode = rawResponse.code - - if (!rawResponse.isSuccessful) { - throw rawResponse.toBKTException(errorResponseJsonAdapter) - } - - val response = - requireNotNull(rawResponse.fromJson()) { "failed to parse GetEvaluationsResponse" } - - response to (rawResponse.body?.contentLength() ?: -1).toInt() + actualClient.newCall(request).runCatching { + logd { "--> Fetch Evaluation\n$body" } + + val (millis, data) = + measureTimeMillisWithResult { + val rawResponse = execute() + responseStatusCode = rawResponse.code + + if (!rawResponse.isSuccessful) { + throw rawResponse.toBKTException(errorResponseJsonAdapter) } - val (response, contentLength) = data + val response = + requireNotNull(rawResponse.fromJson()) { "failed to parse GetEvaluationsResponse" } - logd { "--> END Fetch Evaluation" } - logd { "<-- Fetch Evaluation\n$response\n<-- END Evaluation response" } + response to (rawResponse.body?.contentLength() ?: -1).toInt() + } - GetEvaluationsResult.Success( - value = response, - seconds = millis / 1000.0, - sizeByte = contentLength, - featureTag = featureTag, - ) - } + val (response, contentLength) = data + + logd { "--> END Fetch Evaluation" } + logd { "<-- Fetch Evaluation\n$response\n<-- END Evaluation response" } + + GetEvaluationsResult.Success( + value = response, + seconds = millis / 1000.0, + sizeByte = contentLength, + featureTag = featureTag, + ) } return result.fold( @@ -170,34 +178,24 @@ internal class ApiClientImpl( var responseStatusCode = 0 val result = - runCatching { - retryOnException( - executor = registerEventExecutor, - maxRetries = 0, - delayMillis = 1000L, - exceptionCheck = { ex -> - false - }, - ) { - val call = client.newCall(request) - logd { "--> Register events\n$body" } - val response = call.execute() - responseStatusCode = response.code - - if (!response.isSuccessful) { - val e = response.toBKTException(errorResponseJsonAdapter) - logd(throwable = e) { "<-- Register events error" } - throw e - } + client.newCall(request).runCatching { + logd { "--> Register events\n$body" } + val response = execute() + responseStatusCode = response.code + + if (!response.isSuccessful) { + val e = response.toBKTException(errorResponseJsonAdapter) + logd(throwable = e) { "<-- Register events error" } + throw e + } - val result = - requireNotNull(response.fromJson()) { "failed to parse RegisterEventsResponse" } + val result = + requireNotNull(response.fromJson()) { "failed to parse RegisterEventsResponse" } - logd { "--> END Register events" } - logd { "<-- Register events\n$result\n<-- END Register events" } + logd { "--> END Register events" } + logd { "<-- Register events\n$result\n<-- END Register events" } - RegisterEventsResult.Success(value = result) - } + RegisterEventsResult.Success(value = result) } return result.fold( diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt index 99ba38de..d8dcc8f3 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt @@ -1,9 +1,7 @@ package io.bucketeer.sdk.android.internal.remote -import java.util.concurrent.Callable import java.util.concurrent.ExecutionException import java.util.concurrent.Future -import java.util.concurrent.FutureTask import java.util.concurrent.ScheduledExecutorService internal fun retryOnException( @@ -13,27 +11,45 @@ internal fun retryOnException( exceptionCheck: (Throwable) -> Boolean, block: () -> T, ): T { - val futureTask = - object : FutureTask( - Callable { - var lastException: Throwable? = null - repeat(maxRetries + 1) { attempt -> - try { - return@Callable block() - } catch (e: Throwable) { - lastException = e - if (!exceptionCheck(e) || attempt >= maxRetries) { - throw e - } - Thread.sleep(delayMillis * (attempt + 1)) - } + return executor.submit { + var lastException: Throwable? = null + + for (attempt in 0..maxRetries) { + try { + return@submit block() + } catch (e: Throwable) { + lastException = e + if (!exceptionCheck(e) || attempt >= maxRetries) { + throw e } - throw lastException!! - }, - ) {} + // Sleep directly since we're already inside the executor task + Thread.sleep(delayMillis * (attempt + 1)) + } + } + throw lastException!! + }.getOrThrow() +} - executor.execute(futureTask) - return futureTask.getOrThrow() +internal fun retryOnExceptionSync( + maxRetries: Int = 3, + delayMillis: Long = 1000, + exceptionCheck: (Throwable) -> Boolean, + block: () -> T, +): T { + var lastException: Throwable? = null + + for (attempt in 0..maxRetries) { + try { + return block() + } catch (e: Throwable) { + lastException = e + if (!exceptionCheck(e) || attempt >= maxRetries) { + throw e + } + Thread.sleep(delayMillis * (attempt + 1)) + } + } + throw lastException!! } // Unwrap ExecutionException and throw the cause directly diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt index 78396d4e..ca0b051d 100644 --- a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt @@ -1,24 +1,18 @@ package io.bucketeer.sdk.android import com.google.common.truth.Truth.assertThat -import io.bucketeer.sdk.android.internal.remote.retryOnException +import io.bucketeer.sdk.android.internal.remote.retryOnExceptionSync import org.junit.Assert.assertThrows import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner -import java.util.concurrent.Executors -import java.util.concurrent.ScheduledExecutorService @RunWith(RobolectricTestRunner::class) -class RetryOnExceptionTest { - - private val executor: ScheduledExecutorService = Executors.newSingleThreadScheduledExecutor() - +class RetryOnExceptionSyncTest { @Test fun `success on first attempt`() { var attempts = 0 - val result = retryOnException( - executor = executor, + val result = retryOnExceptionSync( maxRetries = 3, delayMillis = 10, exceptionCheck = { true }, @@ -34,8 +28,7 @@ class RetryOnExceptionTest { @Test fun `success after retries`() { var attempts = 0 - val result = retryOnException( - executor = executor, + val result = retryOnExceptionSync( maxRetries = 3, delayMillis = 10, exceptionCheck = { true }, @@ -52,8 +45,7 @@ class RetryOnExceptionTest { fun `failure after max retries`() { var attempts = 0 val exception = assertThrows(RuntimeException::class.java) { - retryOnException( - executor = executor, + retryOnExceptionSync( maxRetries = 2, delayMillis = 10, exceptionCheck = { true }, @@ -71,8 +63,7 @@ class RetryOnExceptionTest { fun `no retry on non-retriable exception`() { var attempts = 0 val exception = assertThrows(IllegalArgumentException::class.java) { - retryOnException( - executor = executor, + retryOnExceptionSync( maxRetries = 3, delayMillis = 10, exceptionCheck = { it !is IllegalArgumentException }, @@ -89,8 +80,7 @@ class RetryOnExceptionTest { @Test fun `retry stops on first success after failures`() { var attempts = 0 - val result = retryOnException( - executor = executor, + val result = retryOnExceptionSync( maxRetries = 5, delayMillis = 10, exceptionCheck = { true }, @@ -105,8 +95,7 @@ class RetryOnExceptionTest { @Test fun `returns correct value type`() { - val intResult = retryOnException( - executor = executor, + val intResult = retryOnExceptionSync( maxRetries = 1, delayMillis = 10, exceptionCheck = { true }, @@ -116,8 +105,7 @@ class RetryOnExceptionTest { assertThat(intResult).isEqualTo(42) - val listResult = retryOnException( - executor = executor, + val listResult = retryOnExceptionSync( maxRetries = 1, delayMillis = 10, exceptionCheck = { true }, @@ -132,8 +120,7 @@ class RetryOnExceptionTest { fun `exception check with specific exception types`() { var attempts = 0 val exception = assertThrows(IllegalStateException::class.java) { - retryOnException( - executor = executor, + retryOnExceptionSync( maxRetries = 3, delayMillis = 10, exceptionCheck = { it is RuntimeException && it !is IllegalStateException }, @@ -151,8 +138,7 @@ class RetryOnExceptionTest { fun `mixed exceptions - retry on retriable then fail on non-retriable`() { var attempts = 0 val exception = assertThrows(IllegalArgumentException::class.java) { - retryOnException( - executor = executor, + retryOnExceptionSync( maxRetries = 5, delayMillis = 10, exceptionCheck = { it is RuntimeException && it !is IllegalArgumentException }, @@ -174,8 +160,7 @@ class RetryOnExceptionTest { fun `zero maxRetries means one attempt only`() { var attempts = 0 val exception = assertThrows(RuntimeException::class.java) { - retryOnException( - executor = executor, + retryOnExceptionSync( maxRetries = 0, delayMillis = 10, exceptionCheck = { true }, diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplErrorTest.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplErrorTest.kt new file mode 100644 index 00000000..c321f4bd --- /dev/null +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplErrorTest.kt @@ -0,0 +1,143 @@ +package io.bucketeer.sdk.android.internal.remote + +import com.google.common.truth.Truth.assertThat +import com.squareup.moshi.Moshi +import io.bucketeer.sdk.android.BuildConfig +import io.bucketeer.sdk.android.internal.di.DataModule +import io.bucketeer.sdk.android.internal.model.SourceId +import io.bucketeer.sdk.android.internal.model.response.ErrorResponse +import io.bucketeer.sdk.android.internal.model.response.GetEvaluationsResponse +import io.bucketeer.sdk.android.mocks.user1 +import io.bucketeer.sdk.android.mocks.user1Evaluations +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.util.concurrent.TimeUnit + +@RunWith(RobolectricTestRunner::class) +internal class ApiClientImplErrorTest { + private lateinit var server: MockWebServer + private lateinit var client: ApiClientImpl + private lateinit var apiEndpoint: String + private lateinit var moshi: Moshi + private lateinit var mockClientClosedRequestResponse: MockResponse + private lateinit var mockOtherErrorStatusResponse : MockResponse + private lateinit var mockSuccessResponse: MockResponse + + @Before + fun setup() { + server = MockWebServer() + apiEndpoint = server.url("").toString() + moshi = DataModule.createMoshi() + mockClientClosedRequestResponse = MockResponse() + .setResponseCode(499) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 499, + message = "client closed request", + ), + ), + ), + ) + mockOtherErrorStatusResponse = MockResponse() + .setResponseCode(530) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 530, + message = "custom server error", + ), + ), + ), + ) + mockSuccessResponse = MockResponse() + .setBodyDelay(1, TimeUnit.SECONDS) + .apply { + this.setBody( + moshi.adapter(GetEvaluationsResponse::class.java).toJson( + GetEvaluationsResponse( + evaluations = user1Evaluations, + userEvaluationsId = "user_evaluation_id", + ), + ), + ) + }.setResponseCode(200) + } + + @After + fun tearDown() { + server.shutdown() + } + /* + cases + 200 -> done, no retry, request count = 1 + 530 -> error, no retry, request count = 1 + 499, 530 -> error, retry once, request count = 2 + 499, 200 -> done okay, request count = 2 + 499, 499, 200 -> done okay, retry twice, request count = 3 + 499, 499, 499, 200 -> done okay, max retry reached, request count = 4 + 499, 499, 499, 530 -> error, max retry reached, request count = 4 + */ + + @Test + fun `getEvaluations - error with body`( + ) { + server.enqueue( + MockResponse() + .setResponseCode(499) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 499, + message = "client closed request", + ), + ), + ), + ), + ) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.getEvaluations( + user = user1, + userEvaluationsId = "user_evaluation_id", + condition = + UserEvaluationCondition( + evaluatedAt = "1690799200", + userAttributesUpdated = true, + ), + ) + + assertThat(result).isInstanceOf(GetEvaluationsResult.Failure::class.java) + val failure = result as GetEvaluationsResult.Failure + val error = failure.error + + assertThat(error).isInstanceOf(case.expectedClass) + if (case.expectedResponse != null) { + assertThat(error.message).contains(case.expectedResponse) + } + } +} From 8899e1bfd79345ac08362e4c36bf39329f455c52 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Tue, 4 Nov 2025 22:03:04 +0700 Subject: [PATCH 09/20] test: get evaluation --- .../android/internal/remote/ApiClientImpl.kt | 126 +++-- .../internal/remote/RetryOnException.kt | 44 +- .../internal/remote/ApiClientImplErrorTest.kt | 143 ------ .../internal/remote/ApiClientImplRetryTest.kt | 451 ++++++++++++++++++ 4 files changed, 515 insertions(+), 249 deletions(-) delete mode 100644 bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplErrorTest.kt create mode 100644 bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplRetryTest.kt diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt index c0fae7ab..39d57527 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt @@ -49,31 +49,8 @@ internal class ApiClientImpl( private val errorResponseJsonAdapter: JsonAdapter by lazy { moshi.adapter(ErrorResponse::class.java) } - override fun getEvaluations( - user: User, - userEvaluationsId: String, - timeoutMillis: Long?, - condition: UserEvaluationCondition, - ): GetEvaluationsResult = retryOnExceptionSync( - maxRetries = 3, - delayMillis = 1000L, - exceptionCheck = { it is BKTException.ClientClosedRequestException }, - block = { - val result = - getEvaluationsInternal( - user = user, - userEvaluationsId = userEvaluationsId, - timeoutMillis = timeoutMillis, - condition = condition, - ) - if (result is GetEvaluationsResult.Failure) { - throw result.error - } - return@retryOnExceptionSync result - }, - ) - fun getEvaluationsInternal( + override fun getEvaluations( user: User, userEvaluationsId: String, timeoutMillis: Long?, @@ -113,38 +90,48 @@ internal class ApiClientImpl( .writeTimeout(timeoutMillis, TimeUnit.MILLISECONDS) .build() } - actualClient.connectionPool.evictAll() + var responseStatusCode = 0 val result = - actualClient.newCall(request).runCatching { - logd { "--> Fetch Evaluation\n$body" } - - val (millis, data) = - measureTimeMillisWithResult { - val rawResponse = execute() - responseStatusCode = rawResponse.code - - if (!rawResponse.isSuccessful) { - throw rawResponse.toBKTException(errorResponseJsonAdapter) + runCatching { + retryOnExceptionSync( + maxRetries = 3, + delayMillis = 1000L, + exceptionCheck = { err -> + err is BKTException.ClientClosedRequestException + }, + ) { + val call = + actualClient.newCall(request) + logd { "--> Fetch Evaluation\n$body" } + + val (millis, data) = + measureTimeMillisWithResult { + val rawResponse = call.execute() + responseStatusCode = rawResponse.code + + if (!rawResponse.isSuccessful) { + throw rawResponse.toBKTException(errorResponseJsonAdapter) + } + + val response = + requireNotNull(rawResponse.fromJson()) { "failed to parse GetEvaluationsResponse" } + + response to (rawResponse.body?.contentLength() ?: -1).toInt() } - val response = - requireNotNull(rawResponse.fromJson()) { "failed to parse GetEvaluationsResponse" } - - response to (rawResponse.body?.contentLength() ?: -1).toInt() - } + val (response, contentLength) = data - val (response, contentLength) = data + logd { "--> END Fetch Evaluation" } + logd { "<-- Fetch Evaluation\n$response\n<-- END Evaluation response" } - logd { "--> END Fetch Evaluation" } - logd { "<-- Fetch Evaluation\n$response\n<-- END Evaluation response" } - - GetEvaluationsResult.Success( - value = response, - seconds = millis / 1000.0, - sizeByte = contentLength, - featureTag = featureTag, - ) + GetEvaluationsResult.Success( + value = response, + seconds = millis / 1000.0, + sizeByte = contentLength, + featureTag = featureTag, + ) + } } return result.fold( @@ -178,24 +165,33 @@ internal class ApiClientImpl( var responseStatusCode = 0 val result = - client.newCall(request).runCatching { - logd { "--> Register events\n$body" } - val response = execute() - responseStatusCode = response.code - - if (!response.isSuccessful) { - val e = response.toBKTException(errorResponseJsonAdapter) - logd(throwable = e) { "<-- Register events error" } - throw e - } + runCatching { + retryOnExceptionSync( + maxRetries = 3, + delayMillis = 1000L, + exceptionCheck = { err -> + err is BKTException.ClientClosedRequestException + }, + ) { + val call = client.newCall(request) + logd { "--> Register events\n$body" } + val response = call.execute() + responseStatusCode = response.code + + if (!response.isSuccessful) { + val e = response.toBKTException(errorResponseJsonAdapter) + logd(throwable = e) { "<-- Register events error" } + throw e + } - val result = - requireNotNull(response.fromJson()) { "failed to parse RegisterEventsResponse" } + val result = + requireNotNull(response.fromJson()) { "failed to parse RegisterEventsResponse" } - logd { "--> END Register events" } - logd { "<-- Register events\n$result\n<-- END Register events" } + logd { "--> END Register events" } + logd { "<-- Register events\n$result\n<-- END Register events" } - RegisterEventsResult.Success(value = result) + RegisterEventsResult.Success(value = result) + } } return result.fold( diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt index d8dcc8f3..d1d4472b 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt @@ -1,35 +1,5 @@ package io.bucketeer.sdk.android.internal.remote -import java.util.concurrent.ExecutionException -import java.util.concurrent.Future -import java.util.concurrent.ScheduledExecutorService - -internal fun retryOnException( - executor: ScheduledExecutorService, - maxRetries: Int = 3, - delayMillis: Long = 1000, - exceptionCheck: (Throwable) -> Boolean, - block: () -> T, -): T { - return executor.submit { - var lastException: Throwable? = null - - for (attempt in 0..maxRetries) { - try { - return@submit block() - } catch (e: Throwable) { - lastException = e - if (!exceptionCheck(e) || attempt >= maxRetries) { - throw e - } - // Sleep directly since we're already inside the executor task - Thread.sleep(delayMillis * (attempt + 1)) - } - } - throw lastException!! - }.getOrThrow() -} - internal fun retryOnExceptionSync( maxRetries: Int = 3, delayMillis: Long = 1000, @@ -37,13 +7,13 @@ internal fun retryOnExceptionSync( block: () -> T, ): T { var lastException: Throwable? = null - - for (attempt in 0..maxRetries) { + val maxAttempts = if (maxRetries < 0) 1 else maxRetries + 1 + for (attempt in 0..maxAttempts) { try { return block() } catch (e: Throwable) { lastException = e - if (!exceptionCheck(e) || attempt >= maxRetries) { + if (!exceptionCheck(e) || attempt >= maxAttempts) { throw e } Thread.sleep(delayMillis * (attempt + 1)) @@ -51,11 +21,3 @@ internal fun retryOnExceptionSync( } throw lastException!! } - -// Unwrap ExecutionException and throw the cause directly -fun Future.getOrThrow(): T = - try { - get() - } catch (e: ExecutionException) { - throw e.cause ?: e - } diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplErrorTest.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplErrorTest.kt deleted file mode 100644 index c321f4bd..00000000 --- a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplErrorTest.kt +++ /dev/null @@ -1,143 +0,0 @@ -package io.bucketeer.sdk.android.internal.remote - -import com.google.common.truth.Truth.assertThat -import com.squareup.moshi.Moshi -import io.bucketeer.sdk.android.BuildConfig -import io.bucketeer.sdk.android.internal.di.DataModule -import io.bucketeer.sdk.android.internal.model.SourceId -import io.bucketeer.sdk.android.internal.model.response.ErrorResponse -import io.bucketeer.sdk.android.internal.model.response.GetEvaluationsResponse -import io.bucketeer.sdk.android.mocks.user1 -import io.bucketeer.sdk.android.mocks.user1Evaluations -import okhttp3.mockwebserver.MockResponse -import okhttp3.mockwebserver.MockWebServer -import org.junit.After -import org.junit.Before -import org.junit.Test -import org.junit.runner.RunWith -import org.robolectric.RobolectricTestRunner -import java.util.concurrent.TimeUnit - -@RunWith(RobolectricTestRunner::class) -internal class ApiClientImplErrorTest { - private lateinit var server: MockWebServer - private lateinit var client: ApiClientImpl - private lateinit var apiEndpoint: String - private lateinit var moshi: Moshi - private lateinit var mockClientClosedRequestResponse: MockResponse - private lateinit var mockOtherErrorStatusResponse : MockResponse - private lateinit var mockSuccessResponse: MockResponse - - @Before - fun setup() { - server = MockWebServer() - apiEndpoint = server.url("").toString() - moshi = DataModule.createMoshi() - mockClientClosedRequestResponse = MockResponse() - .setResponseCode(499) - .setBody( - moshi - .adapter(ErrorResponse::class.java) - .toJson( - ErrorResponse( - ErrorResponse.ErrorDetail( - code = 499, - message = "client closed request", - ), - ), - ), - ) - mockOtherErrorStatusResponse = MockResponse() - .setResponseCode(530) - .setBody( - moshi - .adapter(ErrorResponse::class.java) - .toJson( - ErrorResponse( - ErrorResponse.ErrorDetail( - code = 530, - message = "custom server error", - ), - ), - ), - ) - mockSuccessResponse = MockResponse() - .setBodyDelay(1, TimeUnit.SECONDS) - .apply { - this.setBody( - moshi.adapter(GetEvaluationsResponse::class.java).toJson( - GetEvaluationsResponse( - evaluations = user1Evaluations, - userEvaluationsId = "user_evaluation_id", - ), - ), - ) - }.setResponseCode(200) - } - - @After - fun tearDown() { - server.shutdown() - } - /* - cases - 200 -> done, no retry, request count = 1 - 530 -> error, no retry, request count = 1 - 499, 530 -> error, retry once, request count = 2 - 499, 200 -> done okay, request count = 2 - 499, 499, 200 -> done okay, retry twice, request count = 3 - 499, 499, 499, 200 -> done okay, max retry reached, request count = 4 - 499, 499, 499, 530 -> error, max retry reached, request count = 4 - */ - - @Test - fun `getEvaluations - error with body`( - ) { - server.enqueue( - MockResponse() - .setResponseCode(499) - .setBody( - moshi - .adapter(ErrorResponse::class.java) - .toJson( - ErrorResponse( - ErrorResponse.ErrorDetail( - code = 499, - message = "client closed request", - ), - ), - ), - ), - ) - - client = - ApiClientImpl( - apiEndpoint = apiEndpoint, - apiKey = "api_key_value", - featureTag = "feature_tag_value", - moshi = moshi, - sourceId = SourceId.ANDROID, - sdkVersion = BuildConfig.SDK_VERSION, - ) - - val result = - client.getEvaluations( - user = user1, - userEvaluationsId = "user_evaluation_id", - condition = - UserEvaluationCondition( - evaluatedAt = "1690799200", - userAttributesUpdated = true, - ), - ) - - assertThat(result).isInstanceOf(GetEvaluationsResult.Failure::class.java) - val failure = result as GetEvaluationsResult.Failure - val error = failure.error - - assertThat(error).isInstanceOf(case.expectedClass) - if (case.expectedResponse != null) { - assertThat(error.message).contains(case.expectedResponse) - } - } -} diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplRetryTest.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplRetryTest.kt new file mode 100644 index 00000000..34f71e6f --- /dev/null +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplRetryTest.kt @@ -0,0 +1,451 @@ +package io.bucketeer.sdk.android.internal.remote + +import com.google.common.truth.Truth.assertThat +import com.squareup.moshi.Moshi +import io.bucketeer.sdk.android.BKTException +import io.bucketeer.sdk.android.BuildConfig +import io.bucketeer.sdk.android.internal.di.DataModule +import io.bucketeer.sdk.android.internal.model.SourceId +import io.bucketeer.sdk.android.internal.model.response.ErrorResponse +import io.bucketeer.sdk.android.internal.model.response.GetEvaluationsResponse +import io.bucketeer.sdk.android.mocks.user1 +import io.bucketeer.sdk.android.mocks.user1Evaluations +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.util.concurrent.TimeUnit + +@RunWith(RobolectricTestRunner::class) +internal class ApiClientImplRetryTest { + private lateinit var server: MockWebServer + private lateinit var client: ApiClientImpl + private lateinit var apiEndpoint: String + private lateinit var moshi: Moshi + private lateinit var mockClientClosedRequestResponse: MockResponse + private lateinit var mockOtherErrorStatusResponse: MockResponse + private lateinit var mockSuccessResponse: MockResponse + + @Before + fun setup() { + server = MockWebServer() + apiEndpoint = server.url("").toString() + moshi = DataModule.createMoshi() + mockClientClosedRequestResponse = MockResponse() + .setResponseCode(499) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 499, + message = "client closed request", + ), + ), + ), + ) + mockOtherErrorStatusResponse = MockResponse() + .setResponseCode(530) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 530, + message = "custom server error", + ), + ), + ), + ) + mockSuccessResponse = MockResponse() + .setBodyDelay(1, TimeUnit.SECONDS) + .apply { + this.setBody( + moshi.adapter(GetEvaluationsResponse::class.java).toJson( + GetEvaluationsResponse( + evaluations = user1Evaluations, + userEvaluationsId = "user_evaluation_id", + ), + ), + ) + }.setResponseCode(200) + } + + @After + fun tearDown() { + server.shutdown() + } + + /* + cases + 200 -> done, no retry, request count = 1 + 530 -> error, no retry, request count = 1 + 499, 530 -> error, retry once, request count = 2 + 499, 200 -> done okay, request count = 2 + 499, 499, 200 -> done okay, retry twice, request count = 3 + 499, 499, 499, 200 -> done okay, max retry reached, request count = 4 + 499, 499, 499, 530 -> error, max retry reached, request count = 4 + */ + + @Test + fun `getEvaluations - success - 200 no retry`() { + server.enqueue(mockSuccessResponse) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.getEvaluations( + user = user1, + userEvaluationsId = "user_evaluation_id", + condition = + UserEvaluationCondition( + evaluatedAt = "1690799200", + userAttributesUpdated = true, + ), + ) + + assertThat(result).isInstanceOf(GetEvaluationsResult.Success::class.java) + assertThat(server.requestCount).isEqualTo(1) + } + + @Test + fun `getEvaluations - error 530 no retry`() { + server.enqueue(mockOtherErrorStatusResponse) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.getEvaluations( + user = user1, + userEvaluationsId = "user_evaluation_id", + condition = + UserEvaluationCondition( + evaluatedAt = "1690799200", + userAttributesUpdated = true, + ), + ) + + assertThat(result).isInstanceOf(GetEvaluationsResult.Failure::class.java) + val failure = result as GetEvaluationsResult.Failure + assertThat(failure.error).isInstanceOf(BKTException.UnknownServerException::class.java) + assertThat(server.requestCount).isEqualTo(1) + } + + @Test + fun `getEvaluations - error 499 then 530 retry once and stop`() { + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockOtherErrorStatusResponse) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.getEvaluations( + user = user1, + userEvaluationsId = "user_evaluation_id", + condition = + UserEvaluationCondition( + evaluatedAt = "1690799200", + userAttributesUpdated = true, + ), + ) + + assertThat(result).isInstanceOf(GetEvaluationsResult.Failure::class.java) + val failure = result as GetEvaluationsResult.Failure + assertThat(failure.error).isInstanceOf(BKTException.UnknownServerException::class.java) + assertThat(server.requestCount).isEqualTo(2) + } + + @Test + fun `getEvaluations - error 499 then 200 success`() { + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockSuccessResponse) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.getEvaluations( + user = user1, + userEvaluationsId = "user_evaluation_id", + condition = + UserEvaluationCondition( + evaluatedAt = "1690799200", + userAttributesUpdated = true, + ), + ) + + assertThat(result).isInstanceOf(GetEvaluationsResult.Success::class.java) + assertThat(server.requestCount).isEqualTo(2) + } + + @Test + fun `getEvaluations - error 499 499 then 200 success with 2 retries`() { + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockSuccessResponse) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.getEvaluations( + user = user1, + userEvaluationsId = "user_evaluation_id", + condition = + UserEvaluationCondition( + evaluatedAt = "1690799200", + userAttributesUpdated = true, + ), + ) + + assertThat(result).isInstanceOf(GetEvaluationsResult.Success::class.java) + assertThat(server.requestCount).isEqualTo(3) + } + + @Test + fun `getEvaluations - error 499 499 499 then 200 success with max 3 retries`() { + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockSuccessResponse) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.getEvaluations( + user = user1, + userEvaluationsId = "user_evaluation_id", + condition = + UserEvaluationCondition( + evaluatedAt = "1690799200", + userAttributesUpdated = true, + ), + ) + + assertThat(result).isInstanceOf(GetEvaluationsResult.Success::class.java) + assertThat(server.requestCount).isEqualTo(4) + } + + @Test + fun `getEvaluations - error 499 499 499 then 530 max retry reached and fail`() { + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockOtherErrorStatusResponse) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.getEvaluations( + user = user1, + userEvaluationsId = "user_evaluation_id", + condition = + UserEvaluationCondition( + evaluatedAt = "1690799200", + userAttributesUpdated = true, + ), + ) + + assertThat(result).isInstanceOf(GetEvaluationsResult.Failure::class.java) + val failure = result as GetEvaluationsResult.Failure + assertThat(failure.error).isInstanceOf(BKTException.UnknownServerException::class.java) + assertThat(server.requestCount).isEqualTo(4) + } + + @Test + fun `should not retry when got 3xx error`() { + server.enqueue( + MockResponse() + .setResponseCode(301) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 301, + message = "redirect", + ), + ), + ), + ), + ) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.getEvaluations( + user = user1, + userEvaluationsId = "user_evaluation_id", + condition = + UserEvaluationCondition( + evaluatedAt = "1690799200", + userAttributesUpdated = true, + ), + ) + + assertThat(result).isInstanceOf(GetEvaluationsResult.Failure::class.java) + val failure = result as GetEvaluationsResult.Failure + assertThat(failure.error).isInstanceOf(BKTException.RedirectRequestException::class.java) + assertThat(server.requestCount).isEqualTo(1) + } + + @Test + fun `should not retry when got 4xx error except 499`() { + server.enqueue( + MockResponse() + .setResponseCode(400) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 400, + message = "bad request", + ), + ), + ), + ), + ) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.getEvaluations( + user = user1, + userEvaluationsId = "user_evaluation_id", + condition = + UserEvaluationCondition( + evaluatedAt = "1690799200", + userAttributesUpdated = true, + ), + ) + + assertThat(result).isInstanceOf(GetEvaluationsResult.Failure::class.java) + val failure = result as GetEvaluationsResult.Failure + assertThat(failure.error).isInstanceOf(BKTException.BadRequestException::class.java) + assertThat(server.requestCount).isEqualTo(1) + } + + @Test + fun `should not retry when got 5xx error`() { + server.enqueue( + MockResponse() + .setResponseCode(500) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 500, + message = "internal server error", + ), + ), + ), + ), + ) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.getEvaluations( + user = user1, + userEvaluationsId = "user_evaluation_id", + condition = + UserEvaluationCondition( + evaluatedAt = "1690799200", + userAttributesUpdated = true, + ), + ) + + assertThat(result).isInstanceOf(GetEvaluationsResult.Failure::class.java) + val failure = result as GetEvaluationsResult.Failure + assertThat(failure.error).isInstanceOf(BKTException.InternalServerErrorException::class.java) + assertThat(server.requestCount).isEqualTo(1) + } +} From f83e3f18d3d3abc5da05b6dcdce61e656319920d Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Tue, 4 Nov 2025 22:33:53 +0700 Subject: [PATCH 10/20] Add registerEvents retry and error handling tests Added comprehensive tests for the registerEvents API client method, covering success, various error codes, retry logic for 499 errors, and ensuring no retries for other 3xx, 4xx, and 5xx errors. This improves test coverage and verifies correct retry and error handling behavior. --- .../bucketeer/sdk/android/RetryOnException.kt | 183 ++++---- .../internal/remote/ApiClientImplRetryTest.kt | 391 ++++++++++++++++-- 2 files changed, 452 insertions(+), 122 deletions(-) diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt index ca0b051d..2beaefce 100644 --- a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt @@ -12,14 +12,15 @@ class RetryOnExceptionSyncTest { @Test fun `success on first attempt`() { var attempts = 0 - val result = retryOnExceptionSync( - maxRetries = 3, - delayMillis = 10, - exceptionCheck = { true }, - ) { - attempts++ - "success" - } + val result = + retryOnExceptionSync( + maxRetries = 3, + delayMillis = 10, + exceptionCheck = { true }, + ) { + attempts++ + "success" + } assertThat(result).isEqualTo("success") assertThat(attempts).isEqualTo(1) @@ -28,14 +29,15 @@ class RetryOnExceptionSyncTest { @Test fun `success after retries`() { var attempts = 0 - val result = retryOnExceptionSync( - maxRetries = 3, - delayMillis = 10, - exceptionCheck = { true }, - ) { - attempts++ - if (attempts < 3) throw RuntimeException("fail") else "success" - } + val result = + retryOnExceptionSync( + maxRetries = 3, + delayMillis = 10, + exceptionCheck = { true }, + ) { + attempts++ + if (attempts < 3) throw RuntimeException("fail") else "success" + } assertThat(result).isEqualTo("success") assertThat(attempts).isEqualTo(3) @@ -44,16 +46,17 @@ class RetryOnExceptionSyncTest { @Test fun `failure after max retries`() { var attempts = 0 - val exception = assertThrows(RuntimeException::class.java) { - retryOnExceptionSync( - maxRetries = 2, - delayMillis = 10, - exceptionCheck = { true }, - ) { - attempts++ - throw RuntimeException("fail") + val exception = + assertThrows(RuntimeException::class.java) { + retryOnExceptionSync( + maxRetries = 2, + delayMillis = 10, + exceptionCheck = { true }, + ) { + attempts++ + throw RuntimeException("fail") + } } - } assertThat(exception.message).isEqualTo("fail") assertThat(attempts).isEqualTo(3) // 1 initial + 2 retries @@ -62,16 +65,17 @@ class RetryOnExceptionSyncTest { @Test fun `no retry on non-retriable exception`() { var attempts = 0 - val exception = assertThrows(IllegalArgumentException::class.java) { - retryOnExceptionSync( - maxRetries = 3, - delayMillis = 10, - exceptionCheck = { it !is IllegalArgumentException }, - ) { - attempts++ - throw IllegalArgumentException("non-retriable") + val exception = + assertThrows(IllegalArgumentException::class.java) { + retryOnExceptionSync( + maxRetries = 3, + delayMillis = 10, + exceptionCheck = { it !is IllegalArgumentException }, + ) { + attempts++ + throw IllegalArgumentException("non-retriable") + } } - } assertThat(exception.message).isEqualTo("non-retriable") assertThat(attempts).isEqualTo(1) @@ -80,14 +84,15 @@ class RetryOnExceptionSyncTest { @Test fun `retry stops on first success after failures`() { var attempts = 0 - val result = retryOnExceptionSync( - maxRetries = 5, - delayMillis = 10, - exceptionCheck = { true }, - ) { - attempts++ - if (attempts < 2) throw RuntimeException("fail") else "success" - } + val result = + retryOnExceptionSync( + maxRetries = 5, + delayMillis = 10, + exceptionCheck = { true }, + ) { + attempts++ + if (attempts < 2) throw RuntimeException("fail") else "success" + } assertThat(result).isEqualTo("success") assertThat(attempts).isEqualTo(2) @@ -95,23 +100,25 @@ class RetryOnExceptionSyncTest { @Test fun `returns correct value type`() { - val intResult = retryOnExceptionSync( - maxRetries = 1, - delayMillis = 10, - exceptionCheck = { true }, - ) { - 42 - } + val intResult = + retryOnExceptionSync( + maxRetries = 1, + delayMillis = 10, + exceptionCheck = { true }, + ) { + 42 + } assertThat(intResult).isEqualTo(42) - val listResult = retryOnExceptionSync( - maxRetries = 1, - delayMillis = 10, - exceptionCheck = { true }, - ) { - listOf("a", "b", "c") - } + val listResult = + retryOnExceptionSync( + maxRetries = 1, + delayMillis = 10, + exceptionCheck = { true }, + ) { + listOf("a", "b", "c") + } assertThat(listResult).containsExactly("a", "b", "c") } @@ -119,16 +126,17 @@ class RetryOnExceptionSyncTest { @Test fun `exception check with specific exception types`() { var attempts = 0 - val exception = assertThrows(IllegalStateException::class.java) { - retryOnExceptionSync( - maxRetries = 3, - delayMillis = 10, - exceptionCheck = { it is RuntimeException && it !is IllegalStateException }, - ) { - attempts++ - throw IllegalStateException("not retriable") + val exception = + assertThrows(IllegalStateException::class.java) { + retryOnExceptionSync( + maxRetries = 3, + delayMillis = 10, + exceptionCheck = { it is RuntimeException && it !is IllegalStateException }, + ) { + attempts++ + throw IllegalStateException("not retriable") + } } - } assertThat(exception.message).isEqualTo("not retriable") assertThat(attempts).isEqualTo(1) @@ -137,20 +145,21 @@ class RetryOnExceptionSyncTest { @Test fun `mixed exceptions - retry on retriable then fail on non-retriable`() { var attempts = 0 - val exception = assertThrows(IllegalArgumentException::class.java) { - retryOnExceptionSync( - maxRetries = 5, - delayMillis = 10, - exceptionCheck = { it is RuntimeException && it !is IllegalArgumentException }, - ) { - attempts++ - if (attempts < 3) { - throw RuntimeException("retriable") - } else { - throw IllegalArgumentException("non-retriable") + val exception = + assertThrows(IllegalArgumentException::class.java) { + retryOnExceptionSync( + maxRetries = 5, + delayMillis = 10, + exceptionCheck = { it is RuntimeException && it !is IllegalArgumentException }, + ) { + attempts++ + if (attempts < 3) { + throw RuntimeException("retriable") + } else { + throw IllegalArgumentException("non-retriable") + } } } - } assertThat(exception.message).isEqualTo("non-retriable") assertThat(attempts).isEqualTo(3) @@ -159,19 +168,19 @@ class RetryOnExceptionSyncTest { @Test fun `zero maxRetries means one attempt only`() { var attempts = 0 - val exception = assertThrows(RuntimeException::class.java) { - retryOnExceptionSync( - maxRetries = 0, - delayMillis = 10, - exceptionCheck = { true }, - ) { - attempts++ - throw RuntimeException("fail") + val exception = + assertThrows(RuntimeException::class.java) { + retryOnExceptionSync( + maxRetries = 0, + delayMillis = 10, + exceptionCheck = { true }, + ) { + attempts++ + throw RuntimeException("fail") + } } - } assertThat(exception.message).isEqualTo("fail") assertThat(attempts).isEqualTo(1) } } - diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplRetryTest.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplRetryTest.kt index 34f71e6f..950d5c3a 100644 --- a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplRetryTest.kt +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplRetryTest.kt @@ -8,6 +8,8 @@ import io.bucketeer.sdk.android.internal.di.DataModule import io.bucketeer.sdk.android.internal.model.SourceId import io.bucketeer.sdk.android.internal.model.response.ErrorResponse import io.bucketeer.sdk.android.internal.model.response.GetEvaluationsResponse +import io.bucketeer.sdk.android.internal.model.response.RegisterEventsResponse +import io.bucketeer.sdk.android.mocks.evaluationEvent1 import io.bucketeer.sdk.android.mocks.user1 import io.bucketeer.sdk.android.mocks.user1Evaluations import okhttp3.mockwebserver.MockResponse @@ -34,46 +36,49 @@ internal class ApiClientImplRetryTest { server = MockWebServer() apiEndpoint = server.url("").toString() moshi = DataModule.createMoshi() - mockClientClosedRequestResponse = MockResponse() - .setResponseCode(499) - .setBody( - moshi - .adapter(ErrorResponse::class.java) - .toJson( - ErrorResponse( - ErrorResponse.ErrorDetail( - code = 499, - message = "client closed request", + mockClientClosedRequestResponse = + MockResponse() + .setResponseCode(499) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 499, + message = "client closed request", + ), ), ), - ), - ) - mockOtherErrorStatusResponse = MockResponse() - .setResponseCode(530) - .setBody( - moshi - .adapter(ErrorResponse::class.java) - .toJson( - ErrorResponse( - ErrorResponse.ErrorDetail( - code = 530, - message = "custom server error", + ) + mockOtherErrorStatusResponse = + MockResponse() + .setResponseCode(530) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 530, + message = "custom server error", + ), ), ), - ), - ) - mockSuccessResponse = MockResponse() - .setBodyDelay(1, TimeUnit.SECONDS) - .apply { - this.setBody( - moshi.adapter(GetEvaluationsResponse::class.java).toJson( - GetEvaluationsResponse( - evaluations = user1Evaluations, - userEvaluationsId = "user_evaluation_id", - ), - ), ) - }.setResponseCode(200) + mockSuccessResponse = + MockResponse() + .setBodyDelay(1, TimeUnit.SECONDS) + .apply { + this.setBody( + moshi.adapter(GetEvaluationsResponse::class.java).toJson( + GetEvaluationsResponse( + evaluations = user1Evaluations, + userEvaluationsId = "user_evaluation_id", + ), + ), + ) + }.setResponseCode(200) } @After @@ -448,4 +453,320 @@ internal class ApiClientImplRetryTest { assertThat(failure.error).isInstanceOf(BKTException.InternalServerErrorException::class.java) assertThat(server.requestCount).isEqualTo(1) } + + @Test + fun `registerEvents - success - 200 no retry`() { + server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody( + moshi.adapter(RegisterEventsResponse::class.java).toJson( + RegisterEventsResponse( + errors = emptyMap(), + ), + ), + ), + ) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = client.registerEvents(events = listOf(evaluationEvent1)) + + assertThat(result).isInstanceOf(RegisterEventsResult.Success::class.java) + assertThat(server.requestCount).isEqualTo(1) + } + + @Test + fun `registerEvents - error 530 no retry`() { + server.enqueue(mockOtherErrorStatusResponse) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = client.registerEvents(events = listOf(evaluationEvent1)) + + assertThat(result).isInstanceOf(RegisterEventsResult.Failure::class.java) + val failure = result as RegisterEventsResult.Failure + assertThat(failure.error).isInstanceOf(BKTException.UnknownServerException::class.java) + assertThat(server.requestCount).isEqualTo(1) + } + + @Test + fun `registerEvents - error 499 then 530 retry once and stop`() { + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockOtherErrorStatusResponse) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = client.registerEvents(events = listOf(evaluationEvent1)) + + assertThat(result).isInstanceOf(RegisterEventsResult.Failure::class.java) + val failure = result as RegisterEventsResult.Failure + assertThat(failure.error).isInstanceOf(BKTException.UnknownServerException::class.java) + assertThat(server.requestCount).isEqualTo(2) + } + + @Test + fun `registerEvents - error 499 then 200 success`() { + server.enqueue(mockClientClosedRequestResponse) + server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody( + moshi.adapter(RegisterEventsResponse::class.java).toJson( + RegisterEventsResponse( + errors = emptyMap(), + ), + ), + ), + ) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = client.registerEvents(events = listOf(evaluationEvent1)) + + assertThat(result).isInstanceOf(RegisterEventsResult.Success::class.java) + assertThat(server.requestCount).isEqualTo(2) + } + + @Test + fun `registerEvents - error 499 499 then 200 success with 2 retries`() { + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockClientClosedRequestResponse) + server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody( + moshi.adapter(RegisterEventsResponse::class.java).toJson( + RegisterEventsResponse( + errors = emptyMap(), + ), + ), + ), + ) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = client.registerEvents(events = listOf(evaluationEvent1)) + + assertThat(result).isInstanceOf(RegisterEventsResult.Success::class.java) + assertThat(server.requestCount).isEqualTo(3) + } + + @Test + fun `registerEvents - error 499 499 499 then 200 success with max 3 retries`() { + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockClientClosedRequestResponse) + server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody( + moshi.adapter(RegisterEventsResponse::class.java).toJson( + RegisterEventsResponse( + errors = emptyMap(), + ), + ), + ), + ) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = client.registerEvents(events = listOf(evaluationEvent1)) + + assertThat(result).isInstanceOf(RegisterEventsResult.Success::class.java) + assertThat(server.requestCount).isEqualTo(4) + } + + @Test + fun `registerEvents - error 499 499 499 then 530 max retry reached and fail`() { + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockClientClosedRequestResponse) + server.enqueue(mockOtherErrorStatusResponse) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = client.registerEvents(events = listOf(evaluationEvent1)) + + assertThat(result).isInstanceOf(RegisterEventsResult.Failure::class.java) + val failure = result as RegisterEventsResult.Failure + assertThat(failure.error).isInstanceOf(BKTException.UnknownServerException::class.java) + assertThat(server.requestCount).isEqualTo(4) + } + + @Test + fun `should not retry when got 3xx error for registerEvents`() { + server.enqueue( + MockResponse() + .setResponseCode(301) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 301, + message = "redirect", + ), + ), + ), + ), + ) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.registerEvents( + events = listOf(evaluationEvent1), + ) + + assertThat(result).isInstanceOf(RegisterEventsResult.Failure::class.java) + val failure = result as RegisterEventsResult.Failure + assertThat(failure.error).isInstanceOf(BKTException.RedirectRequestException::class.java) + assertThat(server.requestCount).isEqualTo(1) + } + + @Test + fun `should not retry when got 4xx error except 499 for registerEvents`() { + server.enqueue( + MockResponse() + .setResponseCode(400) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 400, + message = "bad request", + ), + ), + ), + ), + ) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.registerEvents( + events = listOf(evaluationEvent1), + ) + + assertThat(result).isInstanceOf(RegisterEventsResult.Failure::class.java) + val failure = result as RegisterEventsResult.Failure + assertThat(failure.error).isInstanceOf(BKTException.BadRequestException::class.java) + assertThat(server.requestCount).isEqualTo(1) + } + + @Test + fun `should not retry when got 5xx error for registerEvents`() { + server.enqueue( + MockResponse() + .setResponseCode(500) + .setBody( + moshi + .adapter(ErrorResponse::class.java) + .toJson( + ErrorResponse( + ErrorResponse.ErrorDetail( + code = 500, + message = "internal server error", + ), + ), + ), + ), + ) + + client = + ApiClientImpl( + apiEndpoint = apiEndpoint, + apiKey = "api_key_value", + featureTag = "feature_tag_value", + moshi = moshi, + sourceId = SourceId.ANDROID, + sdkVersion = BuildConfig.SDK_VERSION, + ) + + val result = + client.registerEvents( + events = listOf(evaluationEvent1), + ) + + assertThat(result).isInstanceOf(RegisterEventsResult.Failure::class.java) + val failure = result as RegisterEventsResult.Failure + assertThat(failure.error).isInstanceOf(BKTException.InternalServerErrorException::class.java) + assertThat(server.requestCount).isEqualTo(1) + } } From 419a20c150eea05b4770a632e7ebeeb9f402f126 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Wed, 5 Nov 2025 08:49:11 +0700 Subject: [PATCH 11/20] Rename and update retryOnException function and tests Renamed retryOnExceptionSync to retryOnException and updated all usages in implementation and tests. Adjusted retry logic to fix off-by-one error in maxAttempts. Renamed test file and test class to match the new function name. --- .../android/internal/remote/ApiClientImpl.kt | 4 ++-- .../internal/remote/RetryOnException.kt | 4 ++-- ...OnException.kt => RetryOnExceptionTest.kt} | 24 +++++++++---------- 3 files changed, 16 insertions(+), 16 deletions(-) rename bucketeer/src/test/kotlin/io/bucketeer/sdk/android/{RetryOnException.kt => RetryOnExceptionTest.kt} (92%) diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt index 39d57527..e3fcc107 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt @@ -94,7 +94,7 @@ internal class ApiClientImpl( var responseStatusCode = 0 val result = runCatching { - retryOnExceptionSync( + retryOnException( maxRetries = 3, delayMillis = 1000L, exceptionCheck = { err -> @@ -166,7 +166,7 @@ internal class ApiClientImpl( var responseStatusCode = 0 val result = runCatching { - retryOnExceptionSync( + retryOnException( maxRetries = 3, delayMillis = 1000L, exceptionCheck = { err -> diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt index d1d4472b..9eeb7bf9 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt @@ -1,6 +1,6 @@ package io.bucketeer.sdk.android.internal.remote -internal fun retryOnExceptionSync( +internal fun retryOnException( maxRetries: Int = 3, delayMillis: Long = 1000, exceptionCheck: (Throwable) -> Boolean, @@ -13,7 +13,7 @@ internal fun retryOnExceptionSync( return block() } catch (e: Throwable) { lastException = e - if (!exceptionCheck(e) || attempt >= maxAttempts) { + if (!exceptionCheck(e) || attempt >= maxAttempts - 1) { throw e } Thread.sleep(delayMillis * (attempt + 1)) diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnExceptionTest.kt similarity index 92% rename from bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt rename to bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnExceptionTest.kt index 2beaefce..ae62b950 100644 --- a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnException.kt +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/RetryOnExceptionTest.kt @@ -1,19 +1,19 @@ package io.bucketeer.sdk.android import com.google.common.truth.Truth.assertThat -import io.bucketeer.sdk.android.internal.remote.retryOnExceptionSync +import io.bucketeer.sdk.android.internal.remote.retryOnException import org.junit.Assert.assertThrows import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) -class RetryOnExceptionSyncTest { +class RetryOnExceptionTest { @Test fun `success on first attempt`() { var attempts = 0 val result = - retryOnExceptionSync( + retryOnException( maxRetries = 3, delayMillis = 10, exceptionCheck = { true }, @@ -30,7 +30,7 @@ class RetryOnExceptionSyncTest { fun `success after retries`() { var attempts = 0 val result = - retryOnExceptionSync( + retryOnException( maxRetries = 3, delayMillis = 10, exceptionCheck = { true }, @@ -48,7 +48,7 @@ class RetryOnExceptionSyncTest { var attempts = 0 val exception = assertThrows(RuntimeException::class.java) { - retryOnExceptionSync( + retryOnException( maxRetries = 2, delayMillis = 10, exceptionCheck = { true }, @@ -67,7 +67,7 @@ class RetryOnExceptionSyncTest { var attempts = 0 val exception = assertThrows(IllegalArgumentException::class.java) { - retryOnExceptionSync( + retryOnException( maxRetries = 3, delayMillis = 10, exceptionCheck = { it !is IllegalArgumentException }, @@ -85,7 +85,7 @@ class RetryOnExceptionSyncTest { fun `retry stops on first success after failures`() { var attempts = 0 val result = - retryOnExceptionSync( + retryOnException( maxRetries = 5, delayMillis = 10, exceptionCheck = { true }, @@ -101,7 +101,7 @@ class RetryOnExceptionSyncTest { @Test fun `returns correct value type`() { val intResult = - retryOnExceptionSync( + retryOnException( maxRetries = 1, delayMillis = 10, exceptionCheck = { true }, @@ -112,7 +112,7 @@ class RetryOnExceptionSyncTest { assertThat(intResult).isEqualTo(42) val listResult = - retryOnExceptionSync( + retryOnException( maxRetries = 1, delayMillis = 10, exceptionCheck = { true }, @@ -128,7 +128,7 @@ class RetryOnExceptionSyncTest { var attempts = 0 val exception = assertThrows(IllegalStateException::class.java) { - retryOnExceptionSync( + retryOnException( maxRetries = 3, delayMillis = 10, exceptionCheck = { it is RuntimeException && it !is IllegalStateException }, @@ -147,7 +147,7 @@ class RetryOnExceptionSyncTest { var attempts = 0 val exception = assertThrows(IllegalArgumentException::class.java) { - retryOnExceptionSync( + retryOnException( maxRetries = 5, delayMillis = 10, exceptionCheck = { it is RuntimeException && it !is IllegalArgumentException }, @@ -170,7 +170,7 @@ class RetryOnExceptionSyncTest { var attempts = 0 val exception = assertThrows(RuntimeException::class.java) { - retryOnExceptionSync( + retryOnException( maxRetries = 0, delayMillis = 10, exceptionCheck = { true }, From 678cea2b4fba0d62d9c70105e7bc0629a67e3e71 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Wed, 5 Nov 2025 09:17:47 +0700 Subject: [PATCH 12/20] Skip CLIENT_CLOSED_REQUEST test in ApiClientImplTest Commented out the CLIENT_CLOSED_REQUEST test case since it has special retry handling. The relevant test can be found in ApiClientRetryTest.kt. --- .../bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt index c224fc13..0969fe12 100644 --- a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt @@ -84,7 +84,8 @@ internal class ApiClientImplTest { expectedResponse = null, ), PAYLOAD_TOO_LARGE(413, BKTException.PayloadTooLargeException::class.java, "error: 413"), - CLIENT_CLOSED_REQUEST(499, BKTException.ClientClosedRequestException::class.java, "error: 499"), + //CLIENT_CLOSED_REQUEST has a special handling for retry, so we skip the test here, please check ApiClientRetryTest.kt for the test. + //CLIENT_CLOSED_REQUEST(499, BKTException.ClientClosedRequestException::class.java, "error: 499"), INTERNAL_SERVER_ERROR(500, BKTException.InternalServerErrorException::class.java, "error: 500"), SERVICE_UNAVAILABLE(503, BKTException.ServiceUnavailableException::class.java, "error: 503"), UNKNOWN_SERVER(418, BKTException.UnknownServerException::class.java, "UnknownServerException 418"), From 8ac2f3c8f4bf1f4eab92292e8800c65359268557 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Wed, 5 Nov 2025 09:19:46 +0700 Subject: [PATCH 13/20] Fix comment formatting in ApiClientImplTest Adjusted spacing in comments for consistency in ApiClientImplTest.kt. No functional changes were made. --- .../sdk/android/internal/remote/ApiClientImplTest.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt index 0969fe12..034cef8b 100644 --- a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt @@ -84,8 +84,9 @@ internal class ApiClientImplTest { expectedResponse = null, ), PAYLOAD_TOO_LARGE(413, BKTException.PayloadTooLargeException::class.java, "error: 413"), - //CLIENT_CLOSED_REQUEST has a special handling for retry, so we skip the test here, please check ApiClientRetryTest.kt for the test. - //CLIENT_CLOSED_REQUEST(499, BKTException.ClientClosedRequestException::class.java, "error: 499"), + + // CLIENT_CLOSED_REQUEST has a special handling for retry, so we skip the test here, please check ApiClientRetryTest.kt for the test. + // CLIENT_CLOSED_REQUEST(499, BKTException.ClientClosedRequestException::class.java, "error: 499"), INTERNAL_SERVER_ERROR(500, BKTException.InternalServerErrorException::class.java, "error: 500"), SERVICE_UNAVAILABLE(503, BKTException.ServiceUnavailableException::class.java, "error: 503"), UNKNOWN_SERVER(418, BKTException.UnknownServerException::class.java, "UnknownServerException 418"), From b2072d1a4a73c4a4ebf3eb14cfab34b98a424fe5 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Wed, 5 Nov 2025 09:44:29 +0700 Subject: [PATCH 14/20] Fix retry loop to use correct attempt range Changed the retry loop from '0..maxAttempts' to '0 until maxAttempts' to ensure the number of attempts matches the intended maxRetries logic and avoids an off-by-one error. --- .../bucketeer/sdk/android/internal/remote/RetryOnException.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt index 9eeb7bf9..38f029e3 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt @@ -8,7 +8,7 @@ internal fun retryOnException( ): T { var lastException: Throwable? = null val maxAttempts = if (maxRetries < 0) 1 else maxRetries + 1 - for (attempt in 0..maxAttempts) { + for (attempt in 0 until maxAttempts) { try { return block() } catch (e: Throwable) { From 4c1c5ab29d179ecb0f36c1bda36ef745b9526845 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Thu, 6 Nov 2025 16:11:47 +0700 Subject: [PATCH 15/20] chore: remove unused test --- .../sdk/android/internal/remote/ApiClientImplTest.kt | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt index 034cef8b..9586d8ba 100644 --- a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImplTest.kt @@ -758,16 +758,4 @@ internal class ApiClientImplTest { assertThat(error.statusCode).isEqualTo(case.code) } } - - @Test() - fun `should be retry when got 499 status code at least 3 times before throw error`() { - } - - @Test() - fun `should not be retry when got 3xx, 4xx, 5xx error`() { - } - - @Test() - fun `should stop retry when got other status code != 499`() { - } } From 14f2673c7934b7947ca766733576230a432bbd62 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Thu, 6 Nov 2025 16:58:35 +0700 Subject: [PATCH 16/20] Add documentation and comments to retryOnException Added detailed KDoc and inline comments to the retryOnException function, clarifying its usage, parameters, and behavior. This improves code readability and helps developers understand the retry logic and threading considerations. --- .../android/internal/remote/RetryOnException.kt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt index 38f029e3..50c69e1c 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt @@ -1,5 +1,18 @@ package io.bucketeer.sdk.android.internal.remote +/** + * Retries the given [block] of code up to [maxRetries] times if it throws an exception + * that satisfies [exceptionCheck]. Waits for an increasing delay between retries. + * + * Note: This function blocks the current thread during delays. Use only on background threads. + * + * @param maxRetries Maximum retry attempts (default: 3). + * @param delayMillis Base delay in milliseconds between retries (default: 1000ms). + * @param exceptionCheck Predicate to determine if an exception is retriable. + * @param block Code block to execute. + * @return Result of [block] if successful. + * @throws Throwable Last exception if retries fail or exception is not retriable. + */ internal fun retryOnException( maxRetries: Int = 3, delayMillis: Long = 1000, @@ -16,8 +29,12 @@ internal fun retryOnException( if (!exceptionCheck(e) || attempt >= maxAttempts - 1) { throw e } + // Delay with linear backoff using Thread.sleep. + // Ensure this runs on a background thread as it blocks the current thread. + // Coroutine support is not used since the SDK does not rely on coroutines. Thread.sleep(delayMillis * (attempt + 1)) } } + // This line should never be reached, but Kotlin compiler requires a return statement here. throw lastException!! } From dececc9f3470c77ef19ae59986af7d1847663bdf Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Thu, 6 Nov 2025 17:08:25 +0700 Subject: [PATCH 17/20] Fix KDoc formatting in RetryOnException.kt Corrected the KDoc comment formatting by removing extra indentation to ensure proper documentation rendering. --- .../internal/remote/RetryOnException.kt | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt index 50c69e1c..acb871a3 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt @@ -1,18 +1,18 @@ package io.bucketeer.sdk.android.internal.remote /** - * Retries the given [block] of code up to [maxRetries] times if it throws an exception - * that satisfies [exceptionCheck]. Waits for an increasing delay between retries. - * - * Note: This function blocks the current thread during delays. Use only on background threads. - * - * @param maxRetries Maximum retry attempts (default: 3). - * @param delayMillis Base delay in milliseconds between retries (default: 1000ms). - * @param exceptionCheck Predicate to determine if an exception is retriable. - * @param block Code block to execute. - * @return Result of [block] if successful. - * @throws Throwable Last exception if retries fail or exception is not retriable. - */ +* Retries the given [block] of code up to [maxRetries] times if it throws an exception +* that satisfies [exceptionCheck]. Waits for an increasing delay between retries. +* +* Note: This function blocks the current thread during delays. Use only on background threads. +* +* @param maxRetries Maximum retry attempts (default: 3). +* @param delayMillis Base delay in milliseconds between retries (default: 1000ms). +* @param exceptionCheck Predicate to determine if an exception is retriable. +* @param block Code block to execute. +* @return Result of [block] if successful. +* @throws Throwable Last exception if retries fail or exception is not retriable. +*/ internal fun retryOnException( maxRetries: Int = 3, delayMillis: Long = 1000, From 7526ef0d8910d47b2f0f252c278b48d37ceaad9e Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Fri, 7 Nov 2025 11:04:22 +0700 Subject: [PATCH 18/20] Add tests for fetchEvaluations retry logic on 499 errors Added two tests to verify fetchEvaluations retries on HTTP 499 responses and handles failure after multiple retries. The tests check correct evaluation storage updates, event logging, and request count to ensure proper retry and error handling behavior. --- .../sdk/android/BKTClientImplTest.kt | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/BKTClientImplTest.kt b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/BKTClientImplTest.kt index 3cc550e2..d02b1efe 100644 --- a/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/BKTClientImplTest.kt +++ b/bucketeer/src/test/kotlin/io/bucketeer/sdk/android/BKTClientImplTest.kt @@ -1420,6 +1420,134 @@ class BKTClientImplTest { .isEqualTo(MetricsEventType.INTERNAL_SERVER_ERROR) } + @Test + fun `fetchEvaluations - should retry on 499 status code`() { + server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody( + moshi + .adapter(GetEvaluationsResponse::class.java) + .toJson( + GetEvaluationsResponse( + evaluations = user1Evaluations, + userEvaluationsId = "user_evaluations_id_value", + ), + ), + ), + ) + server.enqueue(MockResponse().setResponseCode(499)) + server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody( + moshi + .adapter(GetEvaluationsResponse::class.java) + .toJson( + GetEvaluationsResponse( + evaluations = user1Evaluations, + userEvaluationsId = "user_evaluations_id_value_updated", + ), + ), + ), + ) + + val initializeFuture = + BKTClient.initialize( + ApplicationProvider.getApplicationContext(), + config, + user1.toBKTUser(), + 1000, + ) + initializeFuture.get() + + val client = BKTClient.getInstance() as BKTClientImpl + val result = client.fetchEvaluations().get() + + Thread.sleep(100) + + assertThat(result).isNull() + + assertThat( + client.componentImpl.dataModule.evaluationStorage + .getCurrentEvaluationId(), + ).isEqualTo("user_evaluations_id_value_updated") + + assertThat( + client.componentImpl.dataModule.evaluationStorage + .get(), + ).hasSize(2) + + // 2 metrics events (latency , size) from the BKTClient internal init() + // 2 metrics events (latency , size) from the test code above + // Because we filter duplicate + // Finally we will have only 2 items, no error event because of the retry + val actualEvents = + client.componentImpl.dataModule.eventSQLDao + .getEvents() + assertThat(actualEvents).hasSize(2) + + // server.requestCount includes the initial request plus retries + assertThat(server.requestCount).isEqualTo(3) + } + + @Test + fun `fetchEvaluations - fail after retry 3 times - should log only 1 error event`() { + server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody( + moshi + .adapter(GetEvaluationsResponse::class.java) + .toJson( + GetEvaluationsResponse( + evaluations = user1Evaluations, + userEvaluationsId = "user_evaluations_id_value", + ), + ), + ), + ) + server.enqueue(MockResponse().setResponseCode(499)) + server.enqueue(MockResponse().setResponseCode(499)) + server.enqueue(MockResponse().setResponseCode(500)) + + val initializeFuture = + BKTClient.initialize( + ApplicationProvider.getApplicationContext(), + config, + user1.toBKTUser(), + 1000, + ) + initializeFuture.get() + + val client = BKTClient.getInstance() as BKTClientImpl + val result = client.fetchEvaluations().get() + + Thread.sleep(100) + + assertThat(result).isInstanceOf(BKTException.InternalServerErrorException::class.java) + + assertThat( + client.componentImpl.dataModule.evaluationStorage + .getCurrentEvaluationId(), + ).isEqualTo("user_evaluations_id_value") + + assertThat( + client.componentImpl.dataModule.evaluationStorage + .get(), + ).hasSize(2) + + // 2 metrics events (latency , size) from the BKTClient internal init() + // 1 metrics events (error) from the test code above + val actualEvents = + client.componentImpl.dataModule.eventSQLDao + .getEvents() + assertThat(actualEvents).hasSize(3) + + // server.requestCount includes the initial request plus retries + assertThat(server.requestCount).isEqualTo(4) + } + @Test fun `fetchEvaluations - onUpdateListener failure`() { server.enqueue( From 44c5a853cca0ae7c7d287e5b25525745ed33e555 Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Fri, 7 Nov 2025 11:45:25 +0700 Subject: [PATCH 19/20] Clone requests before reuse in ApiClientImpl Requests are now cloned using newBuilder().build() before being passed to newCall, preventing issues related to reusing the same request instance in OkHttp. --- .../sdk/android/internal/remote/ApiClientImpl.kt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt index e3fcc107..c5f786a7 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/ApiClientImpl.kt @@ -101,8 +101,10 @@ internal class ApiClientImpl( err is BKTException.ClientClosedRequestException }, ) { + // Clone request to avoid issues with reusing the same request instance + val cloneRequest = request.newBuilder().build() val call = - actualClient.newCall(request) + actualClient.newCall(cloneRequest) logd { "--> Fetch Evaluation\n$body" } val (millis, data) = @@ -173,7 +175,10 @@ internal class ApiClientImpl( err is BKTException.ClientClosedRequestException }, ) { - val call = client.newCall(request) + // Clone request to avoid issues with reusing the same request instance + val cloneRequest = request.newBuilder().build() + val call = + client.newCall(cloneRequest) logd { "--> Register events\n$body" } val response = call.execute() responseStatusCode = response.code From e59782e0fd46afb23970942f15dcd390c1b4b1bb Mon Sep 17 00:00:00 2001 From: duyhungtnn Date: Fri, 7 Nov 2025 11:48:57 +0700 Subject: [PATCH 20/20] Clarify backoff strategy in RetryOnException docs Updated the KDoc for RetryOnException to specify that a linear backoff delay is applied between retries, improving clarity for users of the function. --- .../bucketeer/sdk/android/internal/remote/RetryOnException.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt index acb871a3..6b805e6c 100644 --- a/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt +++ b/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/remote/RetryOnException.kt @@ -2,7 +2,7 @@ package io.bucketeer.sdk.android.internal.remote /** * Retries the given [block] of code up to [maxRetries] times if it throws an exception -* that satisfies [exceptionCheck]. Waits for an increasing delay between retries. +* that satisfies [exceptionCheck]. Linear backoff delay is applied between retries. * * Note: This function blocks the current thread during delays. Use only on background threads. *