Add ability to pass custom http client into node (#880)

.changeset/nervous-chefs-talk.md

@@ -0,0 +1,5 @@
+---
+'@segment/analytics-node': minor
+---
+
+Add `httpClient` setting. This allow users to override default HTTP client with a custom one.

packages/node/package.json

@@ -42,7 +42,8 @@
   },
   "devDependencies": {
     "@internal/config": "0.0.0",
-    "@types/node": "^16"
+    "@types/node": "^16",
+    "axios": "^1.4.0"
   },
   "packageManager": "[email protected]"
 }

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

@@ -1,17 +1,11 @@
-const fetcher = jest.fn()
-jest.mock('../lib/fetch', () => ({ fetch: fetcher }))
-
-import { createError, createSuccess } from './test-helpers/factories'
 import { createTestAnalytics } from './test-helpers/create-test-analytics'
 import { Context } from '../app/context'
 
 describe('Callback behavior', () => {
-  beforeEach(() => {
-    fetcher.mockReturnValue(createSuccess())
-  })
-
   it('should handle success', async () => {
-    const ajs = createTestAnalytics({ maxEventsInBatch: 1 })
+    const ajs = createTestAnalytics({
+      maxEventsInBatch: 1,
+    })
     const ctx = await new Promise<Context>((resolve, reject) =>
       ajs.track(
         {
@@ -29,8 +23,12 @@ describe('Callback behavior', () => {
   })
 
   it('should handle errors', async () => {
-    fetcher.mockReturnValue(createError())
-    const ajs = createTestAnalytics({ maxEventsInBatch: 1 })
+    const ajs = createTestAnalytics(
+      {
+        maxEventsInBatch: 1,
+      },
+      { withError: true }
+    )
     const [err, ctx] = await new Promise<[any, Context]>((resolve) =>
       ajs.track(
         {

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

@@ -1,10 +1,13 @@
-const fetcher = jest.fn()
-jest.mock('../lib/fetch', () => ({ fetch: fetcher }))
-
-import { createTestAnalytics } from './test-helpers/create-test-analytics'
+import {
+  createTestAnalytics,
+  TestFetchClient,
+} from './test-helpers/create-test-analytics'
 
 describe('disable', () => {
-  it('should dispatch callbacks and emit an http request, even if disabled', async () => {
+  const httpClient = new TestFetchClient()
+  const makeReqSpy = jest.spyOn(httpClient, 'makeRequest')
+
+  it('should not emit an http request if disabled', async () => {
     const analytics = createTestAnalytics({
       disable: true,
     })
@@ -13,25 +16,27 @@ describe('disable', () => {
     await new Promise((resolve) =>
       analytics.track({ anonymousId: 'foo', event: 'bar' }, resolve)
     )
-    expect(emitterCb).toBeCalledTimes(1)
+    expect(emitterCb).not.toBeCalled()
   })
 
-  it('should call fetch if disabled is false', async () => {
+  it('should call .send if disabled is false', async () => {
     const analytics = createTestAnalytics({
       disable: false,
+      httpClient: httpClient,
     })
     await new Promise((resolve) =>
       analytics.track({ anonymousId: 'foo', event: 'bar' }, resolve)
     )
-    expect(fetcher).toBeCalled()
+    expect(makeReqSpy).toBeCalledTimes(1)
   })
-  it('should not call fetch if disabled is true', async () => {
+  it('should not call .send if disabled is true', async () => {
     const analytics = createTestAnalytics({
       disable: true,
+      httpClient: httpClient,
     })
     await new Promise((resolve) =>
       analytics.track({ anonymousId: 'foo', event: 'bar' }, resolve)
     )
-    expect(fetcher).not.toBeCalled()
+    expect(makeReqSpy).not.toBeCalled()
   })
 })

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

@@ -1,13 +1,8 @@
-const fetcher = jest.fn()
-jest.mock('../lib/fetch', () => ({ fetch: fetcher }))
-
-import { createError, createSuccess } from './test-helpers/factories'
 import { createTestAnalytics } from './test-helpers/create-test-analytics'
 import { assertHttpRequestEmittedEvent } from './test-helpers/assert-shape'
 
 describe('http_request', () => {
   it('emits an http_request event if success', async () => {
-    fetcher.mockReturnValue(createSuccess())
     const analytics = createTestAnalytics()
     const fn = jest.fn()
     analytics.on('http_request', fn)
@@ -19,8 +14,12 @@ describe('http_request', () => {
   })
 
   it('emits an http_request event if error', async () => {
-    fetcher.mockReturnValue(createError())
-    const analytics = createTestAnalytics({ maxRetries: 0 })
+    const analytics = createTestAnalytics(
+      {
+        maxRetries: 0,
+      },
+      { withError: true }
+    )
     const fn = jest.fn()
     analytics.on('http_request', fn)
     await new Promise((resolve) =>
@@ -30,8 +29,12 @@ describe('http_request', () => {
   })
 
   it('if error, emits an http_request event on every retry', async () => {
-    fetcher.mockReturnValue(createError())
-    const analytics = createTestAnalytics({ maxRetries: 2 })
+    const analytics = createTestAnalytics(
+      {
+        maxRetries: 2,
+      },
+      { withError: true }
+    )
     const fn = jest.fn()
     analytics.on('http_request', fn)
     await new Promise((resolve) =>

packages/node/src/__tests__/graceful-shutdown-integration.test.ts

@@ -1,9 +1,5 @@
-import { createSuccess } from './test-helpers/factories'
+import { TestFetchClient } from './test-helpers/create-test-analytics'
 import { performance as perf } from 'perf_hooks'
-
-const fetcher = jest.fn()
-jest.mock('../lib/fetch', () => ({ fetch: fetcher }))
-
 import { Analytics } from '../app/analytics-node'
 import { sleep } from './test-helpers/sleep'
 import { Plugin, SegmentEvent } from '../app/types'
@@ -17,22 +13,26 @@ const testPlugin: Plugin = {
   isLoaded: () => true,
 }
 
+let testClient: TestFetchClient
+
 describe('Ability for users to exit without losing events', () => {
   let ajs!: Analytics
+  testClient = new TestFetchClient()
+  const makeReqSpy = jest.spyOn(testClient, 'makeRequest')
   beforeEach(async () => {
-    fetcher.mockReturnValue(createSuccess())
     ajs = new Analytics({
       writeKey: 'abc123',
       maxEventsInBatch: 1,
+      httpClient: testClient,
     })
   })
   const _helpers = {
-    getFetchCalls: (mockedFetchFn = fetcher) =>
-      mockedFetchFn.mock.calls.map(([url, request]) => ({
+    getFetchCalls: () =>
+      makeReqSpy.mock.calls.map(([{ url, method, data, headers }]) => ({
         url,
-        method: request.method,
-        headers: request.headers,
-        body: JSON.parse(request.body),
+        method,
+        headers,
+        data,
       })),
     makeTrackCall: (analytics = ajs, cb?: (...args: any[]) => void) => {
       analytics.track({ userId: 'foo', event: 'Thing Updated' }, cb)
@@ -89,6 +89,7 @@ describe('Ability for users to exit without losing events', () => {
       ajs = new Analytics({
         writeKey: 'abc123',
         flushInterval,
+        httpClient: testClient,
       })
       const closeAndFlushTimeout = ajs['_closeAndFlushDefaultTimeout']
       expect(closeAndFlushTimeout).toBe(flushInterval * 1.25)
@@ -190,6 +191,7 @@ describe('Ability for users to exit without losing events', () => {
         writeKey: 'foo',
         flushInterval: 10000,
         maxEventsInBatch: 15,
+        httpClient: testClient,
       })
       _helpers.makeTrackCall(analytics)
       _helpers.makeTrackCall(analytics)
@@ -204,7 +206,7 @@ describe('Ability for users to exit without losing events', () => {
       expect(elapsedTime).toBeLessThan(100)
       const calls = _helpers.getFetchCalls()
       expect(calls.length).toBe(1)
-      expect(calls[0].body.batch.length).toBe(2)
+      expect(calls[0].data.batch.length).toBe(2)
     })
 
     test('should wait to flush if close is called and an event has not made it to the segment.io plugin yet', async () => {
@@ -220,6 +222,7 @@ describe('Ability for users to exit without losing events', () => {
         writeKey: 'foo',
         flushInterval: 10000,
         maxEventsInBatch: 15,
+        httpClient: testClient,
       })
       await analytics.register(_testPlugin)
       _helpers.makeTrackCall(analytics)
@@ -235,7 +238,7 @@ describe('Ability for users to exit without losing events', () => {
       expect(elapsedTime).toBeLessThan(TRACK_DELAY * 2)
       const calls = _helpers.getFetchCalls()
       expect(calls.length).toBe(1)
-      expect(calls[0].body.batch.length).toBe(2)
+      expect(calls[0].data.batch.length).toBe(2)
     })
   })
 })

packages/node/src/__tests__/http-client.integration.test.ts

@@ -0,0 +1,69 @@
+import { FetchHTTPClient, HTTPFetchFn } from '..'
+import { AbortSignal as AbortSignalShim } from '../lib/abort'
+import { httpClientOptionsBodyMatcher } from './test-helpers/assert-shape/segment-http-api'
+import { createTestAnalytics } from './test-helpers/create-test-analytics'
+import { createSuccess } from './test-helpers/factories'
+
+const testFetch: jest.MockedFn<HTTPFetchFn> = jest
+  .fn()
+  .mockResolvedValue(createSuccess())
+
+let analytics: ReturnType<typeof createTestAnalytics>
+
+const helpers = {
+  makeTrackCall: () =>
+    new Promise((resolve) =>
+      analytics.track({ event: 'foo', userId: 'bar' }, resolve)
+    ),
+  assertFetchCallRequest: (
+    ...[url, options]: typeof testFetch['mock']['lastCall']
+  ) => {
+    expect(url).toBe('https://api.segment.io/v1/batch')
+    expect(options.headers).toEqual({
+      Authorization: 'Basic Zm9vOg==',
+      'Content-Type': 'application/json',
+      'User-Agent': 'analytics-node-next/latest',
+    })
+    expect(options.method).toBe('POST')
+    const getLastBatch = (): object[] => {
+      const [, options] = testFetch.mock.lastCall
+      const batch = JSON.parse(options.body!).batch
+      return batch
+    }
+    const batch = getLastBatch()
+    expect(batch.length).toBe(1)
+    expect(batch[0]).toEqual({
+      ...httpClientOptionsBodyMatcher,
+      timestamp: expect.stringMatching(
+        /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/
+      ),
+      properties: {},
+      event: 'foo',
+      type: 'track',
+      userId: 'bar',
+    })
+    // @ts-ignore
+    expect(options.signal).toBeInstanceOf(
+      typeof AbortSignal !== 'undefined' ? AbortSignal : AbortSignalShim
+    )
+  },
+}
+
+describe('httpClient option', () => {
+  it('can be a regular custom HTTP client', async () => {
+    analytics = createTestAnalytics({
+      httpClient: new FetchHTTPClient(testFetch),
+    })
+    expect(testFetch).toHaveBeenCalledTimes(0)
+    await helpers.makeTrackCall()
+    expect(testFetch).toHaveBeenCalledTimes(1)
+    helpers.assertFetchCallRequest(...testFetch.mock.lastCall)
+  })
+  it('can be a simple function that matches the fetch interface', async () => {
+    analytics = createTestAnalytics({ httpClient: testFetch })
+    expect(testFetch).toHaveBeenCalledTimes(0)
+    await helpers.makeTrackCall()
+    expect(testFetch).toHaveBeenCalledTimes(1)
+    helpers.assertFetchCallRequest(...testFetch.mock.lastCall)
+  })
+})

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

@@ -33,7 +33,7 @@ describe('Method Smoke Tests', () => {
   let scope: nock.Scope
   let ajs: Analytics
   beforeEach(async () => {
-    ajs = createTestAnalytics()
+    ajs = createTestAnalytics({}, { useRealHTTPClient: true })
   })
 
   describe('Metadata', () => {
@@ -333,10 +333,13 @@ describe('Client: requestTimeout', () => {
   })
   it('should timeout immediately if request timeout is set to 0', async () => {
     jest.useRealTimers()
-    const ajs = createTestAnalytics({
-      maxEventsInBatch: 1,
-      httpRequestTimeout: 0,
-    })
+    const ajs = createTestAnalytics(
+      {
+        maxEventsInBatch: 1,
+        httpRequestTimeout: 0,
+      },
+      { useRealHTTPClient: true }
+    )
     ajs.track({ event: 'foo', userId: 'foo', properties: { hello: 'world' } })
     try {
       await resolveCtx(ajs, 'track')

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

@@ -1,19 +1,18 @@
-const fetcher = jest.fn()
-jest.mock('../lib/fetch', () => ({ fetch: fetcher }))
-
 import { Plugin } from '../app/types'
 import { resolveCtx } from './test-helpers/resolve-ctx'
 import { testPlugin } from './test-helpers/test-plugin'
-import { createSuccess, createError } from './test-helpers/factories'
-import { createTestAnalytics } from './test-helpers/create-test-analytics'
+import { createError } from './test-helpers/factories'
+import {
+  createTestAnalytics,
+  TestFetchClient,
+} from './test-helpers/create-test-analytics'
 
 const writeKey = 'foo'
 jest.setTimeout(10000)
 const timestamp = new Date()
 
-beforeEach(() => {
-  fetcher.mockReturnValue(createSuccess())
-})
+const testClient = new TestFetchClient()
+const makeReqSpy = jest.spyOn(testClient, 'makeRequest')
 
 describe('Settings / Configuration Init', () => {
   it('throws if no writeKey', () => {
@@ -28,11 +27,12 @@ describe('Settings / Configuration Init', () => {
     const analytics = createTestAnalytics({
       host: 'http://foo.com',
       path: '/bar',
+      httpClient: testClient,
     })
     const track = resolveCtx(analytics, 'track')
     analytics.track({ event: 'foo', userId: 'sup' })
     await track
-    expect(fetcher.mock.calls[0][0]).toBe('http://foo.com/bar')
+    expect(makeReqSpy.mock.calls[0][0].url).toBe('http://foo.com/bar')
   })
 
   it('throws if host / path is bad', async () => {
@@ -53,10 +53,14 @@ describe('Error handling', () => {
   })
 
   it('should emit on an error', async () => {
-    const analytics = createTestAnalytics({ maxRetries: 0 })
-    fetcher.mockReturnValue(
-      createError({ statusText: 'Service Unavailable', status: 503 })
-    )
+    const err = createError({
+      statusText: 'Service Unavailable',
+      status: 503,
+    })
+    const analytics = createTestAnalytics({
+      maxRetries: 0,
+      httpClient: new TestFetchClient({ response: err }),
+    })
     try {
       const promise = resolveCtx(analytics, 'track')
       analytics.track({ event: 'foo', userId: 'sup' })