Skip to content

fix: reverted onInitialized behaviour to previous and moved logic to … #295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jsalaber merged 2 commits into from
May 19, 2026
Merged

fix: reverted onInitialized behaviour to previous and moved logic to … #295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb57d90
fix: reverted onInitialized behaviour to previous and moved logic to …
jsalaber May 14, 2026
4523a0d
chore: fix hanging tests
jsalaber May 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 36 additions & 84 deletions android-client-sdk/src/main/java/com/devcycle/sdk/android/api/DevCycleClient.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,17 @@ class DevCycleClient private constructor(
}

init {
val cacheHit = useCachedConfigForUser(user)
useCachedConfigForUser(user)

if (cacheHit) {
isInitialized.set(true)
initializeJob = CompletableDeferred(Unit)

coroutineScope.launch(coroutineContext) {
initializeJob = coroutineScope.async(coroutineContext) {
isExecuting.set(true)
try {
fetchConfig(user)
Comment thread
jsalaber marked this conversation as resolved.
isInitialized.set(true)
Comment thread
jsalaber marked this conversation as resolved.
Comment thread
jsalaber marked this conversation as resolved.
} catch (t: Throwable) {
DevCycleLogger.e(t, "DevCycle SDK Failed to Initialize!")
throw t
Comment thread
jsalaber marked this conversation as resolved.
Copy link
Copy Markdown
Member

@jonathannorris jonathannorris May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notifyConfigError is defined but never called — the init catch block does not call it before rethrowing, so direct onConfigUpdated subscribers will not hear about init failures. Either call it here or remove the method if it is intentionally out of scope for this PR.

Copy link
Copy Markdown
Contributor Author

@jsalaber jsalaber May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notifyConfigError gets called for any refetch / sse config fetches

Comment thread
jsalaber marked this conversation as resolved.
} finally {
withContext(Dispatchers.IO) {
initEventSource()
val application: Application = context.applicationContext as Application
Expand All @@ -119,38 +123,13 @@ class DevCycleClient private constructor(
)
application.registerActivityLifecycleCallbacks(lifecycleCallbacks)
Comment thread
jsalaber marked this conversation as resolved.
}
// Fetch fresh config in background (ADR 0009). SSE only fires on
// server-side changes, so an explicit fetch is needed to verify the cache.
performBackgroundRefresh()
}
} else {
initializeJob = coroutineScope.async(coroutineContext) {
isExecuting.set(true)
try {
fetchConfig(user)
isInitialized.set(true)
withContext(Dispatchers.IO) {
initEventSource()
val application: Application = context.applicationContext as Application
val lifecycleCallbacks = DVCLifecycleCallbacks(
onPauseApplication,
onResumeApplication,
config?.sse?.inactivityDelay?.toLong(),
customLifecycleHandler
)
application.registerActivityLifecycleCallbacks(lifecycleCallbacks)
}
} catch (t: Throwable) {
DevCycleLogger.e(t, "DevCycle SDK Failed to Initialize!")
throw t
}
}
}

initializeJob.invokeOnCompletion {
coroutineScope.launch(coroutineContext) {
handleQueuedConfigRequests()
isExecuting.set(false)
}
initializeJob.invokeOnCompletion {
coroutineScope.launch(coroutineContext) {
handleQueuedConfigRequests()
isExecuting.set(false)
}
}
}
Expand Down Expand Up @@ -214,12 +193,12 @@ class DevCycleClient private constructor(
}
}

internal val isUsingCachedConfig: Boolean
val hasUsableCachedConfig: Boolean
Comment thread
jsalaber marked this conversation as resolved.
@Synchronized get() = config != null && isConfigCached.get()

/**
* Atomically updates [config] and [isConfigCached] under the intrinsic lock so that
* [isUsingCachedConfig] never observes a torn write where one field reflects the new value
* [hasUsableCachedConfig] never observes a torn write where one field reflects the new value
* and the other still holds the old one.
*/
@Synchronized
Expand Down Expand Up @@ -267,22 +246,14 @@ class DevCycleClient private constructor(

override fun onError(error: Throwable) {
DevCycleLogger.d("Error fetching config for user_id %s: %s", updatedUser.userId, error.message)

if (error is DVCRequestException && (error.isAuthError || error.statusCode == 400)) {
dvcSharedPrefs.clearConfigForUser(updatedUser)
DevCycleLogger.w("Config error during identifyUser (${error.statusCode}). Persisted cache cleared.")
latestIdentifiedUser = previousUser
callback?.onError(error)
return
}

// In the event that the config request fails (i.e. the device is offline)
// Fallback to using a Cached Configuration for the User if available
val hasCachedConfig = tryLoadCachedConfigForUser(updatedUser)
if (hasCachedConfig) {
// Successfully used cached config, return success
DevCycleLogger.i("Using cached config for identifyUser due to network error: $error")
this@DevCycleClient.user = updatedUser
config?.variables?.let { callback?.onSuccess(it) }
Comment thread
jsalaber marked this conversation as resolved.
performBackgroundRefresh()
} else {
// No cached config available, restore previous state and return error
latestIdentifiedUser = previousUser
Expand Down Expand Up @@ -611,7 +582,12 @@ class DevCycleClient private constructor(
fetchConfig(latestIdentifiedUser, sse, lastModified, etag)
config?.variables?.let { callback?.onSuccess(it) }
} catch (t: Throwable) {
callback?.onError(t)
if (callback != null) {
callback.onError(t)
} else {
// SSE-triggered refetch — no caller callback; surface error via onConfigUpdated listeners.
notifyConfigError(t)
}
} finally {
handleQueuedConfigRequests()
isExecuting.set(false)
Expand Down Expand Up @@ -651,10 +627,20 @@ class DevCycleClient private constructor(
return false
}

internal fun onConfigUpdated(callback: DevCycleCallback<Map<String, BaseConfigVariable>>) {
fun onConfigUpdated(callback: DevCycleCallback<Map<String, BaseConfigVariable>>) {
configUpdatedCallbacks.add(callback)
}

Comment thread
jsalaber marked this conversation as resolved.
private fun notifyConfigError(t: Throwable) {
configUpdatedCallbacks.forEach { callback ->
try {
callback.onError(t)
} catch (e: Exception) {
DevCycleLogger.e(e, "Error in config updated error callback")
}
}
}

private fun notifyConfigUpdated(variables: Map<String, BaseConfigVariable>?) {
variables?.let { vars ->
configUpdatedCallbacks.forEach { callback ->
Expand All @@ -667,40 +653,6 @@ class DevCycleClient private constructor(
}
}

private fun notifyConfigError(error: Throwable) {
configUpdatedCallbacks.forEach { callback ->
try {
callback.onError(error)
} catch (e: Exception) {
DevCycleLogger.e(e, "Error in config error callback")
}
}
}

private fun performBackgroundRefresh() {
val userAtRefreshStart = latestIdentifiedUser
refetchConfig(false, null, null, object : DevCycleCallback<Map<String, BaseConfigVariable>> {
override fun onSuccess(result: Map<String, BaseConfigVariable>) {
DevCycleLogger.d("Background refresh succeeded")
}

override fun onError(error: Throwable) {
val isConfigError = error is DVCRequestException &&
(error.isAuthError || error.statusCode == 400)

if (isConfigError) {
dvcSharedPrefs.clearConfigForUser(userAtRefreshStart)
DevCycleLogger.w("Background refresh config error (${(error as DVCRequestException).statusCode}). Persisted cache cleared.")
if (configUpdatedCallbacks.isNotEmpty()) {
notifyConfigError(error)
}
} else {
DevCycleLogger.w("Background refresh failed: ${error.message}. Keeping caches.")
}
}
})
}

class DevCycleClientBuilder {
private var context: Context? = null
private var customLifecycleHandler: Handler? = null
Expand Down
106 changes: 67 additions & 39 deletions android-client-sdk/src/main/java/com/devcycle/sdk/android/openfeature/DevCycleProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,14 @@ class DevCycleProvider(
)

/**
* Helper function to create a ProviderEvaluation from a DevCycle variable.
*
* [isUsingCachedConfig] must be captured from the same client reference used to retrieve
* [variable], so the reason reflects the config state at the moment of evaluation rather than
* a later read of the mutable [_devcycleClient] field.
* [hasUsableCachedConfig] must be captured from the same client reference used to retrieve
* [variable] so the reason reflects the config state at evaluation time rather than a later
* read of the mutable [_devcycleClient] field.
*/
private fun <T> createProviderEvaluation(
variable: Variable<*>,
value: T,
isUsingCachedConfig: Boolean,
hasUsableCachedConfig: Boolean,
): ProviderEvaluation<T> {
val metadataBuilder = EvaluationMetadata.builder()
var hasMetadata = false
Expand All @@ -71,7 +69,7 @@ class DevCycleProvider(

val reason = when {
variable.isDefaulted == true -> Reason.DEFAULT.toString()
isUsingCachedConfig -> Reason.CACHED.toString()
hasUsableCachedConfig -> Reason.CACHED.toString()
else -> variable.eval?.reason ?: Reason.TARGETING_MATCH.toString()
}

Expand Down Expand Up @@ -123,46 +121,76 @@ class DevCycleProvider(

_devcycleClient = clientBuilder.build()

_devcycleClient!!.onConfigUpdated(object : DevCycleCallback<Map<String, BaseConfigVariable>> {
override fun onSuccess(result: Map<String, BaseConfigVariable>) {
_providerEvents.tryEmit(OpenFeatureProviderEvents.ProviderConfigurationChanged)
DevCycleLogger.d("Emitted PROVIDER_CONFIGURATION_CHANGED event")
}

override fun onError(t: Throwable) {
DevCycleLogger.e("Config error: ${t.message}")
_providerEvents.tryEmit(OpenFeatureProviderEvents.ProviderError(
OpenFeatureError.GeneralError(t.message ?: "Config error")
))
}
})

if (_devcycleClient!!.isUsingCachedConfig) {
DevCycleLogger.d("DevCycle OpenFeature provider initialized from cache (PROVIDER_READY)")
return
// consumeResume guard — ensures continuation.resume is called exactly once.
// On a cache hit the cache check below resolves immediately; onInitialized fires later
// (after network) and emits ProviderConfigurationChanged.
// On a cache miss onInitialized is the sole resolve.
val lock = Any()
var didResume = false
fun consumeResume(): Boolean = synchronized(lock) {
if (didResume) false else { didResume = true; true }
}

// Cache miss: block until network fetch completes
// Captured before registering callbacks to avoid a race where onInitialized fires
// first and incorrectly treats a cache-hit init as a fatal error.
val isCacheHit = _devcycleClient!!.hasUsableCachedConfig

suspendCancellableCoroutine<Unit> { continuation ->

// SSE-triggered config updates fire post-init whenever the realtime connection
// delivers a new config.
_devcycleClient!!.onConfigUpdated(object : DevCycleCallback<Map<String, BaseConfigVariable>> {
override fun onSuccess(result: Map<String, BaseConfigVariable>) {
_providerEvents.tryEmit(OpenFeatureProviderEvents.ProviderConfigurationChanged)
}
override fun onError(t: Throwable) {
_providerEvents.tryEmit(OpenFeatureProviderEvents.ProviderError(
OpenFeatureError.GeneralError(t.message ?: "Config error")
))
}
Copy link
Copy Markdown
Member

@jonathannorris jonathannorris May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These continuation.resume / resumeWithException calls aren't guarded by continuation.isActive. If the calling coroutine is cancelled before the network fetch completes (e.g. an OpenFeature init timeout), the callbacks are still registered on _devcycleClient and will try to resume an already-inactive continuation when the fetch eventually finishes. I'd guard each call with if (continuation.isActive), or use invokeOnCancellation to deregister.

})

// onInitialized always fires after the network fetch completes.
// Cache miss: sole resolve — resume the continuation.
// Cache hit: continuation already resumed below; surface the network result as an event.
_devcycleClient!!.onInitialized(object : DevCycleCallback<String> {
override fun onSuccess(result: String) {
DevCycleLogger.d("DevCycle OpenFeature provider initialized successfully")
continuation.resume(Unit)
if (consumeResume()) {
// Cache miss: first to resolve — resume the continuation if still active.
if (continuation.isActive) continuation.resume(Unit)
} else {
// Cache hit: continuation already resolved; surface network result as event.
_providerEvents.tryEmit(OpenFeatureProviderEvents.ProviderConfigurationChanged)
Comment on lines 157 to +163
}
}

override fun onError(t: Throwable) {
DevCycleLogger.e("DevCycle OpenFeature provider initialization failed: ${t.message}")
continuation.resumeWithException(
OpenFeatureError.ProviderFatalError("DevCycle client initialization error: ${t.message}")
)
if (!isCacheHit) {
// Cache miss: fatal init error — resume with exception if still active.
if (consumeResume() && continuation.isActive) {
continuation.resumeWithException(
OpenFeatureError.ProviderFatalError("DevCycle client initialization error: ${t.message}")
)
}
} else {
// Cache hit: network error is non-fatal; always emit — continuation is already resolved.
_providerEvents.tryEmit(OpenFeatureProviderEvents.ProviderError(
OpenFeatureError.GeneralError("Background refresh failed: ${t.message}")
))
Comment on lines 166 to +178
}
}
})

// Cache hit: resolve immediately — the network fetch continues in the background.
if (isCacheHit) {
if (continuation.isActive && consumeResume()) {
continuation.resume(Unit)
}
Comment thread
jsalaber marked this conversation as resolved.
}
}
} catch (e: OpenFeatureError) {
// Re-throw OpenFeature errors as-is
throw e
} catch (e: Exception) {
DevCycleLogger.e("DevCycle OpenFeature provider initialization failed: ${e.message}")
throw OpenFeatureError.ProviderFatalError("DevCycle client initialization error: ${e.message}")
}
}
Expand Down Expand Up @@ -213,7 +241,7 @@ class DevCycleProvider(
): ProviderEvaluation<Boolean> {
val client = _devcycleClient ?: return createDefaultProviderEvaluation(defaultValue)
val variable = client.variable(key, defaultValue)
return createProviderEvaluation(variable, variable.value, client.isUsingCachedConfig)
return createProviderEvaluation(variable, variable.value, client.hasUsableCachedConfig)
}

override fun getDoubleEvaluation(
Expand All @@ -223,7 +251,7 @@ class DevCycleProvider(
): ProviderEvaluation<Double> {
val client = _devcycleClient ?: return createDefaultProviderEvaluation(defaultValue)
val variable = client.variable(key, defaultValue)
return createProviderEvaluation(variable, variable.value.toDouble(), client.isUsingCachedConfig)
return createProviderEvaluation(variable, variable.value.toDouble(), client.hasUsableCachedConfig)
}

override fun getIntegerEvaluation(
Expand All @@ -233,7 +261,7 @@ class DevCycleProvider(
): ProviderEvaluation<Int> {
val client = _devcycleClient ?: return createDefaultProviderEvaluation(defaultValue)
val variable = client.variable(key, defaultValue)
return createProviderEvaluation(variable, variable.value.toInt(), client.isUsingCachedConfig)
return createProviderEvaluation(variable, variable.value.toInt(), client.hasUsableCachedConfig)
}

override fun getObjectEvaluation(
Expand All @@ -242,7 +270,7 @@ class DevCycleProvider(
context: EvaluationContext?
): ProviderEvaluation<Value> {
val client = _devcycleClient ?: return createDefaultProviderEvaluation(defaultValue)
val isUsingCachedConfig = client.isUsingCachedConfig
val hasUsableCachedConfig = client.hasUsableCachedConfig

val (result, variable) = when {
defaultValue is Value.Structure -> {
Expand Down Expand Up @@ -274,7 +302,7 @@ class DevCycleProvider(
}
}

return createProviderEvaluation(variable, result, isUsingCachedConfig)
return createProviderEvaluation(variable, result, hasUsableCachedConfig)
}

override fun getStringEvaluation(
Expand All @@ -284,7 +312,7 @@ class DevCycleProvider(
): ProviderEvaluation<String> {
val client = _devcycleClient ?: return createDefaultProviderEvaluation(defaultValue)
val variable = client.variable(key, defaultValue)
return createProviderEvaluation(variable, variable.value, client.isUsingCachedConfig)
return createProviderEvaluation(variable, variable.value, client.hasUsableCachedConfig)
}

override fun track(
Expand Down
15 changes: 0 additions & 15 deletions android-client-sdk/src/main/java/com/devcycle/sdk/android/util/DVCSharedPrefs.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,21 +187,6 @@ internal class DVCSharedPrefs(context: Context, private val configCacheTTL: Long



@Synchronized
fun clearConfigForUser(user: PopulatedUser) {
try {
val userKey = generateUserConfigKey(user.userId, user.isAnonymous)
val userExpiryDateKey = generateUserExpiryDateKey(user.userId, user.isAnonymous)
val editor = preferences.edit()
editor.remove(userKey)
editor.remove(userExpiryDateKey)
editor.apply()
DevCycleLogger.d("Cleared persisted config for user_id %s", user.userId)
} catch (e: Exception) {
DevCycleLogger.e(e, "Error clearing config for user: ${e.message}")
}
}

@Synchronized
fun remove(key: String?) {
try {
Expand Down
Loading
Loading