chore: buffer track/identify/screen calls made before initialize completes#700
Draft
mahmoud-elmorabea wants to merge 10 commits into
Draft
chore: buffer track/identify/screen calls made before initialize completes#700mahmoud-elmorabea wants to merge 10 commits into
mahmoud-elmorabea wants to merge 10 commits into
Conversation
…etes Replace the IllegalStateException thrown by CustomerIO.instance() pre-init with a bounded FIFO buffer (capacity 100, drop-most-recent on overflow) that absorbs event-shaped public-API calls and replays them in order against the real CustomerIO once initialize() completes. CustomerIO.instance() now returns DataPipelineInstance (super of CustomerIO) so the same accessor can return either the buffering proxy or the real instance. Callers that need CustomerIO-specific members (e.g. analytics, moduleConfig) use the new CustomerIO.realInstance() opt-in API. Closes a silent-data-loss / crash class of bugs that affected cold-start and deep-link flows where track/identify/screen would fire before the SDK had finished initializing. Previously those calls crashed with IllegalStateException; they now buffer and drain. This is the Android side of a coordinated fix; iOS sibling lands as customerio-ios#1054. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sample app builds 📱Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request. |
Move kotlinx.coroutines.test.* imports below java.* etc. to match the ktlint lexicographic ordering enforced in CI.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #700 +/- ##
============================================
- Coverage 69.07% 68.07% -1.00%
- Complexity 838 887 +49
============================================
Files 149 156 +7
Lines 4601 4824 +223
Branches 628 673 +45
============================================
+ Hits 3178 3284 +106
- Misses 1189 1282 +93
- Partials 234 258 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
Build available to test |
📏 SDK Binary Size Comparison Report
|
|
CustomerIO.instance() now returns DataPipelineInstance instead of CustomerIO, allowing the same accessor to return the new pre-init buffering proxy or the real instance. Companion-class signature updated to match.
|
|
… breakage Revert the breaking change to CustomerIO.instance() return type. Existing callers continue to receive CustomerIO and the IllegalStateException behavior on uninitialized state is restored. Add a new companion accessor CustomerIO.safeInstance() that returns a DataPipelineInstance — either the real CustomerIO if initialized, or the PreInitBufferingInstance proxy if not. The buffer mechanism remains identical; only the surface for callers wanting non-throwing access has moved from instance() to safeInstance(). This keeps the public API surface strictly additive: - CustomerIO.instance(): CustomerIO — unchanged - CustomerIO.safeInstance(): DataPipelineInstance — new Wrapper SDKs (React Native, Flutter) and host-app code that fires events during cold-start should call safeInstance() instead of instance() to benefit from the pre-init buffer. That migration ships separately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
|
|
Customers get the buffer transparently on CustomerIO.instance().X(...) —
matching the iOS sibling's behavior. No new accessor, no migration
needed for wrappers or host apps.
How:
- CustomerIO's private constructor is now no-arg. The heavyweight setup
(analytics, plugins, event-bus subscriptions, identity broadcast) moves
from init {} into a synchronized initializeComponents() method called
from Companion.initialize() exactly once.
- Runtime state (analytics, moduleConfig, globalPreferenceStore,
deviceStore, contextPlugin) is lateinit and gated by an
isReady/componentsReady predicate.
- Each event-shaped *Impl method (track, identify, screen,
clearIdentify, setProfileAttributes, setDeviceAttributes,
registerDeviceToken, deleteDeviceToken, trackMetric) checks isReady
and either executes against analytics directly or enqueues onto
the shared PreInitEventBuffer.
- Read-side properties (userId, anonymousId, registeredDeviceToken,
isUserIdentified, profileAttributes/deviceAttributes getters) return
safe defaults (null / empty string / empty map) pre-init.
- Companion.instance() is now lazy and never throws — it returns the
singleton CustomerIO whether or not initialize() has run.
- PreInitBufferingInstance is removed (no longer needed: CustomerIO is
its own buffering proxy).
API surface diff:
- CustomerIO.instance() return type unchanged: CustomerIO.
- safeInstance() removed (the prior interim approach is no longer needed).
- The synthetic constructor for default-args drops out because the
primary constructor no longer has any parameters. The constructor was
private — no external caller could ever have invoked it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
Adds per-event debug logs so production diagnostics can distinguish "buffer never received the event" from "buffer received but lost it": - On every accept: "Pre-init event buffer accepted event (buffered count: N)." - On every drop: "Pre-init event buffer is at capacity (100); dropping event. Total dropped this session: N." - On transition: "Pre-init event buffer transitioned to ready. Drained N event(s) (dropped due to capacity this session: M)." The transition log fires unconditionally on initialize() so customers can confirm the drain step ran even when zero events were buffered. All debug-level so quiet by default; surfaced when log level is bumped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
mahmoud-elmorabea
changed the title
5 tasks
…rdering Addresses PR #700 review feedback. - `CustomerIO.finishInitialization()` now publishes the initial `UserChangedEvent` and drains the pre-init buffer **after** every module has run its `initialize()`. This means MessagingInApp (and any other late-subscribing module) attaches before the identity event is published, so a screen-heavy pre-init buffer can no longer evict it from the SharedFlow replay window. - `isReady` now sources from `preInitBuffer.isReady` instead of the old `componentsReady` flag, so the readiness signal flips atomically with the drain completing. Public setters that don't synchronize via `DataPipelineInstance` (`setProfileAttributes`, `setDeviceAttributes`) therefore route through the buffer for the entire pre-init and drain window, preserving FIFO order with earlier buffered calls. - Buffered closures now invoke private `*Internal` methods (identifyInternal, trackInternal, screenInternal, …) that contain the bodies without the readiness gate, avoiding recursive re-enqueue during replay. The double- init guard moved to a separate `initStarted` flag latched at the start of `initializeComponents()`. - `PreInitEventBuffer.enqueue` now enforces capacity + drop accounting in the `Draining` state as well as `Buffering`. Mirrors the iOS fix; covered by a new `drainingStateEnforcesCapacity` unit test. - New `FinishInitializationOrderTest` verifies that a late-subscribing module's `subscribe(UserChangedEvent)` happens before the initial publish, and that buffered screen events drain after subscription. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
|
|
||
| override fun setProfileAttributes(attributes: CustomAttributes) { | ||
| val identifier = this.userId | ||
| if (!isReady) { |
Contributor
Author
There was a problem hiding this comment.
Let's create a dispatch helper like in customerio/customerio-ios#1054 to avoid repeating that check everywhere
|
|
||
| override val isUserIdentified: Boolean | ||
| get() = !analytics.userId().isNullOrEmpty() | ||
| get() = if (isReady) !analytics.userId().isNullOrEmpty() else false |
Contributor
Author
There was a problem hiding this comment.
These property access calls shouldn't silently return default values
| data class Ready(val impl: DataPipelineInstance) : State() | ||
| } | ||
|
|
||
| private val lock = ReentrantLock() |
Contributor
Author
There was a problem hiding this comment.
Consider using kotlin's synchronized on this object instead
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…d-side property access; swap ReentrantLock for kotlin synchronized Addresses PR #700 bugbot + reviewer feedback. - Extract a `dispatch` helper that wraps the `if (!isReady) preInitBuffer.enqueue { … } else …Internal(…)` pattern used by every event-shaped public method (`setProfileAttributes`, `setDeviceAttributes`, `identifyImpl`, `trackImpl`, `screenImpl`, `clearIdentifyImpl`, `registerDeviceTokenImpl`, `deleteDeviceTokenImpl`, `trackMetricImpl`). De-duplicates nine copies and removes the risk of a new method forgetting the readiness guard. - The helper holds `synchronized(this)` across the readiness check and dispatch, restoring the locking boundary that the pre-refactor `setProfileAttributes -> this.identify(...)` chain used to provide via `DataPipelineInstance`'s `synchronized { identifyImpl(...) }` wrapper. The lock is reentrant, so `*Impl` overrides invoked through the parent's synchronized helper simply re-enter without contention. - Read-side properties (`registeredDeviceToken`, `anonymousId`, `userId`, `isUserIdentified`) now emit a debug log when accessed before the SDK is ready instead of returning their safe default silently. The contract (returning a default) is preserved so callers can still probe the singleton pre-init, but the misuse surfaces in logcat. - `PreInitEventBuffer` switches from `java.util.concurrent.locks .ReentrantLock` + `withLock { … }` to a private `Any()` monitor with kotlin `synchronized(lock) { … }`. We never used the ReentrantLock- specific features (tryLock, conditions, interruptibility), and the built-in monitor is more idiomatic for this use case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
…e-init race Addresses follow-up PR #700 bugbot finding. The companion `initialize(config)` previously short-circuited on `if (customerIO.isReady)` only. Because `isReady` flips at the very end of initialization (`finishInitialization()`), two threads concurrently calling `initialize(...)` could both pass `createInstance` (synchronized on its own), both observe `isReady == false`, and both proceed to register modules + call `finishInitialization()` — double-running `module.initialize()` for every registered module. That double-run adds duplicate analytics plugins and duplicate EventBus subscribers, and publishes the initial `UserChangedEvent` twice. - Add `@Synchronized` to the companion `initialize(config)` so concurrent callers serialize on the companion monitor. By the time the second caller acquires the lock, the first has finished `finishInitialization()` and `isReady` is `true` — the existing early-out then handles the duplicate cleanly. - Update the `initStarted` field comment to reflect that it's the `initializeComponents` re-entry guard, not the companion-level guard. Includes a new `CompanionInitializeIdempotencyTest` that invokes `CustomerIO.initialize(...)` twice with the same counting module and asserts the module's `initialize()` was called exactly once. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d1addf2. Configure here.
…ck param Addresses follow-up PR #700 bugbot findings. Two related cleanups on the pre-init buffer surface: 1. Post-init event hot path no longer hits the buffer's internal lock. Every post-init `track` / `identify` / `screen` / etc. call goes through `dispatch`, which previously read `preInitBuffer.isReady` — a property that synchronized on the buffer's monitor to inspect its state. With the new `@Volatile sdkReady` field on `CustomerIO`, `dispatch` reads a plain volatile boolean instead. The companion `finishInitialization()` flips `sdkReady` to `true` *after* the drain completes, so concurrent callers during the drain window still see `false` and route through the buffer (preserving FIFO order with earlier buffered calls). `sdkReady` is a one-way latch, which makes the relaxed volatile read safe. 2. `Block` is simplified from `(DataPipelineInstance) -> Unit` to `() -> Unit`. Every closure enqueued by `dispatch` already captures its target via lexical scope (`this`); the impl argument the buffer threaded through `State.Ready` / `State.Draining` / `EnqueueOutcome.ExecuteNow` / `transitionToReady(impl)` was dead plumbing. Removing it shrinks the buffer's surface area and matches the way the rest of the SDK actually uses it. The buffer state machine no longer holds a reference to the SDK instance at all. Tests are updated to capture the test `instance` via lexical scope instead of consuming the dropped `DataPipelineInstance` parameter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
hollyschilling-cio
suggested changes
May 29, 2026
hollyschilling-cio
left a comment
hollyschilling-cio
left a comment
Contributor
There was a problem hiding this comment.
This doesn't serve the same purpose as it does on iOS. Specifically, on Android the initialization is entirely synchronous. Given the default installation suggested on the Quick Start Guide, this only collects events that occur before the SDK initialization begins. No Activity can be created before that point.
This also introduces potential issues. If the hosting app spawns a thread before init and that thread creates events rapidly, the draining process will hang indefinitely causing the hosting app to trigger ANR on startup. Additionally, this suppresses the previous IllegalStateException that was triggered if an event was submitted without the SDK being initialized. That means that if the SDK initialization gets removed from the app's startup sequence, there is no error to indicate the problem to the developer and events simply don't make it to the backend.
On iOS, disk operations are handled asynchronously, which necessitates an async startup. That is not the case on Android. Adding this does not solve any existing pain points on Android.
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
2 participants
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
CustomerIO.instance()are now absorbed into a bounded FIFO (capacity 100, drop-most-recent on overflow) and replayed in order onceCustomerIO.initialize()completes.CustomerIO.instance().track(...), wrapper SDK bridges, host-app cold-start code — benefit automatically. The legacyIllegalStateExceptionthrown by pre-initinstance()is gone.This is item 2 of a cross-SDK plan to port targeted improvements from the proposed iOS SDK rewrite back to the current native SDKs.
Public API — unchanged
CustomerIO.instance(): CustomerIOinitialize()has run. Never throws.CustomerIO.initialize(config)api/datapipelines.apidiff: removes onepublic syntheticconstructor (the compiler artifact for the now-no-arg primary constructor — the constructor was alwaysprivate, so no external caller could have invoked it).How it works
CustomerIO's private primary constructor is now no-arg. The heavyweight setup (Analytics instance, plugins, event-bus subscriptions, identity broadcast) moved frominit {}into asynchronized initializeComponents()method called fromCompanion.createInstance()exactly once.analytics,moduleConfig,globalPreferenceStore,deviceStore,contextPlugin) islateinitand gated by anisReady/componentsReadypredicate.*Implmethod checksisReady. When ready, calls execute againstanalyticsdirectly. When not ready, calls enqueue onto aPreInitEventBuffer(ReentrantLock-backed FIFO).initialize()the buffer drains synchronously against the now-ready singleton before any later caller observes the ready state — so the first post-init call cannot race ahead of buffered ones.userId,anonymousId,registeredDeviceToken,isUserIdentified,profileAttributes/deviceAttributesgetters) return safe defaults (null/ empty string / empty map) pre-init.Scope of buffering
All event-shaped methods on
DataPipelineInstancebuffer when called pre-init:track,identify,screen(incl. deprecated typed overloads)clearIdentify,setProfileAttributes,setDeviceAttributesregisterDeviceToken,deleteDeviceToken,trackMetricDecisions worth flagging
initialize()ever called → buffer sits at cap. No TTL, no cleanup. Memory is bounded.[identify(A), track(x), clearIdentify, identify(B), track(y)]drains in that exact order.Test plan
PreInitEventBufferTest.kt: state transitions, overflow, drain order, reentrancy, concurrent enqueue, post-init direct execution, idempotent transition.datapipelinesunit tests pass.messaginginapp,messagingpush,locationunit tests pass.initialize()track()call) — done locally, no PRs.🤖 Generated with Claude Code
Note
Medium Risk
Changes the core
CustomerIOsingleton initialization and dispatch path, which can affect event ordering, concurrency, and identity/device-token handling during app startup.Overview
Adds pre-initialization buffering for event-shaped SDK calls.
CustomerIO.instance()is now lazily created and no longer throws pre-init; calls liketrack/identify/screen(and related setters/token/metric APIs) enqueue into a bounded FIFO (PreInitEventBuffer, cap 100, drop-most-recent) and replay in order once initialization completes.Refactors initialization lifecycle and ordering. Heavy setup moves into
initializeComponents(),initialize()is synchronized and idempotent, andfinishInitialization()now publishes the initialUserChangedEventafter all modules have initialized and then drains the buffer; read-side accessors return safe defaults (and log debug) until ready. Adds unit tests covering buffer behavior, init idempotency, and module-subscription/event ordering; API snapshot updates by removing a synthetic constructor.Reviewed by Cursor Bugbot for commit cdd1c8d. Bugbot is set up for automated code reviews on this repo. Configure here.