Retain messageId if exists (allow overriding) (#1043)

Co-authored-by: H.Drake <hd@activeops.io>

Author: silesky

.changeset/message-id-backward-compatibility.md

@@ -0,0 +1,8 @@
+---
+'@segment/analytics-core': patch
+'@segment/analytics-node': minor
+---
+
+This ensures backward compatibility with analytics-node by modifying '@segment/analytics-core'. Specifically, the changes prevent the generation of a messageId if it is already set. This adjustment aligns with the behavior outlined in analytics-node's source code [here](https://github.com/segmentio/analytics-node/blob/master/index.js#L195-L201).
+
+While this is a core release, only the node library is affected, as the browser has its own EventFactory atm.

packages/core/src/events/__tests__/index.test.ts

@@ -350,6 +350,27 @@ describe('Event Factory', () => {
         innerProp: '👻',
       })
     })
+
+    test('accepts a messageId', () => {
+      const messageId = 'business-id-123'
+      const track = factory.track('Order Completed', shoes, {
+        messageId,
+      })
+
+      expect(track.context).toEqual({})
+      expect(track.messageId).toEqual(messageId)
+    })
+
+    it('should ignore undefined options', () => {
+      const event = factory.track(
+        'Order Completed',
+        { ...shoes },
+        { timestamp: undefined, traits: { foo: 123 } }
+      )
+
+      expect(typeof event.timestamp).toBe('object')
+      expect(isDate(event.timestamp)).toBeTruthy()
+    })
   })
 
   describe('normalize', () => {
@@ -380,15 +401,4 @@ describe('Event Factory', () => {
       })
     })
   })
-
-  it('should ignore undefined options', () => {
-    const event = factory.track(
-      'Order Completed',
-      { ...shoes },
-      { timestamp: undefined, traits: { foo: 123 } }
-    )
-
-    expect(typeof event.timestamp).toBe('object')
-    expect(isDate(event.timestamp)).toBeTruthy()
-  })
 })

packages/core/src/events/index.ts

@@ -19,6 +19,10 @@ interface EventFactorySettings {
   user?: User
 }
 
