Fix segment ga4 bug

.changeset/brave-jokes-train.md

@@ -0,0 +1,6 @@
+---
+'@segment/analytics-core': minor
+---
+
+Do not throw errors in .register() method unless plugin has 'critical' set to true.
+

.changeset/fast-moons-knock.md

@@ -0,0 +1,8 @@
+---
+'@segment/analytics-next': minor
+'@segment/analytics-core': minor
+---
+
+Addresses an issue where, if one of the non-destination actions fails to load/is blocked, the entire SDK fails to load. This is most notable in GA4, where, if GA was blocked, Segment initialization would fail.
+
+In doing so, introduces a critical: true API.

packages/browser/src/browser/__tests__/integration.test.ts

@@ -1048,6 +1048,63 @@ describe('timeout', () => {
     expect(analytics.settings.timeout).toEqual(50)
   })
 })
+describe('register', () => {
+  it('will not invoke any plugins that have initialization errors', async () => {
+    const analytics = AnalyticsBrowser.load({
+      writeKey,
+    })
+
+    const errorPlugin = new ActionDestination('Error Plugin', {
+      ...googleAnalytics,
+      load: () => Promise.reject('foo'),
+    })
+    const errorPluginSpy = jest.spyOn(errorPlugin, 'track')
+
+    const goodPlugin = new ActionDestination('Good Plugin', {
+      ...googleAnalytics,
+      load: () => Promise.resolve('foo'),
+    })
+    const goodPluginSpy = jest.spyOn(goodPlugin, 'track')
+
+    await analytics.register(goodPlugin, errorPlugin)
+    await errorPlugin.ready()
+    await goodPlugin.ready()
+
+    await analytics.track('foo')
+
+    expect(errorPluginSpy).not.toHaveBeenCalled()
+    expect(goodPluginSpy).toHaveBeenCalled()
+  })
+
+  it('will emit initialization errors', async () => {
+    const analytics = AnalyticsBrowser.load({
+      writeKey,
+    })
+
+    const errorPlugin = new ActionDestination('Error Plugin', {
+      ...googleAnalytics,
+      load: () => Promise.reject('foo'),
+    })
+
+    const goodPlugin = new ActionDestination('Good Plugin', {
+      ...googleAnalytics,
+      load: () => Promise.resolve('foo'),
+    })
+
+    await analytics
+    const errorPluginName = new Promise<string>((resolve) => {
+      analytics.instance?.queue.on('initialization_failure', (plugin) =>
+        resolve(plugin.name)
+      )
+    })
+
+    await analytics.register(goodPlugin, errorPlugin)
+    await errorPlugin.ready()
+    await goodPlugin.ready()
+
+    expect(await errorPluginName).toBe('Error Plugin')
+  })
+})
 
 describe('deregister', () => {
   beforeEach(async () => {

packages/browser/src/plugins/env-enrichment/index.ts

@@ -135,6 +135,7 @@ class EnvironmentEnrichmentPlugin implements Plugin {
 
   name = 'Page Enrichment'
   type: PluginType = 'before'
+  critical = true
   version = '0.1.0'
   isLoaded = () => true
   load = async (_ctx: Context, instance: Analytics) => {

packages/browser/src/plugins/schema-filter/index.ts

@@ -77,6 +77,7 @@ export function schemaFilter(
 
   return {
     name: 'Schema Filter',
+    critical: true,
     version: '0.1.0',
     isLoaded: () => true,
     load: () => Promise.resolve(),

packages/core/src/plugins/index.ts

@@ -1,11 +1,6 @@
 import type { CoreAnalytics } from '../analytics'
 import type { CoreContext } from '../context'
 
-interface CorePluginConfig {
-  options: any
-  priority: 'critical' | 'non-critical' // whether AJS should expect this plugin to be loaded before starting event delivery
-}
-
 export type PluginType =
   | 'before'
   | 'after'
@@ -25,12 +20,12 @@ export interface CorePlugin<
   alternativeNames?: string[]
   version: string
   type: PluginType
+  /**
+   * if critical is set to true, will throw an error / abort the entire pipeline if the load() method throws an error.
+   */
+  critical?: boolean
   isLoaded: () => boolean
-  load: (
-    ctx: Ctx,
-    instance: Analytics,
-    config?: CorePluginConfig
-  ) => Promise<unknown>
+  load: (ctx: Ctx, instance: Analytics) => Promise<unknown>
 
   unload?: (ctx: Ctx, instance: Analytics) => Promise<unknown> | unknown
   ready?: () => Promise<unknown>

packages/core/src/queue/__tests__/extension-flushing.test.ts

@@ -53,12 +53,13 @@ describe('Registration', () => {
     expect(load).toHaveBeenCalledWith(ctx, ajs)
   })
 
-  test('fails if plugin cant be loaded', async () => {
+  test('fails if plugin cant be loaded AND is type critical', async () => {
     const eq = new EventQueue()
 
     const plugin: Plugin = {
       name: 'test',
       type: 'before',
+      critical: true,
       version: '0.1.0',
       load: () => Promise.reject(new Error('👻')),
       isLoaded: () => false,

packages/core/src/queue/event-queue.ts

@@ -50,25 +50,37 @@ export abstract class CoreEventQueue<
     plugin: Plugin,
     instance: CoreAnalytics
   ): Promise<void> {
-    if (plugin.type === 'destination' && plugin.name !== 'Segment.io') {
-      plugin.load(ctx, instance).catch((err) => {
-        this.failedInitializations.push(plugin.name)
-        this.emit('initialization_failure', plugin)
-        console.warn(plugin.name, err)
-
-        ctx.log('warn', 'Failed to load destination', {
-          plugin: plugin.name,
-          error: err,
-        })
-
-        // Filter out the failed plugin by excluding it from the list
-        this.plugins = this.plugins.filter((p) => p !== plugin)
+    this.plugins.push(plugin)
+
+    const handleLoadError = (err: any) => {
+      if (plugin.critical) {
+        throw err
+      }
+      this.failedInitializations.push(plugin.name)
+      this.emit('initialization_failure', plugin)
+      console.warn(plugin.name, err)
+
+      ctx.log('warn', 'Failed to load destination', {
+        plugin: plugin.name,
+        error: err,
       })
-    } else {
-      await plugin.load(ctx, instance)
+
+      // Filter out the failed plugin by excluding it from the list
+      this.plugins = this.plugins.filter((p) => p !== plugin)
     }
 
-    this.plugins.push(plugin)
+    if (plugin.type === 'destination' && plugin.name !== 'Segment.io') {
+      plugin.load(ctx, instance).catch(handleLoadError)
+    } else {
+      // for non-destinations plugins, we do need to wait for them to load
+      // reminder: action destinations can require plugins that are not of type "destination".
+      // For example, GA4 loads a type 'before' plugins and addition to a type 'destination' plugin
+      try {
+        await plugin.load(ctx, instance)
+      } catch (err) {
+        handleLoadError(err)
+      }
+    }
   }
 
   async deregister(

packages/node/src/__tests__/integration.test.ts

@@ -406,7 +406,6 @@ describe('version', () => {
 describe('ready', () => {
   it('should only resolve when plugin registration is done', async () => {
     const analytics = createTestAnalytics()
-    expect(analytics['_queue'].plugins.length).toBe(0)
     const result = await analytics.ready
     expect(result).toBeUndefined()
     expect(analytics['_queue'].plugins.length).toBeGreaterThan(0)