Duplicate analytics client exception is raised after having reset the client

[lynfogeek]

I am facing an issue trying to instantiate an Analytics instance, with (possibly) a writeKey which was previously used.

Once I call analytics.reset(), I expect to be able to build a new instance of Analytics with the previously used writeKey, without raising an exception.
Instead, this exception is raised:

analytics-android/analytics/src/main/java/com/segment/analytics/Analytics.java

Lines 1294 to 1298 in 639eedf

	throw new IllegalStateException(
	"Duplicate analytics client created with tag: "
	+ tag
	+ ". If you want to use multiple Analytics clients, use a different writeKey "
	+ "or set a tag via the builder during construction.");

The same use of the "analytics-ios" SDK works.

Could you consider a way to better reset the internals of the framework? or reuse an existing INSTANCE silently? The current APIs don't seem to allow me to do this.

[f2prateek]

What you're looking for is analytics.shutdown(). Reset will only clear local storage, it's meant for cases like user logging out of your app and a new user logs in.

This works in analytics iOS because it's a library that doesn't support multiple writeKeys (at least not properly) at the moment.

[xiaogegexiao]

Hi @f2prateek I'm having the same issue here.

I'm using
analytics = new Analytics.Builder(appContext, Environment.SEGMENT_WRITE_KEY) .trackApplicationLifecycleEvents() .build(); Analytics.setSingletonInstance(analytics);
When app user logged in.

And when the app user logged out. I called these
Analytics.with(appContext).flush(); Analytics.with(appContext).reset();

And then after new user logged in, I tried to call the same code as above. I crashed and said

Duplicate analytics client created with tag:

I'm wondering is reset enough, or I should also call shutdown
Thanks

[f2prateek]

Yes, you need to call shutdown() if you are going to create a new Analytics object instance.

[xiaogegexiao]

Yeah, I used shutdown() after the user logged out.
But unfortunately, it came with error

UnsupportedOperationException: Default singleton instance cannot be shutdown.

I think it is because of I used

Analytics.setSingletonInstance(analytics);

But, if I don't use setSingletonInstance, am I able to log user information. I did this according to https://segment.com/docs/sources/mobile/android/

I guess I don't need to set the global singleton necessarily, as long as I hold the analytics instance reference?

[f2prateek]

Yes, you can't call shutdown the singleton instance.

You should only create new instances of the Analytics object if you are using multiple writeKeys.

It doesn't sound like you need multiple write keys for your use case. Hence I suggest you implement it as the following:

1. in Application.onCreate

Analytics analytics = new Analytics.Builder(appContext, Environment.SEGMENT_WRITE_KEY) .trackApplicationLifecycleEvents() .build(); Analytics.setSingletonInstance(analytics);

2. When user logs out

analytics.reset();

This way you don't need to worry about calling shutdown or creating multiple instances, and can still use the global instance.

[sergiomarqmoura]

This issue makes it hard to integrate the SDK in an Android library, since you don't have an Application class to call the shutdown() on. Definitely ignoring the instance in case one using the same tag is already stored would be a better option for these non-application use cases

[f2prateek]

Hmm... I'm not sure I follow. If you're a library then you shouldn't be using the singleton instance anyway as that should be reserved for the host application.

[sergiomarqmoura]

That's right, but even without setting the singleton instance, apparently it's still possible to get the duplicate analytics client exception

[f2prateek]

Yes - the exception is to remind you to not create multiple instances with the same key.

You should pass around the analytics object in your library wherever you need to use it rather than creating a new one on demand. Call shutdown on this instance when you're done with it so if you do need to create a new one, the previous one is shut down.

[iadcg]

How do you solve this in the react native part?
Is this enough?

try {
  segment.setup(Config.SEGMENT_WRITE_KEY, {
    // Record screen views automatically!
    recordScreenViews: true,
    // Record certain application events automatically!
    trackAppLifecycleEvents: true,
  });
}
catch (e) {
 console.error(e.message);
}

[sergiomarqmoura]

You should pass around the analytics object in your library wherever you need to use it rather than creating a new one on demand. Call shutdown on this instance when you're done with it so if you do need to create a new one, the previous one is shut down.

@f2prateek Being a library, and assuming we have no singleton-related conflicts with the host app (I solved them in a different way), this is what I'm trying to do, and I'm still getting the exception of multiple singletons:

@Provides
    open fun provideAnalyticsApi(context: Context): Analytics {
        try {
            val analytics = Analytics.Builder(context, KEY).build()
            Analytics.setSingletonInstance(analytics)
            return analytics
        } catch (e: IllegalStateException) {
            Analytics.with(context).reset()
            return provideAnalyticsApi(context)
        }
    }

I would expect this code to always return me an Analytics object with KEY. However, apparently the reset() is not clearing the singleton (I read the source code). Also, you cannot call setSingletonInstance(null) when a singleton is already defined. The reason I need the singleton is so that I can call Analytics.with(context) to try to reset it in case we see multiple initialisations.

So in my opinion, either the reset() call should set singleton = null or the setSingletonInstance(null) should be allowed.

Finally, your suggestion of passing the object around wherever needed doesn't seem very nice from the point of view of an Android library with many screens where each one of them must receive this object. It forces us to create interfaces to pass the object around in between each 2 screens, and almost impossible to use together with a dependency injection framework.

[f2prateek]

Finally, your suggestion of passing the object around wherever needed doesn't seem very nice from the point of view of an Android library with many screens where each one of them must receive this object. It forces us to create interfaces to pass the object around in between each 2 screens, and almost impossible to use together with a dependency injection framework

I'm not sure I agree. If you have a dependency injector, you shouldn't need to use our singleton provider and all - e.g. with Dagger you would generally do something like this in the application level module:

@Provides @Singleton Analytics provideAnalytics(Context context) {
    return return new Analytics.Builder(context, KEY).build();
 }

You can check https://github.com/JakeWharton/u2020 as an example of this. Specifically here's an example of module implementation that creates singletons that can be used across throughout the application.

I would expect this code to always return me an Analytics object with KEY. However, apparently the reset() is not clearing the singleton (I read the source code). Also, you cannot call setSingletonInstance(null) when a singleton is already defined. The reason I need the singleton is so that I can call Analytics.with(context) to try to reset it in case we see multiple initializations. So in my opinion, either the reset() call should set singleton = null or the setSingletonInstance(null) should be allowed.

I think it is a reasonable restriction to enforce that you cannot call call setSingletonInstance multiple times. This is to avoid developers from shooting themselves in the foot, for instance some part of your app could retain a reference to the old instance, and now you suddenly have data going through two different instances. Here's this in a contrived example if this was allowed:

// In a background service.
Analytics.setSingletonInstance(new Analytics.Builder(context, key1)..build());
Analytics first = Analytics.with(context);

// In another background service.
Analytics.setSingletonInstance(null);
Analytics.setSingletonInstance(new Analytics.Builder(context, key2)..build());

Analytics second = Analytics.with(context);

// Now a user could accidentally be sending data to different projects unintentionally

In general, it's an anti pattern to be changing this on the fly. e.g. Picasso v2 had the same restriction when it allowed you to set a custom singleton instance, and in v3 it doesn't allow you to change the singleton at all. However, if you'd still really like this behaviour, you can implement your own singleton helper pretty easily.

class AnalyticsSingleton {
  Analytics analytics;

  public void setSingletonInstance(Analytics analytics) {
    this.analytics = analytics;
  }

  public Analytics get() {
    return analytics;
  }
}