diff --git a/.changeset/brave-jokes-train.md b/.changeset/brave-jokes-train.md new file mode 100644 index 000000000..5bf1982e9 --- /dev/null +++ b/.changeset/brave-jokes-train.md @@ -0,0 +1,6 @@ +--- +'@segment/analytics-core': minor +--- + +Do not throw errors in .register() method. + diff --git a/.changeset/fast-moons-knock.md b/.changeset/fast-moons-knock.md new file mode 100644 index 000000000..c2b987e90 --- /dev/null +++ b/.changeset/fast-moons-knock.md @@ -0,0 +1,6 @@ +--- +'@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. \ No newline at end of file diff --git a/packages/browser/src/browser/__tests__/integration.test.ts b/packages/browser/src/browser/__tests__/integration.test.ts index 86c573003..73d4fef9a 100644 --- a/packages/browser/src/browser/__tests__/integration.test.ts +++ b/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((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 () => { diff --git a/packages/core/src/plugins/index.ts b/packages/core/src/plugins/index.ts index 0978acb96..80c79c79d 100644 --- a/packages/core/src/plugins/index.ts +++ b/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' @@ -26,11 +21,7 @@ export interface CorePlugin< version: string type: PluginType isLoaded: () => boolean - load: ( - ctx: Ctx, - instance: Analytics, - config?: CorePluginConfig - ) => Promise + load: (ctx: Ctx, instance: Analytics) => Promise unload?: (ctx: Ctx, instance: Analytics) => Promise | unknown ready?: () => Promise diff --git a/packages/core/src/queue/__tests__/event-queue.test.ts b/packages/core/src/queue/__tests__/event-queue.test.ts index f6f51adee..a1141594c 100644 --- a/packages/core/src/queue/__tests__/event-queue.test.ts +++ b/packages/core/src/queue/__tests__/event-queue.test.ts @@ -611,6 +611,7 @@ describe('Flushing', () => { describe('register', () => { it('only filters out failed destinations after loading', async () => { + jest.spyOn(console, 'warn').mockImplementation(noop) const eq = new TestEventQueue() const goodDestinationPlugin = { ...testPlugin, diff --git a/packages/core/src/queue/__tests__/extension-flushing.test.ts b/packages/core/src/queue/__tests__/extension-flushing.test.ts index e8eb11965..21ce2d20c 100644 --- a/packages/core/src/queue/__tests__/extension-flushing.test.ts +++ b/packages/core/src/queue/__tests__/extension-flushing.test.ts @@ -53,7 +53,7 @@ describe('Registration', () => { expect(load).toHaveBeenCalledWith(ctx, ajs) }) - test('fails if plugin cant be loaded', async () => { + test('does not reject if a plugin cant be loaded', async () => { const eq = new EventQueue() const plugin: Plugin = { @@ -65,9 +65,7 @@ describe('Registration', () => { } const ctx = TestCtx.system() - await expect( - eq.register(ctx, plugin, ajs) - ).rejects.toThrowErrorMatchingInlineSnapshot(`"👻"`) + await expect(eq.register(ctx, plugin, ajs)).resolves.toBe(undefined) }) test('allows for destinations to fail registration', async () => { diff --git a/packages/core/src/queue/event-queue.ts b/packages/core/src/queue/event-queue.ts index 0bcec05f9..c553e484a 100644 --- a/packages/core/src/queue/event-queue.ts +++ b/packages/core/src/queue/event-queue.ts @@ -50,25 +50,34 @@ export abstract class CoreEventQueue< plugin: Plugin, instance: CoreAnalytics ): Promise { - 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) => { + 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( diff --git a/packages/node/src/__tests__/integration.test.ts b/packages/node/src/__tests__/integration.test.ts index a555e89be..08ac060a5 100644 --- a/packages/node/src/__tests__/integration.test.ts +++ b/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)