Repository navigation
feat: SDK integration improvements + code-review fixes (U1–U4, P0, P1) - #2
Merged
Merged
Conversation
Add disable()/clearPendingActions() lifecycle control with a 3-state fireAndForget() (disabled→discard, configured→execute, buffered→replay). Cap pre-configure buffer at maxPendingActions (default 250, drops oldest). Add setContext(url, referrer) API that propagates to ContextProvider, captures url/referrer per-PendingAction for accurate per-event attribution on replay, and restores the current context after replay completes. Add flushAsync() for non-suspending, fire-and-forget flush from Java/non-suspend callers. Fix DebugRoutingSettings.enabled=null to auto-detect debug builds via Build.VERSION.CODENAME rather than always defaulting to false. reset() now also clears isDisabled and cached context fields. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ging) - enabled=false + debugToken no longer activates debug endpoint routing; setDebugActive(true) is now gated on enabled != false so the caller's explicit intent is respected - Replace immortal replayScope with newCore.launch so the replay coroutine is bound to the core's lifecycle: dispose() on a second configure() or reset() cancels it cleanly instead of writing into a disposed core - Add .onFailure logging to replay's runCatching so pre-configure event loss is visible in logs instead of being silently swallowed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
P1.4: Replace two @volatile cachedUrl/cachedReferrer fields with a single AtomicReference<Pair<String,String>> so fireAndForget() always captures an atomic url+referrer snapshot with no torn-read risk under concurrency. P1.5: Keep core==null during replay so live events buffer in pendingActions rather than racing with replay's per-action setContext() calls. core is now assigned inside the replay coroutine after all replay events are processed. A second pendingActions drain catches events that arrived during replay. A pendingCore guard ensures reset()/re-configure() can cancel an in-flight replay: pendingCore?.dispose() at the top of configure() and reset() cancels the replay scope, and the if (pendingCore===newCore) check inside the coroutine prevents a late-running replay from clobbering a reset state. P1.6: Add KDoc to ContextProvider.setContext() documenting that custom implementations must override it for per-event context to work; the default is a no-op. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Section 3: replace old contextProvider.setUrl/setReferrer pattern with the simpler SearchCollector.setContext() API; note that pre-configure events capture context at queue time - API Reference: add setContext, flushAsync, disable, clearPendingActions; expand reset() description to mention clearStorage parameter - New "Opt-out / Disable" section documenting disable(), clearPendingActions(), and reset(clearStorage=true) for consent and GDPR flows Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rd pattern Captures the concurrency bug where SearchCollector.configure() activated the new core before the replay coroutine finished mutating the shared contextProvider, causing live events to record stale URLs/referrers from replayed pre-configure events. Documents the three coordinated fixes: - P1.5: pendingCore guard + delayed core activation + late-arrivals drain - P0: bind replay to newCore.scope instead of an immortal CoroutineScope - P1.4: AtomicReference<Pair<String,String>> replacing two @volatile fields Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rudibatt
requested changes
Jul 1, 2026
rudibatt
left a comment
rudibatt
left a comment
Contributor
There was a problem hiding this comment.
Context retrieval and swapping core seems to complex and could be simpler and less error prone from my point of view.
…rovider ContextProvider previously held URL and referrer as mutable state, which meant events read the context at async processing time rather than at call time — causing attribution errors when setContext() was called between events. URL and referrer are now captured atomically in fireAndForget() and passed through block parameters to Core. BrowserInfoProvider (renamed from ContextProvider) only provides static device metadata: user-agent, language, touch. setNavContext() replaces setContext() on the public API. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The method only transformed url/ref/timestamp by wrapping them in a data class — no actual logic. Replace with getSession() that does the real work (sessionStore.touch() + session ID lookup) and set timestamp, url, ref, and channel directly in each event constructor. Removes the CommonProperties intermediate object. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously, calling configure() a second time hard-cancelled the old core's scope, losing any events still in its EventQueue. gracefulDispose() stops the auto-flush timer, flushes the remaining queue on a short-lived detached scope, and only then cancels the core's scope. reset() keeps hard dispose — it is intentionally destructive (logout / GDPR erasure). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rudibatt
self-requested a review
July 3, 2026 08:04
logReplayError() was a Core method called exclusively by SearchCollector's replay logic. SearchCollector now owns the logger directly (filtered by the configured log level) and logs replay errors inline, removing the unnecessary indirection through Core. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ssing time Tracks two events with different nav contexts set between calls, then asserts each event carries the context that was active when the tracking method was invoked — not the context at async processing time. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The context URL/referrer race was a design mistake (reading ambient state at processing time instead of capturing at call time), not a complex concurrency problem worth documenting in detail. The pendingCore guard and scope-binding are the non-obvious patterns worth keeping. Context handling is now a short note explaining the call-time snapshot approach. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rudibatt
approved these changes
Jul 7, 2026
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
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
Changes
SearchCollector:
ContextProvider:
Test plan