From 98df171e32cd9189c21e38f87bc99a49fe53d398 Mon Sep 17 00:00:00 2001 From: pandeymangg Date: Wed, 5 Aug 2026 18:09:16 +0530 Subject: [PATCH 1/3] fix: don't crash when showing a survey (#43) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `IllegalStateException: FragmentManager has been destroyed`, reported at https://github.com/formbricks/android/issues/43. Three defects stacked up: 1. The fragment transaction was committed on a timer thread. SurveyManager schedules the display on a `java.util.Timer`, whose task called `showSurvey` directly, which reaches `DialogFragment.show()`. androidx requires fragment transactions on the main thread. This is not delay-specific: a zero delay still schedules through the timer, so every display took that path. 2. The stored FragmentManager can outlive the Activity it came from. It is captured once at setup, so in a login -> dashboard flow it belongs to an Activity that has already finished. That is why the reporter saw the crash on first login only and not on later launches. 3. Nothing checked whether the manager was still usable. `showSurvey` now hops to the main looper, skips a destroyed manager with an error that tells the host to call `setFragmentManager` from the current Activity, skips a state-saved manager (a commit after onSaveInstanceState throws, and the host is on its way to the background anyway), and wraps the commit so a survey that fails to show can never take the host app down. As a side effect the timer task now only posts to the main looper, so it can no longer throw — a throwing TimerTask permanently cancels `java.util.Timer`, which would have stopped every later survey in that process. --- .../java/com/formbricks/android/Formbricks.kt | 42 ++++++++++++++++--- .../android/model/error/SDKError.kt | 5 +++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/android/src/main/java/com/formbricks/android/Formbricks.kt b/android/src/main/java/com/formbricks/android/Formbricks.kt index bac728e..27478c7 100644 --- a/android/src/main/java/com/formbricks/android/Formbricks.kt +++ b/android/src/main/java/com/formbricks/android/Formbricks.kt @@ -3,6 +3,8 @@ package com.formbricks.android import android.content.Context import android.net.ConnectivityManager import android.net.NetworkCapabilities +import android.os.Handler +import android.os.Looper import androidx.annotation.Keep import androidx.fragment.app.FragmentManager import com.formbricks.android.api.FormbricksApi @@ -320,16 +322,44 @@ object Formbricks { this.fragmentManager = fragmentManager } - /// Assembles the survey fragment and presents it + /// Assembles the survey fragment and presents it. + /// + /// This is called from `SurveyManager`'s display timer, which runs on a + /// `java.util.Timer` thread. `DialogFragment.show()` commits a fragment + /// transaction, and androidx requires that on the main thread — committing from + /// the timer thread crashed the host app. The stored `FragmentManager` can also + /// outlive the Activity it came from, in which case committing throws + /// `IllegalStateException: FragmentManager has been destroyed`. See + /// https://github.com/formbricks/android/issues/43. internal fun showSurvey(id: String) { - if (fragmentManager == null) { - val error = SDKError.fragmentManagerIsNotSet - Logger.e(error) + val manager = fragmentManager + if (manager == null) { + Logger.e(SDKError.fragmentManagerIsNotSet) return } - fragmentManager?.let { - FormbricksFragment.show(it, surveyId = id) + Handler(Looper.getMainLooper()).post { + // The Activity that owned this manager is gone. Showing is impossible, and + // the host has to hand us a live one. + if (manager.isDestroyed) { + Logger.e(SDKError.fragmentManagerIsDestroyed) + return@post + } + + // A commit after onSaveInstanceState throws. Skipping is the right call: + // the host is on its way to the background, so there is nothing to show. + if (manager.isStateSaved) { + Logger.d("Skipping survey $id: the host has already saved its state.") + return@post + } + + try { + FormbricksFragment.show(manager, surveyId = id) + } catch (e: IllegalStateException) { + // Backstop for any remaining commit-time race: failing to show a survey + // must never take the host app down. + Logger.e(RuntimeException("Unable to show survey $id: ${e.message}")) + } } } diff --git a/android/src/main/java/com/formbricks/android/model/error/SDKError.kt b/android/src/main/java/com/formbricks/android/model/error/SDKError.kt index bc32c2b..c3dda9f 100644 --- a/android/src/main/java/com/formbricks/android/model/error/SDKError.kt +++ b/android/src/main/java/com/formbricks/android/model/error/SDKError.kt @@ -8,6 +8,11 @@ object SDKError { val sdkIsNotInitialized = RuntimeException("Formbricks SDK is not initialized") val sdkIsAlreadyInitialized = RuntimeException("Formbricks SDK is already initialized") val fragmentManagerIsNotSet = RuntimeException("The fragment manager is not set.") + val fragmentManagerIsDestroyed = RuntimeException( + "The fragment manager belongs to a destroyed Activity, so the survey cannot be " + + "shown. Call Formbricks.setFragmentManager(supportFragmentManager) from the " + + "Activity that is currently on screen." + ) // Errors related to network and connectivity val connectionIsNotAvailable = RuntimeException("There is no connection.") From c5f7a5070c174df2b86a52ca606c4394bc6266c9 Mon Sep 17 00:00:00 2001 From: pandeymangg Date: Wed, 5 Aug 2026 18:09:37 +0530 Subject: [PATCH 2/3] feat: support survey-interaction segment filters (ENG-1275) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports the client half of the web SDK change for interaction-based segment filters ("have seen X", "have completed X", ...). Membership for those filters is computed server-side and can flip the moment a contact interacts with a survey, so the SDK now refetches user state instead of waiting for it to expire. - Decode the new per-survey `interactionRefresh` gate from the workspace-state payload. The flags are nullable on purpose: Gson does not run Kotlin default-value initialisers, so a partial object from the server would otherwise leave a non-nullable Boolean undefined. - Add the missing `onFinished` bridge event. The surveys library has always exposed the callback, but it was never passed in, so "have completed X" had no client-side trigger. Because we pass `getSetIsResponseSendingFinished`, this fires only after the finished response is accepted by the backend. - Refresh user state after a display, response or finish, gated twice: no-op for anonymous users, and no-op unless the server flagged that survey and event. Routed through UpdateQueue so a display -> response -> finish burst is debounced into one request. The one-shot guard lives in SurveyInteractionForwarder so it is testable outside the fragment. - Give UpdateQueue an in-flight join and a lock. Nothing serialised its state, and two concurrent POST /user calls could race so the later response would overwrite segments, displays and responses wholesale. - Make the debounce timer one-shot. It was a repeating timer that cancelled itself from inside its own task, reading the shared `timer` field from the timer thread — so a newer timer scheduled in the meantime could be cancelled instead, silently dropping that update. Also hardens the user-state sync timer, which this feature depends on: `startSyncTimer` was only ever reached from a successful sync, so a launch that found a warm cache never scheduled a refresh and segments stayed frozen for the whole process. It is now also armed when the cached state is still valid. The delay is computed relative to now and clamped, because a device clock running ahead of the server puts every `expiresAt` in the past, which java.util.Timer runs immediately and would loop. A pending sync is cancelled on logout and the task re-checks the user id, so a logged-out user is never re-synced. --- ...urveyInteractionRefreshInstrumentedTest.kt | 297 ++++++++++++++++++ .../WebAppInterfaceInstrumentedTest.kt | 9 + .../android/manager/SurveyManager.kt | 14 + .../formbricks/android/manager/UserManager.kt | 79 ++++- .../android/model/javascript/EventType.kt | 1 + .../model/workspace/InteractionRefresh.kt | 47 +++ .../android/model/workspace/Survey.kt | 5 +- .../android/network/queue/UpdateQueue.kt | 76 ++++- .../android/webview/FormbricksFragment.kt | 19 ++ .../android/webview/FormbricksViewModel.kt | 8 + .../webview/SurveyInteractionForwarder.kt | 24 ++ .../android/webview/WebAppInterface.kt | 2 + 12 files changed, 565 insertions(+), 16 deletions(-) create mode 100644 android/src/androidTest/java/com/formbricks/android/manager/SurveyInteractionRefreshInstrumentedTest.kt create mode 100644 android/src/main/java/com/formbricks/android/model/workspace/InteractionRefresh.kt create mode 100644 android/src/main/java/com/formbricks/android/webview/SurveyInteractionForwarder.kt diff --git a/android/src/androidTest/java/com/formbricks/android/manager/SurveyInteractionRefreshInstrumentedTest.kt b/android/src/androidTest/java/com/formbricks/android/manager/SurveyInteractionRefreshInstrumentedTest.kt new file mode 100644 index 0000000..de8748c --- /dev/null +++ b/android/src/androidTest/java/com/formbricks/android/manager/SurveyInteractionRefreshInstrumentedTest.kt @@ -0,0 +1,297 @@ +package com.formbricks.android.manager + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.formbricks.android.model.workspace.InteractionRefresh +import com.formbricks.android.model.workspace.InteractionSource +import com.formbricks.android.model.workspace.Settings +import com.formbricks.android.model.workspace.Survey +import com.formbricks.android.model.workspace.WorkspaceData +import com.formbricks.android.model.workspace.WorkspaceDataHolder +import com.formbricks.android.model.workspace.WorkspaceResponseData +import com.formbricks.android.network.queue.UpdateQueue +import com.formbricks.android.webview.SurveyInteractionForwarder +import com.google.gson.Gson +import org.junit.After +import org.junit.Assert.* +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Covers the delivery path for interaction-based segment refreshes: + * bridge event -> [SurveyInteractionForwarder] -> [SurveyManager.onSurveyInteraction] -> + * [UserManager.refreshSegmentsAfterInteraction] -> [UpdateQueue]. + * + * The observation point is [UpdateQueue]'s private `userId`, which + * `requestUserStateRefresh` sets. That keeps the assertions deterministic — no debounce + * waits and no network — while still proving the whole chain is connected. + */ +@RunWith(AndroidJUnit4::class) +class SurveyInteractionRefreshInstrumentedTest { + + private val gson = Gson() + + @Before + fun setup() { + UpdateQueue.reset() + setUserId(null) + setBackingWorkspaceDataHolder(null) + } + + @After + fun tearDown() { + UpdateQueue.reset() + setUserId(null) + setBackingWorkspaceDataHolder(null) + } + + // MARK: - Decoding + + @Test + fun surveyWithoutInteractionRefreshDecodesToNull() { + val survey = gson.fromJson("""{"id":"survey-a"}""", Survey::class.java) + assertNull(survey.interactionRefresh) + } + + @Test + fun surveyDecodesFullInteractionRefresh() { + val json = """{"id":"survey-a","interactionRefresh":{"onDisplay":true,"onResponse":false,"onFinished":true}}""" + val refresh = gson.fromJson(json, Survey::class.java).interactionRefresh + assertNotNull(refresh) + assertTrue(refresh!!.shouldRefresh(InteractionSource.ON_DISPLAY)) + assertFalse(refresh.shouldRefresh(InteractionSource.ON_RESPONSE)) + assertTrue(refresh.shouldRefresh(InteractionSource.ON_FINISHED)) + } + + /** + * A partial object must not fail or leave a flag undefined. Gson does not run Kotlin + * default-value initialisers, which is why the flags are nullable. + */ + @Test + fun partialInteractionRefreshTreatsMissingKeysAsFalse() { + val json = """{"id":"survey-a","interactionRefresh":{"onDisplay":true}}""" + val refresh = gson.fromJson(json, Survey::class.java).interactionRefresh + assertNotNull(refresh) + assertTrue(refresh!!.shouldRefresh(InteractionSource.ON_DISPLAY)) + assertFalse(refresh.shouldRefresh(InteractionSource.ON_RESPONSE)) + assertFalse(refresh.shouldRefresh(InteractionSource.ON_FINISHED)) + } + + /** The server attaches an all-false object to every survey in such a workspace. */ + @Test + fun allFalseInteractionRefreshIsPresentButNeverRefreshes() { + val json = """{"id":"survey-a","interactionRefresh":{"onDisplay":false,"onResponse":false,"onFinished":false}}""" + val refresh = gson.fromJson(json, Survey::class.java).interactionRefresh + assertNotNull(refresh) + InteractionSource.entries.forEach { assertFalse(refresh!!.shouldRefresh(it)) } + } + + @Test + fun unknownKeyInsideInteractionRefreshIsIgnored() { + val json = """{"id":"survey-a","interactionRefresh":{"onDisplay":true,"onSomethingNew":true}}""" + val refresh = gson.fromJson(json, Survey::class.java).interactionRefresh + assertTrue(refresh!!.shouldRefresh(InteractionSource.ON_DISPLAY)) + } + + // MARK: - The gate + + @Test + fun anonymousUserNeverRefreshes() { + seedWorkspace(survey("survey-a", InteractionRefresh(onDisplay = true))) + setUserId(null) + + SurveyManager.onSurveyInteraction("survey-a", InteractionSource.ON_DISPLAY) + + assertNull(queuedUserId()) + } + + @Test + fun absentInteractionRefreshDoesNotRefresh() { + seedWorkspace(survey("survey-a", null)) + setUserId("user-1") + + SurveyManager.onSurveyInteraction("survey-a", InteractionSource.ON_DISPLAY) + + assertNull(queuedUserId()) + } + + @Test + fun allFalseFlagsDoNotRefresh() { + seedWorkspace(survey("survey-a", InteractionRefresh())) + setUserId("user-1") + + SurveyManager.onSurveyInteraction("survey-a", InteractionSource.ON_DISPLAY) + + assertNull(queuedUserId()) + } + + @Test + fun mismatchedSourceDoesNotRefresh() { + seedWorkspace(survey("survey-a", InteractionRefresh(onDisplay = true))) + setUserId("user-1") + + SurveyManager.onSurveyInteraction("survey-a", InteractionSource.ON_RESPONSE) + + assertNull(queuedUserId()) + } + + @Test + fun matchingFlagRefreshes() { + seedWorkspace(survey("survey-a", InteractionRefresh(onFinished = true))) + setUserId("user-1") + + SurveyManager.onSurveyInteraction("survey-a", InteractionSource.ON_FINISHED) + + assertEquals("user-1", queuedUserId()) + } + + // MARK: - Survey lookup + + /** + * The survey id must actually be matched. If the lookup degraded to "just take the first + * survey", an unknown id would wrongly consult another survey's flags. + */ + @Test + fun unknownSurveyIdDoesNotRefresh() { + seedWorkspace(survey("survey-a", InteractionRefresh(onFinished = true))) + setUserId("user-1") + + SurveyManager.onSurveyInteraction("no-such-survey", InteractionSource.ON_FINISHED) + + assertNull(queuedUserId()) + } + + /** The correct survey's flags are consulted, not the first one in the list. */ + @Test + fun theMatchingSurveysFlagsAreUsed() { + seedWorkspace( + survey("survey-a", InteractionRefresh(onFinished = false)), + survey("survey-b", InteractionRefresh(onFinished = true)) + ) + setUserId("user-1") + + SurveyManager.onSurveyInteraction("survey-b", InteractionSource.ON_FINISHED) + assertEquals("user-1", queuedUserId()) + + UpdateQueue.reset() + SurveyManager.onSurveyInteraction("survey-a", InteractionSource.ON_FINISHED) + assertNull(queuedUserId()) + } + + @Test + fun nullSurveyIdDoesNotRefresh() { + seedWorkspace(survey("survey-a", InteractionRefresh(onFinished = true))) + setUserId("user-1") + + SurveyManager.onSurveyInteraction(null, InteractionSource.ON_FINISHED) + + assertNull(queuedUserId()) + } + + // MARK: - One-shot forwarding + + @Test + fun forwarderRefreshesOncePerSource() { + seedWorkspace(survey("survey-a", InteractionRefresh(onFinished = true, onDisplay = true))) + setUserId("user-1") + val forwarder = SurveyInteractionForwarder() + + forwarder.refreshOnce("survey-a", InteractionSource.ON_FINISHED) + assertEquals("user-1", queuedUserId()) + + // A repeat of the same source must not reach the queue again. + UpdateQueue.reset() + forwarder.refreshOnce("survey-a", InteractionSource.ON_FINISHED) + assertNull(queuedUserId()) + + // A different source still gets through. + forwarder.refreshOnce("survey-a", InteractionSource.ON_DISPLAY) + assertEquals("user-1", queuedUserId()) + } + + @Test + fun aNewShowingCanRefreshAgain() { + seedWorkspace(survey("survey-a", InteractionRefresh(onFinished = true))) + setUserId("user-1") + + SurveyInteractionForwarder().refreshOnce("survey-a", InteractionSource.ON_FINISHED) + assertEquals("user-1", queuedUserId()) + + UpdateQueue.reset() + SurveyInteractionForwarder().refreshOnce("survey-a", InteractionSource.ON_FINISHED) + assertEquals("user-1", queuedUserId()) + } + + // MARK: - Helpers + + private fun survey(id: String, refresh: InteractionRefresh?) = Survey( + id = id, + triggers = null, + recontactDays = null, + displayLimit = null, + delay = null, + displayPercentage = null, + displayOption = null, + segment = null, + styling = null, + languages = null, + projectOverwrites = null, + interactionRefresh = refresh + ) + + private fun seedWorkspace(vararg surveys: Survey) { + val holder = WorkspaceDataHolder( + data = WorkspaceResponseData( + data = WorkspaceData( + surveys = surveys.toList(), + actionClasses = null, + settings = emptySettings() + ), + expiresAt = null + ), + originalResponseMap = emptyMap() + ) + setBackingWorkspaceDataHolder(holder) + } + + private fun emptySettings() = Settings( + id = null, + recontactDays = null, + clickOutsideClose = null, + overlay = null, + placement = null, + inAppSurveyBranding = null, + styling = null + ) + + private fun queuedUserId(): String? { + val field = UpdateQueue::class.java.getDeclaredField("userId") + field.isAccessible = true + return field.get(UpdateQueue) as String? + } + + /** + * Sets the in-memory user id *and* the persisted one, because the getter falls back to + * SharedPreferences when the backing field is null — so clearing only the field would let a + * value written by another test leak in. + */ + private fun setUserId(value: String?) { + val field = UserManager::class.java.getDeclaredField("backingUserId") + field.isAccessible = true + field.set(UserManager, value) + + val prefs = InstrumentationRegistry.getInstrumentation().targetContext + .getSharedPreferences("formbricks_prefs", android.content.Context.MODE_PRIVATE) + prefs.edit().apply { + if (value == null) remove("userIdKey") else putString("userIdKey", value) + commit() + } + } + + private fun setBackingWorkspaceDataHolder(value: WorkspaceDataHolder?) { + val field = SurveyManager::class.java.getDeclaredField("backingWorkspaceDataHolder") + field.isAccessible = true + field.set(SurveyManager, value) + } +} diff --git a/android/src/androidTest/java/com/formbricks/android/webview/WebAppInterfaceInstrumentedTest.kt b/android/src/androidTest/java/com/formbricks/android/webview/WebAppInterfaceInstrumentedTest.kt index 38b3646..7687669 100644 --- a/android/src/androidTest/java/com/formbricks/android/webview/WebAppInterfaceInstrumentedTest.kt +++ b/android/src/androidTest/java/com/formbricks/android/webview/WebAppInterfaceInstrumentedTest.kt @@ -17,11 +17,13 @@ class WebAppInterfaceInstrumentedTest { var closed = false var displayCreated = false var responseCreated = false + var finished = false var filePick: FileUploadData? = null var surveyLibraryLoadError = false override fun onClose() { closed = true } override fun onDisplayCreated() { displayCreated = true } override fun onResponseCreated() { responseCreated = true } + override fun onFinished() { finished = true } override fun onFilePick(data: FileUploadData) { filePick = data } override fun onSurveyLibraryLoadError() { surveyLibraryLoadError = true } } @@ -53,6 +55,13 @@ class WebAppInterfaceInstrumentedTest { assertTrue(callback.responseCreated) } + @Test + fun testMessage_onFinished() { + val json = "{\"event\":\"onFinished\"}" + webAppInterface.message(json) + assertTrue(callback.finished) + } + @Test fun testMessage_onFilePick() { val json = "{\"event\":\"onFilePick\",\"fileUploadParams\":{\"allowedFileExtensions\":\"jpg\",\"allowMultipleFiles\":true}}" diff --git a/android/src/main/java/com/formbricks/android/manager/SurveyManager.kt b/android/src/main/java/com/formbricks/android/manager/SurveyManager.kt index fcb7e15..da3269f 100644 --- a/android/src/main/java/com/formbricks/android/manager/SurveyManager.kt +++ b/android/src/main/java/com/formbricks/android/manager/SurveyManager.kt @@ -7,6 +7,7 @@ import com.formbricks.android.extensions.expiresAt import com.formbricks.android.extensions.guard import com.formbricks.android.logger.Logger import com.formbricks.android.model.workspace.WorkspaceDataHolder +import com.formbricks.android.model.workspace.InteractionSource import com.formbricks.android.model.workspace.Segment import com.formbricks.android.model.workspace.SegmentDeserializer import com.formbricks.android.model.workspace.Survey @@ -254,6 +255,19 @@ object SurveyManager { UserManager.onDisplay(id) } + /** + * Forwards an in-survey interaction so the user manager can pull fresh `segments` when the + * server flagged this survey/source pair as able to change segment membership. + */ + fun onSurveyInteraction(surveyId: String?, source: InteractionSource) { + // Plain null checks rather than `guard`: its fallback path is + // `T::class.java.newInstance()`, which would throw for a data class like `Survey`. + val id = surveyId ?: return + val survey = workspaceDataHolder?.data?.data?.surveys?.firstOrNull { it.id == id } ?: return + + UserManager.refreshSegmentsAfterInteraction(survey, source) + } + /** * Starts a timer to refresh the workspace state after the given timeout [expiresAt]. */ diff --git a/android/src/main/java/com/formbricks/android/manager/UserManager.kt b/android/src/main/java/com/formbricks/android/manager/UserManager.kt index 57e3f44..2aa9559 100644 --- a/android/src/main/java/com/formbricks/android/manager/UserManager.kt +++ b/android/src/main/java/com/formbricks/android/manager/UserManager.kt @@ -11,6 +11,8 @@ import com.formbricks.android.logger.Logger import com.formbricks.android.model.error.SDKError import com.formbricks.android.model.user.AttributeValue import com.formbricks.android.model.user.Display +import com.formbricks.android.model.workspace.InteractionSource +import com.formbricks.android.model.workspace.Survey import com.formbricks.android.network.queue.UpdateQueue import com.google.gson.Gson import kotlinx.coroutines.CoroutineScope @@ -32,6 +34,12 @@ object UserManager { private const val RESPONSES_KEY = "responsesKey" private const val LAST_DISPLAYED_AT_KEY = "lastDisplayedAtKey" private const val EXPIRES_AT_KEY = "expiresAtKey" + /** + * Floor for the gap between two user-state syncs. Guards against a device clock running + * ahead of the server, where every `expiresAt` the server returns is already in the + * device's past and an unclamped timer would sync in a tight loop. + */ + internal var MINIMUM_SYNC_INTERVAL_MS: Long = 60_000 private val prefManager by lazy { Formbricks.applicationContext.getSharedPreferences(FORMBROCKS_PERFS, Context.MODE_PRIVATE) } private var backingUserId: String? = null @@ -42,6 +50,8 @@ object UserManager { private var backingLastDisplayedAt: Date? = null private var backingExpiresAt: Date? = null internal val syncTimer = Timer() + /** The pending expiry-driven sync, so it can be replaced or cancelled on logout. */ + private var syncTask: TimerTask? = null /** * Starts an update queue with the given user id. @@ -106,6 +116,30 @@ object UserManager { SurveyManager.filterSurveys() } + /** + * Pulls fresh server-computed `segments` after an interaction that can flip segment + * membership, instead of waiting for the state to expire. + * + * A `surveyInteraction` segment filter ("have seen X", "have completed X", ...) can change + * who a contact is the moment they interact with a survey. The local bookkeeping in + * [onDisplay] / [onResponse] keeps display caps and recontact days correct on device, but + * segment membership is only ever computed by the server, so it has to be refetched. + * + * The refresh is deliberately gated twice, because a `/user` sync is not cheap: + * - no-op for anonymous users, who never receive segments in the first place; + * - no-op unless the server set the bit for this survey and this event. + * + * It is routed through the [UpdateQueue] rather than calling [syncUser] directly, so a + * display -> response -> finish burst is debounced into a single request. + */ + fun refreshSegmentsAfterInteraction(survey: Survey, source: InteractionSource) { + val id = userId ?: return + if (survey.interactionRefresh?.shouldRefresh(source) != true) return + + Logger.d("Refreshing segments after ${source.value} on survey ${survey.id}") + UpdateQueue.requestUserStateRefresh(id) + } + /** * Syncs the user state with the server if the user id is set and the expiration date has passed. */ @@ -122,6 +156,11 @@ object UserManager { backingSegments = null backingDisplays = null backingResponses = null + + // The state is still valid, but nothing has been scheduled to refresh it when it + // does expire — `startSyncTimer` is otherwise only reached from a successful sync, + // so a launch that finds a warm cache would never refresh segments again. + startSyncTimer() } } @@ -162,6 +201,9 @@ object UserManager { SurveyManager.filterSurveys() startSyncTimer() } catch (e: Exception) { + // Release the in-flight lock so a later refresh nudge isn't swallowed. + // `UpdateQueue.reset()` already does this on the success path. + UpdateQueue.syncDidFinish() val error = SDKError.unableToPostResponse Logger.e(error) } @@ -195,18 +237,45 @@ object UserManager { Formbricks.language = "default" UpdateQueue.reset() + // Drop any pending expiry-driven sync; its captured user id is gone. + syncTask?.cancel() + syncTask = null + SurveyManager.filterSurveys() } + /** + * Schedules the next user-state sync for when the cached state expires. + * + * Two things this guards against: + * - A device clock running ahead of the server makes every `expiresAt` we receive land in + * the device's past. [Timer.schedule] runs a past-dated task immediately, which would + * sync, get another past-dated expiry, and loop. Hence the delay floor. + * - [syncTimer] is a single shared [Timer]; once cancelled it throws on every later + * `schedule`. Catch that rather than tearing down the SDK. + */ private fun startSyncTimer() { - val expiresAt = expiresAt.guard { return } - val userId = userId.guard { return } - syncTimer.schedule(object: TimerTask() { + val expiresAt = expiresAt ?: return + val id = userId ?: return + + val delay = (expiresAt.time - System.currentTimeMillis()) + .coerceAtLeast(MINIMUM_SYNC_INTERVAL_MS) + + syncTask?.cancel() + val task = object : TimerTask() { override fun run() { - syncUser(userId) + // The user may have been logged out or swapped while this was pending. + if (userId != id) return + syncUser(id) } + } + syncTask = task - }, expiresAt) + try { + syncTimer.schedule(task, delay) + } catch (e: IllegalStateException) { + Logger.d("User state sync timer is no longer schedulable: ${e.message}") + } } diff --git a/android/src/main/java/com/formbricks/android/model/javascript/EventType.kt b/android/src/main/java/com/formbricks/android/model/javascript/EventType.kt index accc7aa..a9bb3d4 100644 --- a/android/src/main/java/com/formbricks/android/model/javascript/EventType.kt +++ b/android/src/main/java/com/formbricks/android/model/javascript/EventType.kt @@ -6,6 +6,7 @@ enum class EventType { @SerializedName("onClose") ON_CLOSE, @SerializedName("onDisplayCreated") ON_DISPLAY_CREATED, @SerializedName("onResponseCreated") ON_RESPONSE_CREATED, + @SerializedName("onFinished") ON_FINISHED, @SerializedName("onFilePick") ON_FILE_PICK, @SerializedName("onSurveyLibraryLoadError") ON_SURVEY_LIBRARY_LOAD_ERROR } \ No newline at end of file diff --git a/android/src/main/java/com/formbricks/android/model/workspace/InteractionRefresh.kt b/android/src/main/java/com/formbricks/android/model/workspace/InteractionRefresh.kt new file mode 100644 index 0000000..37242d8 --- /dev/null +++ b/android/src/main/java/com/formbricks/android/model/workspace/InteractionRefresh.kt @@ -0,0 +1,47 @@ +package com.formbricks.android.model.workspace + +import com.google.gson.annotations.SerializedName +import kotlinx.serialization.Serializable + +/** + * The three survey-lifecycle moments that can flip interaction-based segment membership. + * The names match the source names used by the JS SDK so both platforms key the gate off + * the same vocabulary. + */ +enum class InteractionSource(val value: String) { + ON_DISPLAY("onDisplay"), + ON_RESPONSE("onResponse"), + ON_FINISHED("onFinished") +} + +/** + * Per-survey gate for the post-interaction segment refresh. + * + * Each flag says whether interacting with *this* survey via that event can change some live + * survey's segment membership — e.g. a survey referenced only by a "have seen" filter + * refreshes on display but not on response or finish, and a survey no interaction filter + * points at never refreshes at all. + * + * The client API attaches this only for workspaces that use survey-interaction targeting, so + * it is absent for everyone else, and present-but-all-false for surveys in such a workspace + * that no interaction filter references. + * + * The flags are nullable on purpose: Gson does not run Kotlin default-value initialisers, so + * a partial object from the server would otherwise leave a non-nullable `Boolean` in an + * undefined state. Nullable plus `== true` treats anything missing as "do not refresh". + */ +@Serializable +data class InteractionRefresh( + @SerializedName("onDisplay") val onDisplay: Boolean? = null, + @SerializedName("onResponse") val onResponse: Boolean? = null, + @SerializedName("onFinished") val onFinished: Boolean? = null +) { + /** Whether an interaction of this kind should trigger a user-state refresh. */ + fun shouldRefresh(source: InteractionSource): Boolean { + return when (source) { + InteractionSource.ON_DISPLAY -> onDisplay == true + InteractionSource.ON_RESPONSE -> onResponse == true + InteractionSource.ON_FINISHED -> onFinished == true + } + } +} diff --git a/android/src/main/java/com/formbricks/android/model/workspace/Survey.kt b/android/src/main/java/com/formbricks/android/model/workspace/Survey.kt index 4b42314..afeb7fe 100644 --- a/android/src/main/java/com/formbricks/android/model/workspace/Survey.kt +++ b/android/src/main/java/com/formbricks/android/model/workspace/Survey.kt @@ -34,7 +34,10 @@ data class Survey( @SerializedName("segment") val segment: Segment?, @SerializedName("styling") val styling: Styling?, @SerializedName("languages") val languages: List?, - @SerializedName("projectOverwrites") val projectOverwrites: SurveyProjectOverwrites? = null + @SerializedName("projectOverwrites") val projectOverwrites: SurveyProjectOverwrites? = null, + // Whether interacting with this survey can change some live survey's segment + // membership. Absent unless the workspace uses survey-interaction targeting. + @SerializedName("interactionRefresh") val interactionRefresh: InteractionRefresh? = null ) /// Defines the overlay style displayed behind a survey modal. diff --git a/android/src/main/java/com/formbricks/android/network/queue/UpdateQueue.kt b/android/src/main/java/com/formbricks/android/network/queue/UpdateQueue.kt index 2005cdb..96ebff5 100644 --- a/android/src/main/java/com/formbricks/android/network/queue/UpdateQueue.kt +++ b/android/src/main/java/com/formbricks/android/network/queue/UpdateQueue.kt @@ -15,11 +15,20 @@ import kotlin.concurrent.timer object UpdateQueue { private const val DEBOUNCE_INTERVAL: Long = 500 // 500 ms + private val lock = Any() + private var userId: String? = null private var attributes: MutableMap? = null private var language: String? = null private var timer: Timer? = null + /** + * True while a commit-triggered sync is airborne. A repeat nudge joins that request instead + * of starting a second one: two concurrent `POST /user` calls would race and whichever + * response landed last would overwrite `segments` / `displays` / `responses` wholesale. + */ + private var isSyncInFlight = false + fun setUserId(userId: String) { this.userId = userId startDebounceTimer() @@ -50,30 +59,77 @@ object UpdateQueue { } } + /** + * Asks for the user state to be re-read from the server. Carries no new data — it exists so + * an interaction that can change segment membership doesn't have to wait for the state to + * expire. Dropped while a sync is already in flight, because that sync's response already + * brings fresh segments. + */ + fun requestUserStateRefresh(userId: String) { + synchronized(lock) { + if (isSyncInFlight) { + Logger.d("UpdateQueue - refresh skipped, a sync is already in flight") + return + } + this.userId = userId + } + startDebounceTimer() + } + + /** Called by the user manager once a sync finishes, so the next nudge can start a request. */ + fun syncDidFinish() { + synchronized(lock) { + isSyncInFlight = false + } + } + fun reset() { - userId = null - attributes = null - language = null + synchronized(lock) { + userId = null + attributes = null + language = null + isSyncInFlight = false + } } private fun startDebounceTimer() { - timer?.cancel() - timer = timer("debounceTimer", false, DEBOUNCE_INTERVAL, DEBOUNCE_INTERVAL) { - commit() + synchronized(lock) { timer?.cancel() + // One-shot rather than the previous repeating timer that cancelled itself from + // inside its own task: that read the shared `timer` field from the timer thread, so + // a newer timer scheduled in the meantime could be cancelled instead of this one. + val newTimer = Timer("debounceTimer", false) + timer = newTimer + newTimer.schedule(object : TimerTask() { + override fun run() { + commit() + } + }, DEBOUNCE_INTERVAL) } } private fun commit() { - val effectiveUserId = userId - ?: UserManager.userId + val effectiveUserId: String? + val effectiveAttributes: Map? + + // Capture a consistent snapshot, and only mark a sync in flight when one is actually + // about to be sent — otherwise a commit with no user id would leave the flag stuck and + // swallow every later refresh nudge. + synchronized(lock) { + effectiveUserId = userId ?: UserManager.userId + effectiveAttributes = attributes?.toMap() + if (effectiveUserId != null) { + isSyncInFlight = true + } + } + if (effectiveUserId == null) { val error = SDKError.noUserIdSetError Logger.e(error) return } - Logger.d("UpdateQueue - commit() called on UpdateQueue with $effectiveUserId and $attributes") - UserManager.syncUser(effectiveUserId, attributes) + Logger.d("UpdateQueue - commit() called on UpdateQueue with $effectiveUserId and $effectiveAttributes") + UserManager.syncUser(effectiveUserId, effectiveAttributes) } } diff --git a/android/src/main/java/com/formbricks/android/webview/FormbricksFragment.kt b/android/src/main/java/com/formbricks/android/webview/FormbricksFragment.kt index ae95e57..9d4ce56 100644 --- a/android/src/main/java/com/formbricks/android/webview/FormbricksFragment.kt +++ b/android/src/main/java/com/formbricks/android/webview/FormbricksFragment.kt @@ -31,6 +31,7 @@ import com.formbricks.android.logger.Logger import com.formbricks.android.manager.SurveyManager import com.formbricks.android.model.error.SDKError import com.formbricks.android.model.javascript.FileUploadData +import com.formbricks.android.model.workspace.InteractionSource import com.google.android.material.bottomsheet.BottomSheetBehavior import com.google.android.material.bottomsheet.BottomSheetDialogFragment import com.google.gson.JsonObject @@ -43,6 +44,13 @@ class FormbricksFragment : BottomSheetDialogFragment() { private val viewModel: FormbricksViewModel by viewModels() private var isDismissing = false + /** Scoped to this showing, so each interaction refreshes segments at most once. */ + private val interactionForwarder = SurveyInteractionForwarder() + + private fun refreshSegmentsOnce(source: InteractionSource) { + interactionForwarder.refreshOnce(surveyId, source) + } + private var webAppInterface = WebAppInterface(object : WebAppInterface.WebAppCallback { override fun onClose() { Handler(Looper.getMainLooper()).post { @@ -57,6 +65,7 @@ class FormbricksFragment : BottomSheetDialogFragment() { val error = SDKError.couldNotCreateDisplayError Logger.e(error) } + refreshSegmentsOnce(InteractionSource.ON_DISPLAY) } override fun onResponseCreated() { @@ -66,6 +75,16 @@ class FormbricksFragment : BottomSheetDialogFragment() { val error = SDKError.couldNotCreateResponseError Logger.e(error) } + refreshSegmentsOnce(InteractionSource.ON_RESPONSE) + } + + /** + * Fires when the survey is completed and the finished response has been accepted by + * the backend. Only used to refresh interaction-based segments — the sheet is still + * dismissed by [onClose]. + */ + override fun onFinished() { + refreshSegmentsOnce(InteractionSource.ON_FINISHED) } override fun onFilePick(data: FileUploadData) { diff --git a/android/src/main/java/com/formbricks/android/webview/FormbricksViewModel.kt b/android/src/main/java/com/formbricks/android/webview/FormbricksViewModel.kt index 21bba5e..092207a 100644 --- a/android/src/main/java/com/formbricks/android/webview/FormbricksViewModel.kt +++ b/android/src/main/java/com/formbricks/android/webview/FormbricksViewModel.kt @@ -57,6 +57,13 @@ class FormbricksViewModel : ViewModel() { FormbricksJavascript.message(JSON.stringify({ event: "onResponseCreated" })); }; + // Fires once the finished response has been accepted by the backend — the + // surveys library gates this on `isResponseSendingFinished`, and we supply + // `getSetIsResponseSendingFinished` below, so it starts out false. + function onFinished() { + FormbricksJavascript.message(JSON.stringify({ event: "onFinished" })); + }; + let setResponseFinished = null; function getSetIsResponseSendingFinished(callback) { setResponseFinished = callback; @@ -69,6 +76,7 @@ class FormbricksViewModel : ViewModel() { getSetIsResponseSendingFinished, onDisplayCreated, onResponseCreated, + onFinished, onClose, }; diff --git a/android/src/main/java/com/formbricks/android/webview/SurveyInteractionForwarder.kt b/android/src/main/java/com/formbricks/android/webview/SurveyInteractionForwarder.kt new file mode 100644 index 0000000..102acbb --- /dev/null +++ b/android/src/main/java/com/formbricks/android/webview/SurveyInteractionForwarder.kt @@ -0,0 +1,24 @@ +package com.formbricks.android.webview + +import com.formbricks.android.manager.SurveyManager +import com.formbricks.android.model.workspace.InteractionSource +import java.util.Collections + +/** + * Forwards in-survey interactions to the survey manager at most once per source. + * + * One instance lives per survey showing, so the de-duplication is scoped to that showing. The + * surveys library guards `onResponseCreated` itself, but `onFinished` is not guarded there, and + * a self-hosted server may serve an older bundle — so the refresh is gated on our side too. + * Only the refresh is gated; the existing displays/responses bookkeeping keeps its behaviour. + * + * Bridge callbacks arrive on the WebView's JavaBridge thread, hence the synchronised set. + */ +internal class SurveyInteractionForwarder { + private val refreshedSources = Collections.synchronizedSet(mutableSetOf()) + + fun refreshOnce(surveyId: String, source: InteractionSource) { + if (!refreshedSources.add(source)) return + SurveyManager.onSurveyInteraction(surveyId, source) + } +} diff --git a/android/src/main/java/com/formbricks/android/webview/WebAppInterface.kt b/android/src/main/java/com/formbricks/android/webview/WebAppInterface.kt index e59ab3f..76a4c8b 100644 --- a/android/src/main/java/com/formbricks/android/webview/WebAppInterface.kt +++ b/android/src/main/java/com/formbricks/android/webview/WebAppInterface.kt @@ -14,6 +14,7 @@ class WebAppInterface(private val callback: WebAppCallback?) { fun onClose() fun onDisplayCreated() fun onResponseCreated() + fun onFinished() fun onFilePick(data: FileUploadData) fun onSurveyLibraryLoadError() } @@ -31,6 +32,7 @@ class WebAppInterface(private val callback: WebAppCallback?) { EventType.ON_CLOSE -> callback?.onClose() EventType.ON_DISPLAY_CREATED -> callback?.onDisplayCreated() EventType.ON_RESPONSE_CREATED -> callback?.onResponseCreated() + EventType.ON_FINISHED -> callback?.onFinished() EventType.ON_FILE_PICK -> { callback?.onFilePick(FileUploadData.from(data)) } EventType.ON_SURVEY_LIBRARY_LOAD_ERROR -> { callback?.onSurveyLibraryLoadError() } } From 02b93d9e753c5a11c88853a1fa28c2aa60bcc02d Mon Sep 17 00:00:00 2001 From: pandeymangg Date: Wed, 5 Aug 2026 18:29:42 +0530 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20stale?= =?UTF-8?q?=20FragmentManager,=20lost=20refreshes,=20failure=20retry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Read `fragmentManager` inside the main-thread runnable instead of capturing it at schedule time. The host can hand us a newer one via `setFragmentManager` between the two, and the stale reference would report "destroyed" while a usable manager was available. - Defer and replay a refresh that arrives mid-sync instead of dropping it. The in-flight request was built before that interaction, so its response cannot reflect it — dropping the nudge left segments stale until the next trigger. Only the latest deferred refresh is kept, so many interactions behind a slow sync still cost a single follow-up. `syncDidFinish()` drains it and is now called on the success path too; `reset()` deliberately keeps it, and logout clears it via `clearPendingRefresh()`. - Re-arm the sync when the request fails. The task that fired is spent and `startSyncTimer()` is otherwise only reached from a successful sync, so one transient network error ended the refresh cycle for the whole process. The retry backs off by `RETRY_AFTER_FAILURE_MS` rather than the minimum sync interval, so a sustained outage doesn't become a fixed-rate request stream. Not addressed, and worth its own ticket: `commit()` can still start a request while a sync is in flight, and the expiry task calls `syncUser` directly rather than through the queue. Both predate this PR — before it there was no in-flight flag at all, so every commit raced. Making attribute-carrying commits wait on a sync changes when user data reaches the backend, which needs its own design rather than a drive-by change here. 4 more instrumented tests, bringing the file to 19. --- ...urveyInteractionRefreshInstrumentedTest.kt | 98 +++++++++++++++++-- .../java/com/formbricks/android/Formbricks.kt | 15 +-- .../formbricks/android/manager/UserManager.kt | 39 +++++++- .../android/network/queue/UpdateQueue.kt | 43 ++++++-- 4 files changed, 171 insertions(+), 24 deletions(-) diff --git a/android/src/androidTest/java/com/formbricks/android/manager/SurveyInteractionRefreshInstrumentedTest.kt b/android/src/androidTest/java/com/formbricks/android/manager/SurveyInteractionRefreshInstrumentedTest.kt index de8748c..8e37f06 100644 --- a/android/src/androidTest/java/com/formbricks/android/manager/SurveyInteractionRefreshInstrumentedTest.kt +++ b/android/src/androidTest/java/com/formbricks/android/manager/SurveyInteractionRefreshInstrumentedTest.kt @@ -33,15 +33,18 @@ class SurveyInteractionRefreshInstrumentedTest { private val gson = Gson() @Before - fun setup() { - UpdateQueue.reset() - setUserId(null) - setBackingWorkspaceDataHolder(null) - } + fun setup() = resetAll() @After - fun tearDown() { + fun tearDown() = resetAll() + + /** + * `reset()` deliberately keeps `pendingRefreshUserId` (the sync success path relies on it + * surviving), so tests have to clear it explicitly or one leaks into the next. + */ + private fun resetAll() { UpdateQueue.reset() + UpdateQueue.clearPendingRefresh() setUserId(null) setBackingWorkspaceDataHolder(null) } @@ -223,6 +226,72 @@ class SurveyInteractionRefreshInstrumentedTest { assertEquals("user-1", queuedUserId()) } + // MARK: - In-flight handling + + /** + * A nudge that lands mid-sync must be deferred and then replayed — not dropped. The + * in-flight request was built before that interaction, so its response cannot reflect it. + */ + @Test + fun refreshDuringAnInFlightSyncIsDeferredThenReplayed() { + seedWorkspace(survey("survey-a", InteractionRefresh(onFinished = true))) + setUserId("user-1") + + setSyncInFlight(true) + SurveyManager.onSurveyInteraction("survey-a", InteractionSource.ON_FINISHED) + + // Deferred, so nothing has been queued for sending yet. + assertNull(queuedUserId()) + assertEquals("user-1", pendingRefreshUserId()) + + // Completing the sync replays it on its own, with no further interaction. + UpdateQueue.syncDidFinish() + + assertEquals("user-1", queuedUserId()) + assertNull(pendingRefreshUserId()) + } + + /** Only the latest deferred refresh is kept, so a slow sync costs one follow-up. */ + @Test + fun multipleDeferredRefreshesCollapseIntoOneReplay() { + seedWorkspace(survey("survey-a", InteractionRefresh(onDisplay = true, onFinished = true))) + setUserId("user-1") + + setSyncInFlight(true) + SurveyManager.onSurveyInteraction("survey-a", InteractionSource.ON_DISPLAY) + SurveyManager.onSurveyInteraction("survey-a", InteractionSource.ON_FINISHED) + + assertEquals("user-1", pendingRefreshUserId()) + + UpdateQueue.syncDidFinish() + + assertEquals("user-1", queuedUserId()) + assertNull(pendingRefreshUserId()) + } + + /** Teardown drops the deferred refresh rather than replaying it for a gone user. */ + @Test + fun clearPendingRefreshDropsTheDeferredNudge() { + seedWorkspace(survey("survey-a", InteractionRefresh(onFinished = true))) + setUserId("user-1") + + setSyncInFlight(true) + SurveyManager.onSurveyInteraction("survey-a", InteractionSource.ON_FINISHED) + assertEquals("user-1", pendingRefreshUserId()) + + UpdateQueue.clearPendingRefresh() + UpdateQueue.syncDidFinish() + + assertNull(queuedUserId()) + assertNull(pendingRefreshUserId()) + } + + /** The failure retry backs off past the minimum interval rather than hammering. */ + @Test + fun failureRetryBacksOffFurtherThanTheMinimumInterval() { + assertTrue(UserManager.RETRY_AFTER_FAILURE_MS > UserManager.MINIMUM_SYNC_INTERVAL_MS) + } + // MARK: - Helpers private fun survey(id: String, refresh: InteractionRefresh?) = Survey( @@ -265,10 +334,21 @@ class SurveyInteractionRefreshInstrumentedTest { styling = null ) - private fun queuedUserId(): String? { - val field = UpdateQueue::class.java.getDeclaredField("userId") + private fun queuedUserId(): String? = readQueueField("userId") as String? + + private fun pendingRefreshUserId(): String? = + readQueueField("pendingRefreshUserId") as String? + + private fun setSyncInFlight(value: Boolean) { + val field = UpdateQueue::class.java.getDeclaredField("isSyncInFlight") + field.isAccessible = true + field.set(UpdateQueue, value) + } + + private fun readQueueField(name: String): Any? { + val field = UpdateQueue::class.java.getDeclaredField(name) field.isAccessible = true - return field.get(UpdateQueue) as String? + return field.get(UpdateQueue) } /** diff --git a/android/src/main/java/com/formbricks/android/Formbricks.kt b/android/src/main/java/com/formbricks/android/Formbricks.kt index 27478c7..4560242 100644 --- a/android/src/main/java/com/formbricks/android/Formbricks.kt +++ b/android/src/main/java/com/formbricks/android/Formbricks.kt @@ -332,13 +332,16 @@ object Formbricks { /// `IllegalStateException: FragmentManager has been destroyed`. See /// https://github.com/formbricks/android/issues/43. internal fun showSurvey(id: String) { - val manager = fragmentManager - if (manager == null) { - Logger.e(SDKError.fragmentManagerIsNotSet) - return - } - Handler(Looper.getMainLooper()).post { + // Read the manager here rather than at schedule time: the host may have handed us + // a newer one via `setFragmentManager` in between, and using the stale reference + // would report "destroyed" while a usable manager was available. + val manager = fragmentManager + if (manager == null) { + Logger.e(SDKError.fragmentManagerIsNotSet) + return@post + } + // The Activity that owned this manager is gone. Showing is impossible, and // the host has to hand us a live one. if (manager.isDestroyed) { diff --git a/android/src/main/java/com/formbricks/android/manager/UserManager.kt b/android/src/main/java/com/formbricks/android/manager/UserManager.kt index 2aa9559..3ae683e 100644 --- a/android/src/main/java/com/formbricks/android/manager/UserManager.kt +++ b/android/src/main/java/com/formbricks/android/manager/UserManager.kt @@ -40,6 +40,13 @@ object UserManager { * device's past and an unclamped timer would sync in a tight loop. */ internal var MINIMUM_SYNC_INTERVAL_MS: Long = 60_000 + + /** + * How long to wait before retrying a user-state sync that failed. Deliberately much longer + * than the minimum interval so a sustained outage doesn't become a fixed-rate request + * stream. Mirrors the workspace-state path's error timeout. + */ + internal var RETRY_AFTER_FAILURE_MS: Long = 10 * 60_000 private val prefManager by lazy { Formbricks.applicationContext.getSharedPreferences(FORMBROCKS_PERFS, Context.MODE_PRIVATE) } private var backingUserId: String? = null @@ -198,14 +205,21 @@ object UserManager { } UpdateQueue.reset() + // `reset()` clears the in-flight lock, but only this drains a refresh that + // arrived while the request was out — that interaction happened after this + // response was computed, so it still needs its own sync. + UpdateQueue.syncDidFinish() SurveyManager.filterSurveys() startSyncTimer() } catch (e: Exception) { - // Release the in-flight lock so a later refresh nudge isn't swallowed. - // `UpdateQueue.reset()` already does this on the success path. + // Release the in-flight lock and replay a refresh that arrived mid-sync. UpdateQueue.syncDidFinish() val error = SDKError.unableToPostResponse Logger.e(error) + // Re-arm, otherwise the refresh cycle ends here for the whole process: the + // task that fired is spent and startSyncTimer() is otherwise only reached + // from a successful sync. + scheduleSyncRetry() } } } @@ -237,9 +251,11 @@ object UserManager { Formbricks.language = "default" UpdateQueue.reset() - // Drop any pending expiry-driven sync; its captured user id is gone. + // Drop any pending expiry-driven sync and any deferred refresh; the user they were + // captured for is gone. syncTask?.cancel() syncTask = null + UpdateQueue.clearPendingRefresh() SurveyManager.filterSurveys() } @@ -260,7 +276,23 @@ object UserManager { val delay = (expiresAt.time - System.currentTimeMillis()) .coerceAtLeast(MINIMUM_SYNC_INTERVAL_MS) + scheduleSync(delay, id) + } + + /** + * Re-arms the sync after a failed request. + * + * `expiresAt` still holds the value from the last success, so it is not a usable cadence + * here. Back off well past the minimum sync interval so a sustained outage doesn't turn + * into a fixed-rate request stream. + */ + private fun scheduleSyncRetry() { + val id = userId ?: return + scheduleSync(RETRY_AFTER_FAILURE_MS, id) + } + /** Replaces any pending sync with one scheduled [delay] ms from now. */ + private fun scheduleSync(delay: Long, id: String) { syncTask?.cancel() val task = object : TimerTask() { override fun run() { @@ -274,6 +306,7 @@ object UserManager { try { syncTimer.schedule(task, delay) } catch (e: IllegalStateException) { + // The shared Timer was cancelled (logout/cleanup); nothing more to schedule. Logger.d("User state sync timer is no longer schedulable: ${e.message}") } } diff --git a/android/src/main/java/com/formbricks/android/network/queue/UpdateQueue.kt b/android/src/main/java/com/formbricks/android/network/queue/UpdateQueue.kt index 96ebff5..a192c96 100644 --- a/android/src/main/java/com/formbricks/android/network/queue/UpdateQueue.kt +++ b/android/src/main/java/com/formbricks/android/network/queue/UpdateQueue.kt @@ -29,6 +29,13 @@ object UpdateQueue { */ private var isSyncInFlight = false + /** + * A refresh that arrived while a sync was already airborne, replayed once that sync + * finishes. Only the latest is kept, so many interactions behind a slow sync still cost a + * single follow-up request. + */ + private var pendingRefreshUserId: String? = null + fun setUserId(userId: String) { this.userId = userId startDebounceTimer() @@ -62,13 +69,20 @@ object UpdateQueue { /** * Asks for the user state to be re-read from the server. Carries no new data — it exists so * an interaction that can change segment membership doesn't have to wait for the state to - * expire. Dropped while a sync is already in flight, because that sync's response already - * brings fresh segments. + * expire. + * + * While a sync is airborne the nudge is deferred rather than sent, because two concurrent + * `POST /user` calls would race and the later response would overwrite segments, displays + * and responses wholesale. It is replayed by [syncDidFinish]. */ fun requestUserStateRefresh(userId: String) { synchronized(lock) { if (isSyncInFlight) { - Logger.d("UpdateQueue - refresh skipped, a sync is already in flight") + Logger.d("UpdateQueue - refresh deferred, a sync is already in flight") + // The in-flight request was built before this interaction, so its response + // cannot reflect it. Dropping the nudge would leave segments stale until the + // next trigger. + pendingRefreshUserId = userId return } this.userId = userId @@ -76,11 +90,19 @@ object UpdateQueue { startDebounceTimer() } - /** Called by the user manager once a sync finishes, so the next nudge can start a request. */ + /** + * Called by the user manager once a sync finishes. Releases the in-flight lock and replays a + * refresh that arrived while the request was out. + */ fun syncDidFinish() { - synchronized(lock) { + val deferredUserId = synchronized(lock) { isSyncInFlight = false - } + pendingRefreshUserId.also { pendingRefreshUserId = null } + } ?: return + + Logger.d("UpdateQueue - replaying a refresh that arrived mid-sync") + // Outside the block above: `requestUserStateRefresh` takes the same lock. + requestUserStateRefresh(deferredUserId) } fun reset() { @@ -89,6 +111,15 @@ object UpdateQueue { attributes = null language = null isSyncInFlight = false + // `pendingRefreshUserId` is deliberately kept: reset() runs on the sync success + // path, and syncDidFinish() still has to replay it. + } + } + + /** Teardown, unlike [reset]: drop the deferred refresh instead of replaying it. */ + fun clearPendingRefresh() { + synchronized(lock) { + pendingRefreshUserId = null } }