fix: Race conditions when initialising the RN SDK (#91)

posthog-core/src/index.ts

@@ -556,7 +556,13 @@ export abstract class PostHogCoreStateless {
     clearTimeout(this._flushTimer)
     try {
       await this.flushAsync()
-      await Promise.allSettled(Object.values(this.pendingPromises))
+      await Promise.all(
+        Object.values(this.pendingPromises).map((x) =>
+          x.catch(() => {
+            // ignore errors as we are shutting down and can't deal with them anyways.
+          })
+        )
+      )
     } catch (e) {
       if (!isPostHogFetchError(e)) {
         throw e
@@ -587,13 +593,6 @@ export abstract class PostHogCore extends PostHogCoreStateless {
 
     this.sendFeatureFlagEvent = options?.sendFeatureFlagEvent ?? true
     this._sessionExpirationTimeSeconds = options?.sessionExpirationTimeSeconds ?? 1800 // 30 minutes
-
-    // NOTE: It is important we don't initiate anything in the constructor as some async IO may still be underway on the parent
-    if (options?.preloadFeatureFlags !== false) {
-      safeSetTimeout(() => {
-        this.reloadFeatureFlags()
-      }, 1)
-    }
   }
 
   protected setupBootstrap(options?: Partial<PosthogCoreOptions>): void {
@@ -743,9 +742,7 @@ export abstract class PostHogCore extends PostHogCoreStateless {
       this.setPersistedProperty(PostHogPersistedProperty.AnonymousId, previousDistinctId)
       this.setPersistedProperty(PostHogPersistedProperty.DistinctId, distinctId)
 
-      if (this.getFeatureFlags()) {
-        this.reloadFeatureFlags()
-      }
+      this.reloadFeatureFlags()
     }
 
     super.identifyStateless(distinctId, allProperties, options)
@@ -812,7 +809,7 @@ export abstract class PostHogCore extends PostHogCoreStateless {
       },
     })
 
-    if (Object.keys(groups).find((type) => existingGroups[type] !== groups[type]) && this.getFeatureFlags()) {
+    if (Object.keys(groups).find((type) => existingGroups[type] !== groups[type])) {
       this.reloadFeatureFlags()
     }
 

posthog-core/test/posthog.featureflags.spec.ts

@@ -16,6 +16,7 @@ describe('PostHog Core', () => {
     'json-payload': true,
   })
 
+  // NOTE: Aren't these supposed to be strings?
   const createMockFeatureFlagPayloads = (): any => ({
     'feature-1': {
       color: 'blue',
@@ -99,42 +100,8 @@ describe('PostHog Core', () => {
 
     describe('when loaded', () => {
       beforeEach(() => {
-        jest.runOnlyPendingTimers() // trigger init setImmediate
-      })
-
-      it('should load feature flags on init', async () => {
-        expect(mocks.fetch).toHaveBeenCalledWith('https://app.posthog.com/decide/?v=3', {
-          body: JSON.stringify({
-            token: 'TEST_API_KEY',
-            distinct_id: posthog.getDistinctId(),
-            groups: {},
-            person_properties: {},
-            group_properties: {},
-            $anon_distinct_id: posthog.getAnonymousId(),
-          }),
-          method: 'POST',
-          headers: {
-            'Content-Type': 'application/json',
-          },
-          signal: expect.anything(),
-        })
-
-        expect(posthog.getFeatureFlags()).toEqual({
-          'feature-1': true,
-          'feature-2': true,
-          'json-payload': true,
-          'feature-variant': 'variant',
-        })
-
-        expect(posthog.getFeatureFlagPayloads()).toEqual({
-          'feature-1': {
-            color: 'blue',
-          },
-          'json-payload': {
-            a: 'payload',
-          },
-          'feature-variant': 5,
-        })
+        // The core doesn't reload flags by default (this is handled differently by web and RN)
+        posthog.reloadFeatureFlags()
       })
 
       it('should return the value of a flag', async () => {
@@ -171,7 +138,7 @@ describe('PostHog Core', () => {
             })
           })
 
-          jest.runOnlyPendingTimers() // trigger init setImmediate
+          posthog.reloadFeatureFlags()
         })
 
         it('should return undefined', async () => {
@@ -241,7 +208,7 @@ describe('PostHog Core', () => {
               })
           })
 
-          jest.runOnlyPendingTimers() // trigger init setImmediate
+          posthog.reloadFeatureFlags()
         })
 
         it('should return combined results', async () => {
@@ -344,7 +311,7 @@ describe('PostHog Core', () => {
               })
           })
 
-          jest.runOnlyPendingTimers() // trigger init setImmediate
+          posthog.reloadFeatureFlags()
         })
 
         it('should return only latest results', async () => {
@@ -557,7 +524,7 @@ describe('PostHog Core', () => {
 
     describe('when loaded', () => {
       beforeEach(() => {
-        jest.runOnlyPendingTimers() // trigger init setImmediate
+        posthog.reloadFeatureFlags()
       })
 
       it('should load new feature flags', async () => {

posthog-core/test/posthog.groups.spec.ts

@@ -41,7 +41,8 @@ describe('PostHog Core', () => {
     it('should call groupIdentify if including props', () => {
       posthog.group('other', 'team', { foo: 'bar' })
 
-      expect(parseBody(mocks.fetch.mock.calls[0])).toMatchObject({
+      expect(mocks.fetch).toHaveBeenCalledTimes(2) // 1 for decide, 1 for groupIdentify
+      expect(parseBody(mocks.fetch.mock.calls[1])).toMatchObject({
         batch: [
           {
             event: '$groupidentify',

posthog-core/test/posthog.identify.spec.ts

@@ -14,11 +14,12 @@ describe('PostHog Core', () => {
   })
 
   describe('identify', () => {
+    // Identify also triggers a decide call so we should expect 2 calls
     it('should send an $identify event', () => {
       posthog.identify('id-1', { foo: 'bar' })
 
-      expect(mocks.fetch).toHaveBeenCalledTimes(1)
-      expect(parseBody(mocks.fetch.mock.calls[0])).toEqual({
+      expect(mocks.fetch).toHaveBeenCalledTimes(2)
+      expect(parseBody(mocks.fetch.mock.calls[1])).toEqual({
         api_key: 'TEST_API_KEY',
         batch: [
           {
@@ -47,8 +48,8 @@ describe('PostHog Core', () => {
     it('should include anonymous ID if set', () => {
       posthog.identify('id-1', { foo: 'bar' })
 
-      expect(mocks.fetch).toHaveBeenCalledTimes(1)
-      expect(parseBody(mocks.fetch.mock.calls[0])).toMatchObject({
+      expect(mocks.fetch).toHaveBeenCalledTimes(2)
+      expect(parseBody(mocks.fetch.mock.calls[1])).toMatchObject({
         batch: [
           {
             distinct_id: posthog.getDistinctId(),
@@ -74,8 +75,8 @@ describe('PostHog Core', () => {
       posthog.identify('id-1', { foo: 'bar' })
       // One call exists for the queueing, one for persisting distinct id
       expect(mocks.storage.setItem).toHaveBeenCalledWith('distinct_id', 'id-1')
-      expect(mocks.fetch).toHaveBeenCalledTimes(1)
-      expect(parseBody(mocks.fetch.mock.calls[0])).toMatchObject({
+      expect(mocks.fetch).toHaveBeenCalledTimes(2) // Once for reload flags, once for identify
+      expect(parseBody(mocks.fetch.mock.calls[1])).toMatchObject({
         batch: [
           {
             distinct_id: 'id-1',

posthog-node/test/posthog-node.spec.ts

@@ -210,7 +210,6 @@ describe('PostHog Node.js', () => {
         host: 'http://example.com',
         disableGeoip: false,
       })
-      client.debug()
       client.capture({ distinctId: '123', event: 'test-event', properties: { foo: 'bar' }, groups: { org: 123 } })
 
       jest.runOnlyPendingTimers()
@@ -296,7 +295,7 @@ describe('PostHog Node.js', () => {
         flushAt: 1,
       })
 
-      const logSpy = jest.spyOn(global.console, 'log')
+      const logSpy = jest.spyOn(global.console, 'log').mockImplementation(() => {})
       jest.useRealTimers()
       // using debug mode to check console.log output
       // which tells us when the flush is complete

posthog-react-native/CHANGELOG.md

@@ -1,3 +1,9 @@
+# 2.7.0 - 2023-04-20
+
+1. Fixes a race condition that could occur when initialising PostHog
+2. Fixes an issue where feature flags would not be reloaded after a reset
+3. PostHog should always be initialized via .initAsync and will now warn if this is not the case
+
 # 2.6.0 - 2023-04-19
 
 1. Some small fixes to incorrect types

posthog-react-native/package.json

@@ -1,6 +1,6 @@
 {
   "name": "posthog-react-native",
-  "version": "2.6.0",
+  "version": "2.7.0",
   "main": "lib/posthog-react-native/index.js",
   "files": [
     "lib/"

posthog-react-native/src/posthog-rn.ts

@@ -24,73 +24,101 @@ export type PostHogOptions = PosthogCoreOptions & {
   customAsyncStorage?: PostHogCustomAsyncStorage
 }
 
+const clientMap = new Map<string, Promise<PostHog>>()
+
 export class PostHog extends PostHogCore {
   private _persistence: PostHogOptions['persistence']
   private _memoryStorage = new PostHogMemoryStorage()
-  private _semiAsyncStorage: SemiAsyncStorage
+  private _semiAsyncStorage?: SemiAsyncStorage
   private _appProperties: PostHogCustomAppProperties = {}
 
+  static _resetClientCache(): void {
+    // NOTE: this method is intended for testing purposes only
+    clientMap.clear()
+  }
+
   /** Await this method to ensure that all state has been loaded from the async provider */
   static async initAsync(apiKey: string, options?: PostHogOptions): Promise<PostHog> {
-    const storage = new SemiAsyncStorage(options?.customAsyncStorage || buildOptimisiticAsyncStorage())
-    const posthog = new PostHog(apiKey, options, storage)
-    await posthog._semiAsyncStorage.preloadAsync()
+    let posthog = clientMap.get(apiKey)
+
+    if (!posthog) {
+      const persistence = options?.persistence ?? 'file'
+      if (persistence === 'file') {
+        const storage = new SemiAsyncStorage(options?.customAsyncStorage || buildOptimisiticAsyncStorage())
+        posthog = storage.preloadAsync().then(() => new PostHog(apiKey, options, storage))
+      } else {
+        posthog = Promise.resolve(new PostHog(apiKey, options))
+      }
+
+      clientMap.set(apiKey, posthog)
+    } else {
+      console.warn('PostHog.initAsync called twice with the same apiKey. The first instance will be used.')
+    }
+
     return posthog
   }
 
   constructor(apiKey: string, options?: PostHogOptions, storage?: SemiAsyncStorage) {
     super(apiKey, options)
-    this._persistence = options?.persistence
+    this._persistence = options?.persistence ?? 'file'
 
     // Either build the app properties from the existing ones
     this._appProperties =
       typeof options?.customAppProperties === 'function'
         ? options.customAppProperties(getAppProperties())
         : options?.customAppProperties || getAppProperties()
 
-    this._semiAsyncStorage =
-      storage || new SemiAsyncStorage(options?.customAsyncStorage || buildOptimisiticAsyncStorage())
-
     AppState.addEventListener('change', () => {
       this.flush()
     })
 
+    if (this._persistence === 'file') {
+      if (!storage) {
+        console.warn(
+          'PostHog was initialised without using PostHog.initAsync - this can lead to race condition issues.'
+        )
+      }
+
+      this._semiAsyncStorage =
+        storage || new SemiAsyncStorage(options?.customAsyncStorage || buildOptimisiticAsyncStorage())
+    }
+
     // Ensure the async storage has been preloaded (this call is cached)
 
-    const setupFromStorage = (): void => {
+    const setupAsync = async (): Promise<void> => {
+      if (!this._semiAsyncStorage?.isPreloaded) {
+        await this._semiAsyncStorage?.preloadAsync()
+      }
       this.setupBootstrap(options)
 
       // It is possible that the old library was used so we try to get the legacy distinctID
-      if (!this._semiAsyncStorage.getItem(PostHogPersistedProperty.AnonymousId)) {
-        getLegacyValues().then((legacyValues) => {
-          if (legacyValues?.distinctId) {
-            this._semiAsyncStorage.setItem(PostHogPersistedProperty.DistinctId, legacyValues.distinctId)
-            this._semiAsyncStorage.setItem(PostHogPersistedProperty.AnonymousId, legacyValues.anonymousId)
-          }
-        })
+      if (!this._semiAsyncStorage?.getItem(PostHogPersistedProperty.AnonymousId)) {
+        const legacyValues = await getLegacyValues()
+        if (legacyValues?.distinctId) {
+          this._semiAsyncStorage?.setItem(PostHogPersistedProperty.DistinctId, legacyValues.distinctId)
+          this._semiAsyncStorage?.setItem(PostHogPersistedProperty.AnonymousId, legacyValues.anonymousId)
+        }
       }
-    }
 
-    if (this._semiAsyncStorage.isPreloaded) {
-      setupFromStorage()
-    } else {
-      void this._semiAsyncStorage.preloadAsync().then(() => {
-        setupFromStorage()
-      })
+      if (options?.preloadFeatureFlags !== false) {
+        this.reloadFeatureFlags()
+      }
     }
+
+    void setupAsync()
   }
 
   getPersistedProperty<T>(key: PostHogPersistedProperty): T | undefined {
-    if (this._persistence === 'memory') {
-      return this._memoryStorage.getProperty(key)
-    }
-    return this._semiAsyncStorage.getItem(key) || undefined
+    return this._semiAsyncStorage
+      ? this._semiAsyncStorage.getItem(key) || undefined
+      : this._memoryStorage.getProperty(key)
   }
   setPersistedProperty<T>(key: PostHogPersistedProperty, value: T | null): void {
-    if (this._persistence === 'memory') {
-      return this._memoryStorage.setProperty(key, value)
-    }
-    return value !== null ? this._semiAsyncStorage.setItem(key, value) : this._semiAsyncStorage.removeItem(key)
+    return this._semiAsyncStorage
+      ? value !== null
+        ? this._semiAsyncStorage.setItem(key, value)
+        : this._semiAsyncStorage.removeItem(key)
+      : this._memoryStorage.setProperty(key, value)
   }
 
   fetch(url: string, options: PostHogFetchOptions): Promise<PostHogFetchResponse> {

posthog-react-native/src/storage.ts

@@ -17,26 +17,35 @@ export class SemiAsyncStorage {
   }
 
   preloadAsync(): Promise<void> {
+    if (this.isPreloaded) {
+      return Promise.resolve()
+    }
+
     if (this._preloadSemiAsyncStoragePromise) {
       return this._preloadSemiAsyncStoragePromise
     }
+
     this._preloadSemiAsyncStoragePromise = this._asyncStorage.getItem(POSTHOG_STORAGE_KEY).then((res) => {
       try {
         const data = res ? JSON.parse(res).content : {}
 
         for (const key in data) {
           this._memoryCache[key] = data[key]
         }
-
-        this.isPreloaded = true
       } catch (e) {
         console.warn(
           "PostHog failed to load persisted data from storage. This is likely because the storage format is. We'll reset the storage.",
           e
         )
+      } finally {
+        this.isPreloaded = true
       }
     })
 
+    this._preloadSemiAsyncStoragePromise.finally(() => {
+      this._preloadSemiAsyncStoragePromise = undefined
+    })
+
     return this._preloadSemiAsyncStoragePromise
   }