Skip to content
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.

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

Advertising Id not being sent for Android Application Installed events using analytics-react-native-plugin-advertising-id #995

Open
maurispalletti opened this issue Sep 3, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@maurispalletti
Copy link

maurispalletti commented Sep 3, 2024

Updated issue:

There is an issue in the analytics-react-native-plugin-advertising-id (https://github.com/segmentio/analytics-react-native/tree/master/packages/plugins/plugin-advertising-id) where its functioning defers from the Android native implementation.
When the the event Application Installed is triggered, it is missing the advertisingId value.

Why is this happening?

It seems like at this point, where plugins are being added in the forEeach, and since at least AdvertisingIdPlugin is async, the manual flush from this.store.pendingEvents including the event Application Installed is being executed before finishing adding all plugins.

private async onReady() {
// Add all plugins awaiting store
if (this.pluginsToAdd.length > 0 && !this.isAddingPlugins) {
this.isAddingPlugins = true;
try {
// start by adding the plugins
this.pluginsToAdd.forEach((plugin) => {
this.addPlugin(plugin);
});
// now that they're all added, clear the cache
// this prevents this block running for every update
this.pluginsToAdd = [];
} finally {
this.isAddingPlugins = false;
}
}
// Start flush policies
// This should be done before any pending events are added to the queue so that any policies that rely on events queued can trigger accordingly
this.setupFlushPolicies();
// Send all events in the queue
const pending = await this.store.pendingEvents.get(true);
for (const e of pending) {
await this.startTimelineProcessing(e);
await this.store.pendingEvents.remove(e);
}
this.flushPolicyExecuter.manualFlush();
}

So, the current process follows this sequence:

  1. this.checkInstalledVersion() is being called here, where Application Installed event is created internally. However, since this.isReady.value is not yet set to true, the event is added to a store of pending events here.
  2. Then await this.onReady() is being called, during which the plugins are being added. Inside a forEach, this.addPlugin(plugin) is executed for each plugin. In the case of the plugin AdvertisingIdPlugin it should be done async since the plugin itself is async. And not synchronously as it's done in the forEeach here.
  3. Finally, the list of pending events is being processed and a manual flush is performed to trigger them here.

So there may be a case where AdvertisingIdPlugin has not been added yet, including its context with an advertisingId and pending events like Application Installed were flushed, resulting in these first events being sent without its corresponding advertisingId.

Proposed fix

Replace:

try {
// start by adding the plugins
this.pluginsToAdd.forEach((plugin) => {
this.addPlugin(plugin);
});
// now that they're all added, clear the cache
// this prevents this block running for every update
this.pluginsToAdd = [];
} finally {
this.isAddingPlugins = false;
}

by:

  try {
    // start by adding the plugins
    for (const plugin of this.pluginsToAdd) {
      await this.addPlugin(plugin);
    }
    // now that they're all added, clear the cache
    // this prevents this block running for every update
    this.pluginsToAdd = [];
  } finally {
    this.isAddingPlugins = false;
  }

Details

  • analytics-react-native version: 2.17.0
  • Integrations versions (if used): analytics-react-native-plugin-advertising-id: 1.3.1
  • React Native version: 0.71.18
  • iOS or Android or both? Android

Steps to reproduce

  • Check Application Installed events on Segment.

Expected behavior

  • All Application Installed events from Android should have advertisingId.

Actual behavior

  • No Application Installed events from Android actually have advertisingId.
@maurispalletti maurispalletti added the bug Something isn't working label Sep 3, 2024
@cgadam
Copy link

cgadam commented Sep 4, 2024

Thanks for the analysis @maurispalletti ! Yes, it seems that configure in the plugin is of async nature given that clearly it needs to communicate with the native layer. That part can be seen here:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants