[SDK-547] Reduce excess JWT auth token requests (timer race + crypto-timeout wipe) - #1077
Open
franco-zalamena-iterable wants to merge 2 commits into
Open
[SDK-547] Reduce excess JWT auth token requests (timer race + crypto-timeout wipe)#1077franco-zalamena-iterable wants to merge 2 commits into
franco-zalamena-iterable wants to merge 2 commits into
Conversation
The refresh timer, app foreground, and 401 retry paths all call scheduleAuthTokenRefresh from different threads. The isTimerScheduled guard was a non-atomic check-then-act: concurrent callers could all pass it and each schedule a TimerTask, and each foreground reschedule could create a new Timer while orphaning the previous one. Orphaned timers could not be cancelled by clearRefreshTimer and kept firing onAuthTokenRequested, inflating backend JWT generation over time. Make scheduleAuthTokenRefresh and clearRefreshTimer synchronized, and set isTimerScheduled before scheduling (reset on failure) so only one refresh timer is ever active. Adds a concurrency test asserting a single timer under 8 racing callers, and a test asserting foregrounding with a valid, far-from-expiry token does not request a new token. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
IterableKeychain decrypts/encrypts the stored auth token under a 500ms timeout. A slow AndroidKeyStore operation that exceeded it was caught in the same branch as a genuine decryption failure, which wipes the stored email, userId, and auth token and permanently disables encryption. That forces a re-login and a fresh onAuthTokenRequested on the next launch — on slower devices this can recur, inflating backend JWT generation. Handle TimeoutException separately from real crypto failures: on a timeout, preserve the encrypted data and fall back only for the current read/write (plaintext on save), without wiping credentials or disabling encryption. Genuine decryption errors still wipe as before. Adds a test asserting a crypto timeout does not wipe credentials, disable encryption, or invoke the decryption-failure handler. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.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
1 participant
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.
Summary
Addresses two Android-specific defects that inflate
IterableAuthHandler.onAuthTokenRequested()calls (and backend JWT generation), reported by a customer seeing ~4x more Android JWT requests than iOS (SDK-547).Fix 1 — Synchronize JWT refresh timer scheduling (race)
scheduleAuthTokenRefreshused a non-atomicisTimerScheduledcheck-then-set. The refresh timer (timer thread), app foreground (main thread), and 401 retry (network thread) can hit it concurrently; each could pass the guard and schedule its ownTimerTask.clearRefreshTimer()only cancels the oneTimerit holds, so extra timers leak uncancellable and each keeps requesting tokens. Scheduling and clearing are nowsynchronized, and the flag is set before scheduling.Fix 2 — Don't wipe credentials on a transient crypto timeout
IterableKeychainruns crypto under a 500 ms timeout. A slow AndroidKeyStore operation that exceeded it was caught in the same branch as a genuine decryption failure — wiping the stored email, userId, and auth token and disabling encryption — forcing a re-login and a fresh token request on the next launch. Crypto timeouts are now handled as transient: the encrypted data is preserved and only the current read/write falls back, without wiping credentials or disabling encryption. Genuine decryption failures still wipe as before.Testing
Scope note
These are the two defects we could prove reduce request volume. The exact ~4x production ratio is a fleet-scale / device-dependent effect; full attribution needs backend request segmentation + decryption-failure telemetry (to be tracked separately). So we could not 1-1 reproduce it with a long lived token even with different configurations. Worth asking client what is their Iterable setup to try to replicate what they see on our side