SK-3002 retry backoff support#368
Merged
Devesh-Skyflow merged 15 commits intoJul 22, 2026
Merged
Conversation
Hand-written OkHttp interceptor replicating Fern's retry algorithm (exponential backoff + proportional jitter, factor 0.2 fixed). Exposes maxRetries / initialRetryDelayMillis / maxRetryDelayMillis; retries on 408/429/5xx. Carries a TODO to replace with Fern's generated interceptor once a version exposing the delay knobs is vendored. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- VaultConfig: vault-level timeout/maxRetries/initialRetryDelayMillis/ maxRetryDelayMillis (nullable = inherit) - Skyflow.SkyflowClientBuilder: flat client-wide setters + propagation to existing controllers (mirrors addSkyflowCredentials) - VaultClient.updateExecutorInHTTP: set callTimeout + attach SkyflowRetryInterceptor; per-field precedence vault > client > SDK default (defaults 60s / 3 / 500ms / 2000ms) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- SkyflowRetryInterceptorTests: retry count, stop-on-success, 408/429/5xx triggers, non-retryable passthrough, zero-retries - VaultClientHttpConfigTests: callTimeout applied to shared client, precedence (vault > client > default), bounded-not-zero regression guard, VaultConfig field getters/setters Note: pre-existing VaultClientTests#testSetBearerTokenWithEnvCredentials fails on Java 21 (PowerMock byte-buddy cannot instrument JDK security classes) — reproduced on base v3, unrelated to this change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Demonstrates client-wide defaults on Skyflow.builder() and per-vault overrides on VaultConfig, with precedence and unit notes. Compiles against the local SDK build (samples module pins a released version, so it builds once this change ships). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drives an OkHttpClient built like VaultClient (callTimeout + SkyflowRetry Interceptor) against a real loopback MockWebServer: - retries 503 then succeeds (server sees 1+retries requests) - exhausts retries, returns last failure - no retry on 400 - callTimeout aborts a never-responding server (~1s, not hanging) -- the fix - observable backoff between retries Adds com.squareup.okhttp3:mockwebserver as a test dependency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MockWebServer's static init loads JDK security providers, which the PowerMock/byte-buddy agent fails to instrument on Java 21 (IllegalClassFormatException: Unsupported class file major version 65) -> ExceptionInInitializerError in setUp. This is the same agent/JDK incompatibility that already fails testSetBearerTokenWithEnvCredentials. Retry/timeout behavior remains covered by the mocked-Chain unit tests (SkyflowRetryInterceptorTests) and the config/precedence tests (VaultClientHttpConfigTests), both green in CI. Also drops the mockwebserver test dependency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cover the previously-untested new code paths flagged by Codecov: - Skyflow.builder() setters + propagateHttpConfig loop (config set before and after addVaultConfig) -> SkyflowClientBuilderHttpConfigTests - resolveInt/resolveLong vault + client branches via maxRetries + backoff precedence, asserted on the built SkyflowRetryInterceptor's fields - interceptor InterruptedException-during-backoff path (throws IOException, preserves interrupt flag) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…alRetryDelay/maxRetryDelay - Swap hand-written SkyflowRetryInterceptor for Fern's v4 generated RetryInterceptor (public 4-arg ctor: maxRetries, Optional<Long> initial/max delay, Optional<Double> jitter); delete SkyflowRetryInterceptor + its unit test - Rename public knobs initialRetryDelayMillis/maxRetryDelayMillis -> initialRetryDelay/ maxRetryDelay (Fern naming); still long ms, client + vault levels only - VaultClient attaches the generated RetryInterceptor on the shared client, passing resolved values as Optional (jitter Optional.empty -> Fern default 0.2). Gains Retry-After / X-RateLimit-Reset header awareness for free - Update VaultClientHttpConfigTests to reflect Fern's Duration fields; rename sample Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The v4-regen added a static <argLine> (--add-opens ...) to maven-surefire-plugin,
which overwrote the argLine JaCoCo's prepare-agent injects. The coverage agent
therefore never attached -> no jacoco.exec -> "Skipping JaCoCo ... missing
execution data" -> no jacoco.xml -> Codecov "No coverage reports found" (build
green, 439 tests pass, but upload fails).
Prepend @{argLine} (Surefire late-binding of JaCoCo's property) so both the
coverage agent and the --add-opens flags apply. Verified locally: jacoco.exec
and jacoco.xml are generated again.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves the PR merge-build compile error: the base's VaultClientTests (SK-2955, #346) calls .name() directly on FlowEnumUpdateType, which the v4 regen on this branch changed from an enum to a wrapped-enum class. Updated those two assertions (lines ~347-348) to .getUpdateType().get().getEnumValue().name() to match, consistent with the rest of the file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dempotent writes Retries fire on 408/429/5xx for all operations regardless of HTTP verb, so auto-retrying a non-idempotent write (e.g. insert without upsert) after a 5xx/timeout could create a duplicate record. Default maxRetries to 0 so the unconfigured SDK does not retry; customers opt in by setting maxRetries. The 60s callTimeout still bounds every call. initialRetryDelay/maxRetryDelay defaults only take effect once maxRetries > 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…DelayMillis / maxRetryDelayMillis Match Fern's public setter names exactly (initialRetryDelayMillis / maxRetryDelayMillis) so the unit (ms) is self-documenting and consistent with Fern. timeout stays in seconds (also matching Fern). Removes the mixed-unit footgun where the dropped `Millis` suffix made `initialRetryDelay(500)` look like seconds. Renamed across VaultConfig, Skyflow.SkyflowClientBuilder, VaultClient, the config tests, and the sample. Fern interceptor field reflection in tests kept as-is (Fern's field is `initialRetryDelay: Duration`, not the ctor param name). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Timeouts previously surfaced in response.errors as code 500 with a raw "...ApiClientException: Network error..." message, indistinguishable from a server error. In the four bulk handlers, when a batch fails with a non-API exception, walk the cause chain for an InterruptedIOException (OkHttp's call/socket timeout) and report 408 + "Request timed out." instead of 500. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-retry-backoff-support # Conflicts: # samples/src/main/java/com/example/vault/TimeoutAndRetryConfigExample.java # v3/src/main/java/com/skyflow/Skyflow.java # v3/src/main/java/com/skyflow/VaultClient.java # v3/src/main/java/com/skyflow/config/VaultConfig.java # v3/src/test/java/com/skyflow/SkyflowClientBuilderHttpConfigTests.java # v3/src/test/java/com/skyflow/VaultClientHttpConfigTests.java
skyflow-bharti
approved these changes
Jul 22, 2026
skyflow-bharti
changed the title
Devesh-Skyflow
added a commit
that referenced
this pull request
Jul 22, 2026
* SK-2955 align upsert/upsertType with Flow-DB changes (docs, tests, error-message cleanup) (#346) Flow-DB now defaults record-level upsert to UPDATE (was REPLACE) and owns upsert-placement validation server-side. The SDK already serializes upsertType correctly and omits updateType when unset, so this is docs, tests, and validation hygiene only. - README: document upsertType is optional, default is now UPDATE, and UPDATE vs REPLACE semantics. - Samples: drop redundant explicit upsertType(UPDATE) (now the default) in BulkMultiTableInsert{Sync,Async}; keep REPLACE samples as-is. - Validations: defer upsert-placement checks to the backend (authoritative messages); SDK only guards empty upsert columns. - Tests: cover default-UPDATE omission at request/record level, both-level serialization, and updated placement tests to reflect deferral. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * [AUTOMATED] Private Release 2.1.1-dev-ce79ce1 * SK-3002 retry backoff support (#367) * SK-3002 add SkyflowRetryInterceptor (configurable retry count + backoff) Hand-written OkHttp interceptor replicating Fern's retry algorithm (exponential backoff + proportional jitter, factor 0.2 fixed). Exposes maxRetries / initialRetryDelayMillis / maxRetryDelayMillis; retries on 408/429/5xx. Carries a TODO to replace with Fern's generated interceptor once a version exposing the delay knobs is vendored. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 expose configurable timeout + retries (client & vault level) - VaultConfig: vault-level timeout/maxRetries/initialRetryDelayMillis/ maxRetryDelayMillis (nullable = inherit) - Skyflow.SkyflowClientBuilder: flat client-wide setters + propagation to existing controllers (mirrors addSkyflowCredentials) - VaultClient.updateExecutorInHTTP: set callTimeout + attach SkyflowRetryInterceptor; per-field precedence vault > client > SDK default (defaults 60s / 3 / 500ms / 2000ms) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 tests for configurable timeout + retries - SkyflowRetryInterceptorTests: retry count, stop-on-success, 408/429/5xx triggers, non-retryable passthrough, zero-retries - VaultClientHttpConfigTests: callTimeout applied to shared client, precedence (vault > client > default), bounded-not-zero regression guard, VaultConfig field getters/setters Note: pre-existing VaultClientTests#testSetBearerTokenWithEnvCredentials fails on Java 21 (PowerMock byte-buddy cannot instrument JDK security classes) — reproduced on base v3, unrelated to this change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 add sample for configurable timeout + retries Demonstrates client-wide defaults on Skyflow.builder() and per-vault overrides on VaultConfig, with precedence and unit notes. Compiles against the local SDK build (samples module pins a released version, so it builds once this change ships). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 add MockWebServer integration test (live timeout + retry proof) Drives an OkHttpClient built like VaultClient (callTimeout + SkyflowRetry Interceptor) against a real loopback MockWebServer: - retries 503 then succeeds (server sees 1+retries requests) - exhausts retries, returns last failure - no retry on 400 - callTimeout aborts a never-responding server (~1s, not hanging) -- the fix - observable backoff between retries Adds com.squareup.okhttp3:mockwebserver as a test dependency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 remove MockWebServer test (breaks CI on PowerMock + Java 21) MockWebServer's static init loads JDK security providers, which the PowerMock/byte-buddy agent fails to instrument on Java 21 (IllegalClassFormatException: Unsupported class file major version 65) -> ExceptionInInitializerError in setUp. This is the same agent/JDK incompatibility that already fails testSetBearerTokenWithEnvCredentials. Retry/timeout behavior remains covered by the mocked-Chain unit tests (SkyflowRetryInterceptorTests) and the config/precedence tests (VaultClientHttpConfigTests), both green in CI. Also drops the mockwebserver test dependency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 raise coverage for new timeout/retry code Cover the previously-untested new code paths flagged by Codecov: - Skyflow.builder() setters + propagateHttpConfig loop (config set before and after addVaultConfig) -> SkyflowClientBuilderHttpConfigTests - resolveInt/resolveLong vault + client branches via maxRetries + backoff precedence, asserted on the built SkyflowRetryInterceptor's fields - interceptor InterruptedException-during-backoff path (throws IOException, preserves interrupt flag) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002-retry-backoff-support * SK-3002 use Fern v4 generated RetryInterceptor; rename knobs to initialRetryDelay/maxRetryDelay - Swap hand-written SkyflowRetryInterceptor for Fern's v4 generated RetryInterceptor (public 4-arg ctor: maxRetries, Optional<Long> initial/max delay, Optional<Double> jitter); delete SkyflowRetryInterceptor + its unit test - Rename public knobs initialRetryDelayMillis/maxRetryDelayMillis -> initialRetryDelay/ maxRetryDelay (Fern naming); still long ms, client + vault levels only - VaultClient attaches the generated RetryInterceptor on the shared client, passing resolved values as Optional (jitter Optional.empty -> Fern default 0.2). Gains Retry-After / X-RateLimit-Reset header awareness for free - Update VaultClientHttpConfigTests to reflect Fern's Duration fields; rename sample Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 fix CI coverage: prepend @{argLine} so JaCoCo agent attaches The v4-regen added a static <argLine> (--add-opens ...) to maven-surefire-plugin, which overwrote the argLine JaCoCo's prepare-agent injects. The coverage agent therefore never attached -> no jacoco.exec -> "Skipping JaCoCo ... missing execution data" -> no jacoco.xml -> Codecov "No coverage reports found" (build green, 439 tests pass, but upload fails). Prepend @{argLine} (Surefire late-binding of JaCoCo's property) so both the coverage agent and the --add-opens flags apply. Verified locally: jacoco.exec and jacoco.xml are generated again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 default maxRetries=0 (retries opt-in) to avoid retrying non-idempotent writes Retries fire on 408/429/5xx for all operations regardless of HTTP verb, so auto-retrying a non-idempotent write (e.g. insert without upsert) after a 5xx/timeout could create a duplicate record. Default maxRetries to 0 so the unconfigured SDK does not retry; customers opt in by setting maxRetries. The 60s callTimeout still bounds every call. initialRetryDelay/maxRetryDelay defaults only take effect once maxRetries > 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Devesh-Skyflow <devesh.bhardwaj@skyflow.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * [AUTOMATED] Private Release 2.1.1-dev-1236334 * SK-3002 retry backoff support (#368) * SK-3002 add SkyflowRetryInterceptor (configurable retry count + backoff) Hand-written OkHttp interceptor replicating Fern's retry algorithm (exponential backoff + proportional jitter, factor 0.2 fixed). Exposes maxRetries / initialRetryDelayMillis / maxRetryDelayMillis; retries on 408/429/5xx. Carries a TODO to replace with Fern's generated interceptor once a version exposing the delay knobs is vendored. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 expose configurable timeout + retries (client & vault level) - VaultConfig: vault-level timeout/maxRetries/initialRetryDelayMillis/ maxRetryDelayMillis (nullable = inherit) - Skyflow.SkyflowClientBuilder: flat client-wide setters + propagation to existing controllers (mirrors addSkyflowCredentials) - VaultClient.updateExecutorInHTTP: set callTimeout + attach SkyflowRetryInterceptor; per-field precedence vault > client > SDK default (defaults 60s / 3 / 500ms / 2000ms) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 tests for configurable timeout + retries - SkyflowRetryInterceptorTests: retry count, stop-on-success, 408/429/5xx triggers, non-retryable passthrough, zero-retries - VaultClientHttpConfigTests: callTimeout applied to shared client, precedence (vault > client > default), bounded-not-zero regression guard, VaultConfig field getters/setters Note: pre-existing VaultClientTests#testSetBearerTokenWithEnvCredentials fails on Java 21 (PowerMock byte-buddy cannot instrument JDK security classes) — reproduced on base v3, unrelated to this change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 add sample for configurable timeout + retries Demonstrates client-wide defaults on Skyflow.builder() and per-vault overrides on VaultConfig, with precedence and unit notes. Compiles against the local SDK build (samples module pins a released version, so it builds once this change ships). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 add MockWebServer integration test (live timeout + retry proof) Drives an OkHttpClient built like VaultClient (callTimeout + SkyflowRetry Interceptor) against a real loopback MockWebServer: - retries 503 then succeeds (server sees 1+retries requests) - exhausts retries, returns last failure - no retry on 400 - callTimeout aborts a never-responding server (~1s, not hanging) -- the fix - observable backoff between retries Adds com.squareup.okhttp3:mockwebserver as a test dependency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 remove MockWebServer test (breaks CI on PowerMock + Java 21) MockWebServer's static init loads JDK security providers, which the PowerMock/byte-buddy agent fails to instrument on Java 21 (IllegalClassFormatException: Unsupported class file major version 65) -> ExceptionInInitializerError in setUp. This is the same agent/JDK incompatibility that already fails testSetBearerTokenWithEnvCredentials. Retry/timeout behavior remains covered by the mocked-Chain unit tests (SkyflowRetryInterceptorTests) and the config/precedence tests (VaultClientHttpConfigTests), both green in CI. Also drops the mockwebserver test dependency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 raise coverage for new timeout/retry code Cover the previously-untested new code paths flagged by Codecov: - Skyflow.builder() setters + propagateHttpConfig loop (config set before and after addVaultConfig) -> SkyflowClientBuilderHttpConfigTests - resolveInt/resolveLong vault + client branches via maxRetries + backoff precedence, asserted on the built SkyflowRetryInterceptor's fields - interceptor InterruptedException-during-backoff path (throws IOException, preserves interrupt flag) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002-retry-backoff-support * SK-3002 use Fern v4 generated RetryInterceptor; rename knobs to initialRetryDelay/maxRetryDelay - Swap hand-written SkyflowRetryInterceptor for Fern's v4 generated RetryInterceptor (public 4-arg ctor: maxRetries, Optional<Long> initial/max delay, Optional<Double> jitter); delete SkyflowRetryInterceptor + its unit test - Rename public knobs initialRetryDelayMillis/maxRetryDelayMillis -> initialRetryDelay/ maxRetryDelay (Fern naming); still long ms, client + vault levels only - VaultClient attaches the generated RetryInterceptor on the shared client, passing resolved values as Optional (jitter Optional.empty -> Fern default 0.2). Gains Retry-After / X-RateLimit-Reset header awareness for free - Update VaultClientHttpConfigTests to reflect Fern's Duration fields; rename sample Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 fix CI coverage: prepend @{argLine} so JaCoCo agent attaches The v4-regen added a static <argLine> (--add-opens ...) to maven-surefire-plugin, which overwrote the argLine JaCoCo's prepare-agent injects. The coverage agent therefore never attached -> no jacoco.exec -> "Skipping JaCoCo ... missing execution data" -> no jacoco.xml -> Codecov "No coverage reports found" (build green, 439 tests pass, but upload fails). Prepend @{argLine} (Surefire late-binding of JaCoCo's property) so both the coverage agent and the --add-opens flags apply. Verified locally: jacoco.exec and jacoco.xml are generated again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 default maxRetries=0 (retries opt-in) to avoid retrying non-idempotent writes Retries fire on 408/429/5xx for all operations regardless of HTTP verb, so auto-retrying a non-idempotent write (e.g. insert without upsert) after a 5xx/timeout could create a duplicate record. Default maxRetries to 0 so the unconfigured SDK does not retry; customers opt in by setting maxRetries. The 60s callTimeout still bounds every call. initialRetryDelay/maxRetryDelay defaults only take effect once maxRetries > 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 rename retry delay knobs to Fern's public names: initialRetryDelayMillis / maxRetryDelayMillis Match Fern's public setter names exactly (initialRetryDelayMillis / maxRetryDelayMillis) so the unit (ms) is self-documenting and consistent with Fern. timeout stays in seconds (also matching Fern). Removes the mixed-unit footgun where the dropped `Millis` suffix made `initialRetryDelay(500)` look like seconds. Renamed across VaultConfig, Skyflow.SkyflowClientBuilder, VaultClient, the config tests, and the sample. Fern interceptor field reflection in tests kept as-is (Fern's field is `initialRetryDelay: Duration`, not the ctor param name). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * SK-3002 classify client-side timeout as 408 in batch error records Timeouts previously surfaced in response.errors as code 500 with a raw "...ApiClientException: Network error..." message, indistinguishable from a server error. In the four bulk handlers, when a batch fails with a non-API exception, walk the cause chain for an InterruptedIOException (OkHttp's call/socket timeout) and report 408 + "Request timed out." instead of 500. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: skyflow-bharti <bharti.sagar@skyflow.com> * [AUTOMATED] Private Release 2.1.1-dev-3196451 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Devesh-Skyflow <Devesh-Skyflow@users.noreply.github.com> Co-authored-by: skyflow-bharti <118584001+skyflow-bharti@users.noreply.github.com> Co-authored-by: skyflow-bharti <bharti.sagar@skyflow.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Start with a concise summary of the PR. The first three sections are required. The questions present in each section is there to help you guide you what to add. They are meant to be overwritten by your comments.
Why
Goal
Testing
Tech debt