wip

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

@@ -5,9 +5,9 @@ import {
 
 describe('disable', () => {
   const httpClient = new TestFetchClient()
-  const mockSend = jest.spyOn(httpClient, 'send')
+  const makeReqSpy = jest.spyOn(httpClient, 'makeRequest')
 
-  it('should dispatch callbacks and emit an http request, even if disabled', async () => {
+  it('should not emit an http request if disabled', async () => {
     const analytics = createTestAnalytics({
       disable: true,
     })
@@ -16,7 +16,7 @@ describe('disable', () => {
     await new Promise((resolve) =>
       analytics.track({ anonymousId: 'foo', event: 'bar' }, resolve)
     )
-    expect(emitterCb).toBeCalledTimes(1)
+    expect(emitterCb).not.toBeCalled()
   })
 
   it('should call .send if disabled is false', async () => {
@@ -27,7 +27,7 @@ describe('disable', () => {
     await new Promise((resolve) =>
       analytics.track({ anonymousId: 'foo', event: 'bar' }, resolve)
     )
-    expect(mockSend).toBeCalledTimes(1)
+    expect(makeReqSpy).toBeCalledTimes(1)
   })
   it('should not call .send if disabled is true', async () => {
     const analytics = createTestAnalytics({
@@ -37,6 +37,6 @@ describe('disable', () => {
     await new Promise((resolve) =>
       analytics.track({ anonymousId: 'foo', event: 'bar' }, resolve)
     )
-    expect(mockSend).not.toBeCalled()
+    expect(makeReqSpy).not.toBeCalled()
   })
 })

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

@@ -13,11 +13,12 @@ const testPlugin: Plugin = {
   isLoaded: () => true,
 }
 
-const testClient = new TestFetchClient()
-const sendSpy = jest.spyOn(testClient, 'makeRequest')
+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 () => {
     ajs = new Analytics({
       writeKey: 'abc123',
@@ -27,11 +28,11 @@ describe('Ability for users to exit without losing events', () => {
   })
   const _helpers = {
     getFetchCalls: () =>
-      sendSpy.mock.calls.map(([url, request]) => ({
+      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)
@@ -205,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 () => {
@@ -237,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__/integration.test.ts

@@ -12,7 +12,7 @@ jest.setTimeout(10000)
 const timestamp = new Date()
 
 const testClient = new TestFetchClient()
-const sendSpy = jest.spyOn(testClient, 'makeRequest')
+const makeReqSpy = jest.spyOn(testClient, 'makeRequest')
 
 describe('Settings / Configuration Init', () => {
   it('throws if no writeKey', () => {
@@ -32,7 +32,7 @@ describe('Settings / Configuration Init', () => {
     const track = resolveCtx(analytics, 'track')
     analytics.track({ event: 'foo', userId: 'sup' })
     await track
-    expect(sendSpy.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 () => {

packages/node/src/__tests__/test-helpers/assert-shape/http-request-event.ts

@@ -4,7 +4,7 @@ type HttpRequestEmitterEvent = NodeEmitterEvents['http_request'][0]
 export const assertHttpRequestEmittedEvent = (
   event: HttpRequestEmitterEvent
 ) => {
-  const body = JSON.parse(event.body)
+  const body = event.body
   expect(Array.isArray(body.batch)).toBeTruthy()
   expect(body.batch.length).toBe(1)
   expect(typeof event.headers).toBe('object')

packages/node/src/__tests__/test-helpers/create-test-analytics.ts

@@ -1,6 +1,6 @@
 import { Analytics } from '../../app/analytics-node'
 import { AnalyticsSettings } from '../../app/settings'
-import { AnalyticsHTTPClientDELETE, HTTPClient } from '../../lib/http-client'
+import { FetchHTTPClient, HTTPFetchFn } from '../../lib/http-client'
 import { createError, createSuccess } from './factories'
 
 export const createTestAnalytics = (
@@ -29,17 +29,14 @@ export type TestFetchClientOptions = {
 /**
  * Test http client. By default, it will return a successful response.
  */
-export class TestFetchClient implements HTTPClient {
-  private withError?: TestFetchClientOptions['withError']
-  private response?: TestFetchClientOptions['response']
+export class TestFetchClient extends FetchHTTPClient {
   constructor({ withError, response }: TestFetchClientOptions = {}) {
-    this.withError = withError
-    this.response = response
-  }
-  makeRequest() {
-    if (this.response) {
-      return Promise.resolve(this.response)
+    const _mockFetch: HTTPFetchFn = (..._args) => {
+      if (response) {
+        return Promise.resolve(response)
+      }
+      return Promise.resolve(withError ? createError() : createSuccess())
     }
-    return Promise.resolve(this.withError ? createError() : createSuccess())
+    super(_mockFetch)
   }
 }

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

@@ -53,7 +53,10 @@ export class Analytics extends NodeEmitter implements CoreAnalytics {
         httpRequestTimeout: settings.httpRequestTimeout,
         disable: settings.disable,
         flushInterval,
-        httpClient: settings.httpClient ?? new FetchHTTPClient(fetch),
+        httpClient:
+          typeof settings.httpClient === 'function'
+            ? new FetchHTTPClient(settings.httpClient)
+            : settings.httpClient ?? new FetchHTTPClient(fetch),
       },
       this as NodeEmitter
     )

packages/node/src/app/emitter.ts

@@ -14,7 +14,7 @@ export type NodeEmitterEvents = CoreEmitterContract<Context> & {
       url: string
       method: string
       headers: Record<string, string>
-      body: string
+      body: any
     }
   ]
   drained: []

packages/node/src/app/settings.ts

@@ -1,5 +1,5 @@
 import { ValidationError } from '@segment/analytics-core'
-import { HTTPClient } from '../lib/http-client'
+import { HTTPClient, HTTPFetchFn } from '../lib/http-client'
 
 export interface AnalyticsSettings {
   /**
@@ -36,9 +36,11 @@ export interface AnalyticsSettings {
    */
   disable?: boolean
   /**
-   * Supply a default http client implementation (such as one supporting proxy). Default: The value of globalThis.fetch, with node-fetch as a fallback.
+   * Supply a default http client implementation (such as one supporting proxy).
+   * Accepts either an HTTPClient instance or a fetch function.
+   * Default: an HTTP client that uses globalThis.fetch, with node-fetch as a fallback.
    */
-  httpClient?: HTTPClient
+  httpClient?: HTTPFetchFn | HTTPClient
 }
 
 export const validateSettings = (settings: AnalyticsSettings) => {

packages/node/src/index.ts

@@ -2,8 +2,7 @@ export { Analytics } from './app/analytics-node'
 export { Context } from './app/context'
 export type {
   FetchHTTPClient,
-  AnalyticsHTTPClientDELETE,
-  HTTPClientOptions,
+  FetchHTTPClientOptions,
   HTTPFetchFn,
   HTTPFetchClientResponse,
 } from './lib/http-client'

packages/node/src/lib/__tests__/abort.test.ts

@@ -1,5 +1,6 @@
 import { abortSignalAfterTimeout } from '../abort'
 import nock from 'nock'
+import { fetch } from '../fetch'
 import { sleep } from '@segment/analytics-core'
 
 describe(abortSignalAfterTimeout, () => {

packages/node/src/lib/http-client.ts

@@ -1,5 +1,3 @@
-import type { Analytics } from '../app/analytics-node'
-import { NodeEmitter } from '../app/emitter'
 import { abortSignalAfterTimeout } from './abort'
 
 export interface HTTPFetchClientResponse {
@@ -13,56 +11,42 @@ export interface HTTPFetchClientResponse {
   // type: string
 }
 
+/**
+ * This interface is meant to be compatible with different fetch implementations.
+ */
 export interface HTTPFetchFn {
   (
-    url: string | URL,
-    options?: HTTPClientOptions
+    url: string,
+    options: FetchHTTPClientOptions
   ): Promise<HTTPFetchClientResponse>
 }
 
 export interface HTTPClient {
-  makeRequest(
-    _options: HTTPRequestOptions,
-    emitter: NodeEmitter
-  ): Promise<HTTPFetchClientResponse>
+  makeRequest(_options: HTTPRequestOptions): Promise<HTTPFetchClientResponse>
 }
 
-interface HTTPRequestOptions {
+export interface HTTPRequestOptions {
   url: string
   method: string
   headers: Record<string, string>
   data: Record<string, any>
   timeout: number
 }
 
-export interface HTTPClientOptions {
+export interface FetchHTTPClientOptions {
   headers?: Record<string, string>
   body?: string
   method?: string
   signal?: any // AbortSignal type does not play nicely with node-fetch
 }
 
-/**
- * A client that sends http requests.
- */
-export interface AnalyticsHTTPClientDELETE {
-  /**
-   *  Compatible with the fetch API
-   */
-  send(
-    url: string,
-    options: HTTPClientOptions
-  ): Promise<HTTPFetchClientResponse>
-}
-
 export class FetchHTTPClient implements HTTPClient {
   private _fetch: HTTPFetchFn
   constructor(fetchFn: HTTPFetchFn | typeof globalThis.fetch) {
     this._fetch = fetchFn
   }
   async makeRequest(
-    options: HTTPRequestOptions,
-    analytics: Analytics
+    options: HTTPRequestOptions
   ): Promise<HTTPFetchClientResponse> {
     const [signal, timeoutId] = abortSignalAfterTimeout(options.timeout)
 
@@ -74,14 +58,8 @@ export class FetchHTTPClient implements HTTPClient {
       signal: signal,
     }
 
-    analytics.emit('http_request', {
-      url: requestInit.url,
-      method: requestInit.method,
-      headers: requestInit.headers,
-      body: requestInit.body,
-    })
-    const response = await this._fetch(options.url, requestInit)
-    clearTimeout(timeoutId)
-    return response
+    return this._fetch(options.url, requestInit).finally(() =>
+      clearTimeout(timeoutId)
+    )
   }
 }