+/**
+ * This is currently only used by node.js, but the original idea was to have something that could be shared between browser and node.
+ * Unfortunately, there are some differences in the way the two environments handle events, so this is not currently shared.
+ */
 export class EventFactory {
   createMessageId: EventFactorySettings['createMessageId']
   user?: User
@@ -201,6 +205,7 @@ export class EventFactory {
       'userId',
       'anonymousId',
       'timestamp',
+      'messageId',
     ]
 
     delete options['integrations']
@@ -271,7 +276,7 @@ export class EventFactory {
 
     const evt: CoreSegmentEvent = {
       ...body,
-      messageId: this.createMessageId(),
+      messageId: options.messageId || this.createMessageId(),
     }
 
     validateEvent(evt)

packages/core/src/events/interfaces.ts

@@ -33,6 +33,12 @@ export interface CoreOptions {
   anonymousId?: string
   userId?: string
   traits?: Traits
+  /**
+   * Override the messageId. Under normal circumstances, this is not recommended -- but neccessary for deduping events.
+   *
+   * **Currently, This option only works in `@segment/analytics-node`.**
+   */
+  messageId?: string
   // ugh, this is ugly, but we allow literally any property to be passed to options (which get spread onto the event)
   [key: string]: any
 }

packages/core/src/validation/__tests__/assertions.test.ts

@@ -4,7 +4,9 @@ import { validateEvent } from '../assertions'
 const baseEvent: Partial<CoreSegmentEvent> = {
   userId: 'foo',
   event: 'Test Event',
+  messageId: 'foo',
 }
+
 describe(validateEvent, () => {
   test('should be capable of working with empty properties and traits', () => {
     expect(() => validateEvent(undefined)).toThrowError()
@@ -40,6 +42,7 @@ describe(validateEvent, () => {
   test('identify: traits should be an object', () => {
     expect(() =>
       validateEvent({
+        ...baseEvent,
         type: 'identify',
         traits: undefined,
       })
@@ -151,5 +154,29 @@ describe(validateEvent, () => {
         })
       ).toThrowError(/nil/i)
     })
+
+    test('should fail if messageId is _not_ string', () => {
+      expect(() =>
+        validateEvent({
+          ...baseEvent,
+          type: 'track',
+          properties: {},
+          userId: undefined,
+          anonymousId: 'foo',
+          messageId: 'bar',
+        })
+      ).not.toThrow()
+
+      expect(() =>
+        validateEvent({
+          ...baseEvent,
+          type: 'track',
+          properties: {},
+          userId: undefined,
+          anonymousId: 'foo',
+          messageId: 123 as any,
+        })
+      ).toThrow(/messageId/)
+    })
   })
 })

packages/core/src/validation/assertions.ts

@@ -55,9 +55,16 @@ export function assertTraits(event: CoreSegmentEvent): void {
   }
 }
 
+export function assertMessageId(event: CoreSegmentEvent): void {
+  if (!isString(event.messageId)) {
+    throw new ValidationError('.messageId', stringError)
+  }
+}
+
 export function validateEvent(event?: CoreSegmentEvent | null) {
   assertEventExists(event)
   assertEventType(event)
+  assertMessageId(event)
 
   if (event.type === 'track') {
     assertTrackEventName(event)

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

@@ -88,6 +88,16 @@ describe('alias', () => {
     expect(ctx.event.userId).toEqual('chris radek')
     expect(ctx.event.previousId).toEqual('chris')
     expect(ctx.event.timestamp).toEqual(timestamp)
+    expect(ctx.event.messageId).toEqual(expect.any(String))
+  })
+  it('allows messageId to be overridden', async () => {
+    const analytics = createTestAnalytics({
+      httpClient: testClient,
+    })
+    const messageId = 'overridden'
+    analytics.alias({ userId: 'foo', previousId: 'bar', messageId })
+    const ctx = await resolveCtx(analytics, 'alias')
+    expect(ctx.event.messageId).toBe(messageId)
   })
 })
 
@@ -107,6 +117,7 @@ describe('group', () => {
     expect(ctx.event.userId).toEqual('foo')
     expect(ctx.event.anonymousId).toBe('bar')
     expect(ctx.event.timestamp).toEqual(timestamp)
+    expect(ctx.event.messageId).toEqual(expect.any(String))
   })
 
   it('invocations are isolated', async () => {
@@ -134,6 +145,16 @@ describe('group', () => {
     expect(ctx2.event.anonymousId).toBeUndefined()
     expect(ctx2.event.userId).toEqual('me')
   })
+
+  it('allows messageId to be overridden', async () => {
+    const analytics = createTestAnalytics({
+      httpClient: testClient,
+    })
+    const messageId = 'overridden'
+    analytics.group({ groupId: 'foo', userId: 'sup', messageId })
+    const ctx = await resolveCtx(analytics, 'group')
+    expect(ctx.event.messageId).toBe(messageId)
+  })
 })
 
 describe('identify', () => {
@@ -181,6 +202,7 @@ describe('page', () => {
     expect(ctx1.event.userId).toBeUndefined()
     expect(ctx1.event.properties).toEqual({ category })
     expect(ctx1.event.timestamp).toEqual(timestamp)
+    expect(ctx1.event.messageId).toEqual(expect.any(String))
 
     analytics.page({ name, properties: { title: 'wip' }, userId: 'user-id' })
 
@@ -192,6 +214,7 @@ describe('page', () => {
     expect(ctx2.event.userId).toEqual('user-id')
     expect(ctx2.event.properties).toEqual({ title: 'wip' })
     expect(ctx2.event.timestamp).toEqual(expect.any(Date))
+    expect(ctx2.event.messageId).toEqual(expect.any(String))
 
     analytics.page({ properties: { title: 'invisible' }, userId: 'user-id' })
     const ctx3 = await resolveCtx(analytics, 'page')
@@ -201,6 +224,17 @@ describe('page', () => {
     expect(ctx3.event.anonymousId).toBeUndefined()
     expect(ctx3.event.userId).toEqual('user-id')
     expect(ctx3.event.properties).toEqual({ title: 'invisible' })
+    expect(ctx3.event.messageId).toEqual(expect.any(String))
+  })
+
+  it('allows messageId to be overridden', async () => {
+    const analytics = createTestAnalytics({
+      httpClient: testClient,
+    })
+    const messageId = 'overridden'
+    analytics.page({ name: 'foo', userId: 'sup', messageId })
+    const ctx = await resolveCtx(analytics, 'page')
+    expect(ctx.event.messageId).toBe(messageId)
   })
 })
 
@@ -224,6 +258,7 @@ describe('screen', () => {
     expect(ctx1.event.userId).toEqual('user-id')
     expect(ctx1.event.properties).toEqual({ title: 'wip' })
     expect(ctx1.event.timestamp).toEqual(timestamp)
+    expect(ctx1.event.messageId).toEqual(expect.any(String))
 
     analytics.screen({
       properties: { title: 'invisible' },
@@ -238,6 +273,16 @@ describe('screen', () => {
     expect(ctx2.event.userId).toEqual('user-id')
     expect(ctx2.event.properties).toEqual({ title: 'invisible' })
     expect(ctx2.event.timestamp).toEqual(expect.any(Date))
+    expect(ctx2.event.messageId).toEqual(expect.any(String))
+  })
+  it('allows messageId to be overridden', async () => {
+    const analytics = createTestAnalytics({
+      httpClient: testClient,
+    })
+    const messageId = 'overridden'
+    analytics.screen({ name: 'foo', userId: 'sup', messageId })
+    const ctx = await resolveCtx(analytics, 'screen')
+    expect(ctx.event.messageId).toBe(messageId)
   })
 })
 
@@ -272,6 +317,7 @@ describe('track', () => {
     expect(ctx1.event.anonymousId).toEqual('unknown')
     expect(ctx1.event.userId).toEqual('known')
     expect(ctx1.event.timestamp).toEqual(timestamp)
+    expect(ctx1.event.messageId).toEqual(expect.any(String))
 
     analytics.track({
       event: eventName,
@@ -286,6 +332,17 @@ describe('track', () => {
     expect(ctx2.event.anonymousId).toBeUndefined()
     expect(ctx2.event.userId).toEqual('known')
     expect(ctx2.event.timestamp).toEqual(expect.any(Date))
+    expect(ctx2.event.messageId).toEqual(expect.any(String))
+  })
+
+  it('allows messageId to be overridden', async () => {
+    const analytics = createTestAnalytics({
+      httpClient: testClient,
+    })
+    const messageId = 'overridden'
+    analytics.track({ event: 'foo', userId: 'sup', messageId })
+    const ctx = await resolveCtx(analytics, 'track')
+    expect(ctx.event.messageId).toBe(messageId)
   })
 })
 

packages/node/src/app/analytics-node.ts

@@ -148,13 +148,21 @@ export class Analytics extends NodeEmitter implements CoreAnalytics {
    * @link https://segment.com/docs/connections/sources/catalog/libraries/server/node/#alias
    */
   alias(
-    { userId, previousId, context, timestamp, integrations }: AliasParams,
+    {
+      userId,
+      previousId,
+      context,
+      timestamp,
+      integrations,
+      messageId,
+    }: AliasParams,
     callback?: Callback
   ): void {
     const segmentEvent = this._eventFactory.alias(userId, previousId, {
       context,
       integrations,
       timestamp,
+      messageId,
     })
     this._dispatch(segmentEvent, callback)
   }
@@ -172,6 +180,7 @@ export class Analytics extends NodeEmitter implements CoreAnalytics {
       traits = {},
       context,
       integrations,
+      messageId,
     }: GroupParams,
     callback?: Callback
   ): void {
@@ -181,6 +190,7 @@ export class Analytics extends NodeEmitter implements CoreAnalytics {
       userId,
       timestamp,
       integrations,
+      messageId,
     })
 
     this._dispatch(segmentEvent, callback)
@@ -198,6 +208,7 @@ export class Analytics extends NodeEmitter implements CoreAnalytics {
       context,
       timestamp,
       integrations,
+      messageId,
     }: IdentifyParams,
     callback?: Callback
   ): void {
@@ -207,6 +218,7 @@ export class Analytics extends NodeEmitter implements CoreAnalytics {
       userId,
       timestamp,
       integrations,
+      messageId,
     })
     this._dispatch(segmentEvent, callback)
   }
@@ -225,14 +237,15 @@ export class Analytics extends NodeEmitter implements CoreAnalytics {
       context,
       timestamp,
       integrations,
+      messageId,
     }: PageParams,
     callback?: Callback
   ): void {
     const segmentEvent = this._eventFactory.page(
       category ?? null,
       name ?? null,
       properties,
-      { context, anonymousId, userId, timestamp, integrations }
+      { context, anonymousId, userId, timestamp, integrations, messageId }
     )
     this._dispatch(segmentEvent, callback)
   }
@@ -253,14 +266,15 @@ export class Analytics extends NodeEmitter implements CoreAnalytics {
       context,
       timestamp,
       integrations,
+      messageId,
     }: PageParams,
     callback?: Callback
   ): void {
     const segmentEvent = this._eventFactory.screen(
       category ?? null,
       name ?? null,
       properties,
-      { context, anonymousId, userId, timestamp, integrations }
+      { context, anonymousId, userId, timestamp, integrations, messageId }
     )
 
     this._dispatch(segmentEvent, callback)
@@ -279,6 +293,7 @@ export class Analytics extends NodeEmitter implements CoreAnalytics {
       context,
       timestamp,
       integrations,
+      messageId,
     }: TrackParams,
     callback?: Callback
   ): void {
@@ -288,6 +303,7 @@ export class Analytics extends NodeEmitter implements CoreAnalytics {
       anonymousId,
       timestamp,
       integrations,
+      messageId,
     })
 
     this._dispatch(segmentEvent, callback)