Skip to content

Commit

Permalink
Addressing some PR comments, mostly renaming things
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelGHSeg committed Jul 1, 2023
1 parent 52947b6 commit db7d425
Show file tree
Hide file tree
Showing 13 changed files with 70 additions and 50 deletions.
4 changes: 2 additions & 2 deletions packages/node/src/__tests__/callback.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('Callback behavior', () => {
it('should handle success', async () => {
const ajs = createTestAnalytics({
maxEventsInBatch: 1,
customClient: testClient,
httpClient: testClient,
})
const ctx = await new Promise<Context>((resolve, reject) =>
ajs.track(
Expand All @@ -37,7 +37,7 @@ describe('Callback behavior', () => {
testClient.returnValue = createError()
const ajs = createTestAnalytics({
maxEventsInBatch: 1,
customClient: testClient,
httpClient: testClient,
})
const [err, ctx] = await new Promise<[any, Context]>((resolve) =>
ajs.track(
Expand Down
6 changes: 3 additions & 3 deletions packages/node/src/__tests__/disable.integration.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { createTestAnalytics } from './test-helpers/create-test-analytics'
import { CustomHTTPClient } from '../lib/customhttpclient'
import { CustomHTTPClient } from '../lib/custom-http-client'

export class CheckFetchClient implements CustomHTTPClient {
private _wasCalled = false
Expand Down Expand Up @@ -38,7 +38,7 @@ describe('disable', () => {
it('should call fetch if disabled is false', async () => {
const analytics = createTestAnalytics({
disable: false,
customClient: checkFetchClient,
httpClient: checkFetchClient,
})
checkFetchClient.wasCalled = false
await new Promise((resolve) =>
Expand All @@ -49,7 +49,7 @@ describe('disable', () => {
it('should not call fetch if disabled is true', async () => {
const analytics = createTestAnalytics({
disable: true,
customClient: checkFetchClient,
httpClient: checkFetchClient,
})
checkFetchClient.wasCalled = false
await new Promise((resolve) =>
Expand Down
4 changes: 2 additions & 2 deletions packages/node/src/__tests__/emitter.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('http_request', () => {
testClient.returnValue = createError()
const analytics = createTestAnalytics({
maxRetries: 0,
customClient: testClient,
httpClient: testClient,
})
const fn = jest.fn()
analytics.on('http_request', fn)
Expand All @@ -38,7 +38,7 @@ describe('http_request', () => {
testClient.returnValue = createError()
const analytics = createTestAnalytics({
maxRetries: 2,
customClient: testClient,
httpClient: testClient,
})
const fn = jest.fn()
analytics.on('http_request', fn)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('Ability for users to exit without losing events', () => {
ajs = new Analytics({
writeKey: 'abc123',
maxEventsInBatch: 1,
customClient: testClient,
httpClient: testClient,
})
})
const _helpers = {
Expand Down Expand Up @@ -88,7 +88,7 @@ describe('Ability for users to exit without losing events', () => {
ajs = new Analytics({
writeKey: 'abc123',
flushInterval,
customClient: testClient,
httpClient: testClient,
})
const closeAndFlushTimeout = ajs['_closeAndFlushDefaultTimeout']
expect(closeAndFlushTimeout).toBe(flushInterval * 1.25)
Expand Down Expand Up @@ -190,7 +190,7 @@ describe('Ability for users to exit without losing events', () => {
writeKey: 'foo',
flushInterval: 10000,
maxEventsInBatch: 15,
customClient: testClient,
httpClient: testClient,
})
_helpers.makeTrackCall(analytics)
_helpers.makeTrackCall(analytics)
Expand Down Expand Up @@ -221,7 +221,7 @@ describe('Ability for users to exit without losing events', () => {
writeKey: 'foo',
flushInterval: 10000,
maxEventsInBatch: 15,
customClient: testClient,
httpClient: testClient,
})
await analytics.register(_testPlugin)
_helpers.makeTrackCall(analytics)
Expand Down
6 changes: 3 additions & 3 deletions packages/node/src/__tests__/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe('Settings / Configuration Init', () => {
const analytics = createTestAnalytics({
host: 'http://foo.com',
path: '/bar',
customClient: testClient,
httpClient: testClient,
})
const track = resolveCtx(analytics, 'track')
analytics.track({ event: 'foo', userId: 'sup' })
Expand All @@ -52,10 +52,10 @@ describe('Error handling', () => {
expect(() => analytics.track({} as any)).toThrowError(/event/i)
})

it.only('should emit on an error', async () => {
it('should emit on an error', async () => {
const analytics = createTestAnalytics({
maxRetries: 0,
customClient: testClient,
httpClient: testClient,
})
testClient.returnValue = createError({
statusText: 'Service Unavailable',
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/__tests__/test-helpers/factories.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CustomHTTPClient } from '../../lib/customhttpclient'
import { CustomHTTPClient } from '../../lib/custom-http-client'

export const createSuccess = (body?: any) => {
return Promise.resolve({
Expand Down
4 changes: 2 additions & 2 deletions packages/node/src/app/analytics-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
} from './types'
import { Context } from './context'
import { NodeEventQueue } from './event-queue'
import { DefaultFetchClient } from '../lib/customhttpclient'
import { DefaultHttpClient } from '../lib/custom-http-client'

export class Analytics extends NodeEmitter implements CoreAnalytics {
private readonly _eventFactory: NodeEventFactory
Expand Down Expand Up @@ -52,7 +52,7 @@ export class Analytics extends NodeEmitter implements CoreAnalytics {
httpRequestTimeout: settings.httpRequestTimeout,
disable: settings.disable,
flushInterval,
customclient: settings.customClient ?? new DefaultFetchClient(),
httpClient: settings.httpClient ?? new DefaultHttpClient(),
},
this as NodeEmitter
)
Expand Down
6 changes: 3 additions & 3 deletions packages/node/src/app/settings.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ValidationError } from '@segment/analytics-core'
import { CustomHTTPClient } from '../lib/customhttpclient'
import { CustomHTTPClient } from '../lib/custom-http-client'

export interface AnalyticsSettings {
/**
Expand Down Expand Up @@ -38,10 +38,10 @@ export interface AnalyticsSettings {

/**
* Supply a CustomHTTPClient implementation (such as one supporting proxy)
* Default: DefaultFetchClient which will use the existing global fetch, or
* Default: DefaultHttpClient which will use the existing global fetch, or
* node-fetch if it doesn't exist
*/
customClient?: CustomHTTPClient
httpClient?: CustomHTTPClient
}

export const validateSettings = (settings: AnalyticsSettings) => {
Expand Down
6 changes: 3 additions & 3 deletions packages/node/src/lib/__tests__/abort.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { abortSignalAfterTimeout } from '../abort'
import nock from 'nock'
import { sleep } from '@segment/analytics-core'
import { DefaultFetchClient } from '../customhttpclient'
import { DefaultHttpClient } from '../custom-http-client'

describe(abortSignalAfterTimeout, () => {
const HOST = 'https://foo.com'
Expand Down Expand Up @@ -30,7 +30,7 @@ describe(abortSignalAfterTimeout, () => {
try {
const [signal] = abortSignalAfterTimeout(2000)
jest.advanceTimersByTime(6000)
const client = new DefaultFetchClient()
const client = new DefaultHttpClient()
await client.send(HOST, { signal })
throw Error('fail test.')
} catch (err: any) {
Expand All @@ -43,7 +43,7 @@ describe(abortSignalAfterTimeout, () => {
nock(HOST).get('/').reply(201)
const [signal] = abortSignalAfterTimeout(0)
try {
const client = new DefaultFetchClient()
const client = new DefaultHttpClient()
await client.send(HOST, { signal })
throw Error('fail test.')
} catch (err: any) {
Expand Down
20 changes: 20 additions & 0 deletions packages/node/src/lib/custom-http-client.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export interface CustomHTTPClient {
send: (resource: any, options: any) => Promise<Response>
}

export class DefaultHttpClient implements CustomHTTPClient {
send = async (resource: any, options: any): Promise<Response> => {
if (globalThis.fetch) {
return globalThis.fetch(resource, options)
} // @ts-ignore
// This guard causes is important, as it causes dead-code elimination to be enabled inside this block.
else if (typeof EdgeRuntime !== 'string') {
// @ts-ignore
return (await import('node-fetch')).default(payload, options) as Response
} else {
throw new Error(
'Invariant: an edge runtime that does not support fetch should not exist'
)
}
}
}
12 changes: 6 additions & 6 deletions packages/node/src/plugins/segmentio/__tests__/methods.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ test('alias', async () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const event = eventFactory.alias('to', 'from')
Expand Down Expand Up @@ -65,7 +65,7 @@ test('group', async () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const event = eventFactory.group(
Expand Down Expand Up @@ -105,7 +105,7 @@ test('identify', async () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const event = eventFactory.identify('foo-user-id', {
Expand Down Expand Up @@ -138,7 +138,7 @@ test('page', async () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const event = eventFactory.page(
Expand Down Expand Up @@ -178,7 +178,7 @@ test('screen', async () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const event = eventFactory.screen(
Expand Down Expand Up @@ -217,7 +217,7 @@ test('track', async () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const event = eventFactory.track(
Expand Down
30 changes: 15 additions & 15 deletions packages/node/src/plugins/segmentio/__tests__/publisher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ it('supports multiple events in a batch', async () => {
maxEventsInBatch: 3,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

// Create 3 events of mixed types to send.
Expand Down Expand Up @@ -65,7 +65,7 @@ it('supports waiting a max amount of time before sending', async () => {
maxEventsInBatch: 3,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const context = new Context(eventFactory.alias('to', 'from'))
Expand Down Expand Up @@ -94,7 +94,7 @@ it('sends as soon as batch fills up or max time is reached', async () => {
maxEventsInBatch: 2,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const context = new Context(eventFactory.alias('to', 'from'))
Expand Down Expand Up @@ -131,7 +131,7 @@ it('sends if batch will exceed max size in bytes when adding event', async () =>
maxEventsInBatch: 20,
flushInterval: 100,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const contexts: Context[] = []
Expand Down Expand Up @@ -188,7 +188,7 @@ describe('flushAfterClose', () => {
maxEventsInBatch: 20,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

publisher.flushAfterClose(3)
Expand All @@ -206,7 +206,7 @@ describe('flushAfterClose', () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

publisher.flushAfterClose(3)
Expand All @@ -223,7 +223,7 @@ describe('flushAfterClose', () => {
maxEventsInBatch: 3,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

publisher.flushAfterClose(5)
Expand All @@ -239,7 +239,7 @@ describe('flushAfterClose', () => {
maxEventsInBatch: 7,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

range(3).forEach(() => segmentPlugin.track(_createTrackCtx())) // should not flush
Expand All @@ -256,7 +256,7 @@ describe('flushAfterClose', () => {
maxEventsInBatch: 7,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

range(3).forEach(() => segmentPlugin.track(_createTrackCtx())) // should not flush
Expand All @@ -279,7 +279,7 @@ describe('error handling', () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const context = new Context(
Expand Down Expand Up @@ -317,7 +317,7 @@ describe('error handling', () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const context = new Context(eventFactory.alias('to', 'from'))
Expand Down Expand Up @@ -350,7 +350,7 @@ describe('error handling', () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const context = new Context(eventFactory.alias('to', 'from'))
Expand Down Expand Up @@ -381,7 +381,7 @@ describe('error handling', () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const context = new Context(eventFactory.alias('my', 'from'))
Expand Down Expand Up @@ -411,7 +411,7 @@ describe('error handling', () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const fn = jest.fn()
Expand All @@ -437,7 +437,7 @@ describe('http_request emitter event', () => {
maxEventsInBatch: 1,
flushInterval: 1000,
writeKey: '',
customclient: testClient,
httpClient: testClient,
})

const fn = jest.fn()
Expand Down
Loading

0 comments on commit db7d425

Please sign in to comment.