update critical path to be more async #118
Merged
Conversation
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
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the startup and device ID flows to run heavy operations asynchronously, improves thread safety, and updates build configuration.
- Moves storage setup, device ID generation, and attestation into background
async/launchblocks. - Updates token refresh logic to use non-blocking coroutines and adds exponential backoff for attestation retries.
- Bumps
clerk-sdkversion and enables code minification for release builds.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| DeviceIdGenerator.kt | Simplified storage load/save checks and added async persistence logic |
| ConfigurationManager.kt | Converted init, refresh, and attestation flows to background tasks |
| AppLifecycleListener.kt | Ensured lifecycle observer addition happens on the main thread |
| source/src/test/java/com/clerk/configuration/DeviceIdGeneratorTest.kt | Updated tests to use explicit call counts instead of constants |
| source/build.gradle.kts | Enabled isMinifyEnabled for release builds |
| gradle/libs.versions.toml | Upgraded clerk-sdk from 0.1.1 to 0.1.2 |
Comments suppressed due to low confidence (3)
source/src/main/kotlin/com/clerk/configuration/ConfigurationManager.kt:189
- [nitpick] The log message fires after awaiting the data refresh job, so it actually indicates background initialization completed, not started. Update the wording for accuracy.
ClerkLog.d("ConfigurationManager configured successfully - background initialization started")
source/src/main/kotlin/com/clerk/configuration/DeviceIdGenerator.kt:150
- Current code always overwrites any existing stored device ID. Consider re-adding a check (load first) so you only save when no ID exists to avoid unintentional overrides.
StorageHelper.saveValue(StorageKey.DEVICE_ID, deviceId)
source/src/main/kotlin/com/clerk/configuration/ConfigurationManager.kt:217
- Using
asyncwithout awaiting the returnedDeferredcan launch untracked coroutines. Uselaunchor explicitlyawaitthe result to ensure proper coroutine lifecycle and error handling.
async { session.fetchToken(GetTokenOptions(skipCache = false)) }
| // Ensure observer is added on the main thread | ||
| if (Looper.myLooper() == Looper.getMainLooper()) { | ||
| // Already on main thread | ||
| ProcessLifecycleOwner.get().lifecycle.addObserver(listener) |
There was a problem hiding this comment.
[nitpick] Repeated calls to configure can add the same observer multiple times. Consider tracking a flag or removing existing observers to make configure idempotent.
Copilot uses AI. Check for mistakes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This pull request introduces several improvements to the
ConfigurationManagerandDeviceIdGeneratorclasses, focusing on optimizing asynchronous operations, enhancing thread safety, and improving initialization performance. Additionally, it updates theclerk-sdklibrary version and enables code minification for release builds. Below is a categorized summary of the most important changes.Build Configuration Updates:
clerk-sdklibrary version from0.1.1to0.1.2ingradle/libs.versions.toml.source/build.gradle.ktsby settingisMinifyEnabledtotrue.Asynchronous Initialization Enhancements:
ConfigurationManagerinitialization by moving storage setup, device ID generation, and lifecycle monitoring to background tasks usingasyncblocks. This avoids blocking the main thread and improves startup performance. [1] [2]async) for fetching tokens, ensuring the refresh loop remains non-blocking.Device Attestation Improvements:
ConfigurationManagerto run independently in background tasks usingasyncfor better isolation and concurrency. Added retry logic with exponential backoff for failed attestation attempts. [1] [2] [3]Device ID Generation Enhancements:
DeviceIdGeneratorto prioritize loading existing device IDs from storage before generating new ones. Improved thread safety with double-checked locking and added asynchronous persistence for new IDs. [1] [2]Logging and Error Handling:
ConfigurationManagerandDeviceIdGeneratorto provide more detailed insights into initialization, attestation, and token refresh processes. Enhanced error handling to ensure background operations fail gracefully without blocking critical workflows. [1] [2]