From 53db69480ae7d9adf87b14e4c1ab2e407c079a45 Mon Sep 17 00:00:00 2001 From: Daniel Jackins Date: Tue, 15 Apr 2025 09:43:46 -0600 Subject: [PATCH 1/4] Check attempts before dispatching --- .../segmentio/__tests__/retries.test.ts | 53 ++++++++++++++++++- .../browser/src/plugins/segmentio/index.ts | 5 ++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts b/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts index 5a49e4a7c..2a9fd370a 100644 --- a/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts +++ b/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts @@ -36,7 +36,7 @@ describe('Segment.io retries 500s and 429', () => { const ctx = await analytics.track('event') jest.runAllTimers() - expect(ctx.attempts).toBeGreaterThanOrEqual(3) // Gets incremented after use + expect(ctx.attempts).toBeGreaterThanOrEqual(2) // Gets incremented after use expect(fetch.mock.calls.length).toBeGreaterThanOrEqual(2) expect(fetch.mock.lastCall[1].body).toContain('"retryCount":') }) @@ -131,3 +131,54 @@ describe('Batches retry 500s and 429', () => { expect(fetch.mock.lastCall[1].body).toContain('"retryCount":2') }) }) + +describe('retryQueue', () => { + let options: SegmentioSettings + let analytics: Analytics + let segment: Plugin + beforeEach(async () => { + jest.useRealTimers() + jest.resetAllMocks() + jest.restoreAllMocks() + + options = { + apiKey: 'foo', + } + + fetch.mockReturnValue(createError({ status: 500 })) + }) + + it('Only attempts once if retryQueue is false', async () => { + analytics = new Analytics( + { writeKey: options.apiKey }, + { retryQueue: false } + ) + segment = await segmentio( + analytics, + options, + cdnSettingsMinimal.integrations + ) + await analytics.register(segment, envEnrichment) + + await analytics.track('foo') + await new Promise((resolve) => setTimeout(resolve, 1000)) + expect(fetch).toHaveBeenCalledTimes(1) + }) + + it('Attempts multiple times if retryQueue is not false', async () => { + analytics = new Analytics( + { writeKey: options.apiKey }, + { retryQueue: true } + ) + segment = await segmentio( + analytics, + options, + cdnSettingsMinimal.integrations + ) + await analytics.register(segment, envEnrichment) + + await analytics.track('foo') + await new Promise((resolve) => setTimeout(resolve, 1000)) + expect(fetch.mock.calls.length).toBeGreaterThanOrEqual(2) + }) +}) diff --git a/packages/browser/src/plugins/segmentio/index.ts b/packages/browser/src/plugins/segmentio/index.ts index f3e40bf26..e05298586 100644 --- a/packages/browser/src/plugins/segmentio/index.ts +++ b/packages/browser/src/plugins/segmentio/index.ts @@ -112,6 +112,11 @@ export function segmentio( json = onAlias(analytics, json) } + if (buffer.getAttempts(ctx) >= buffer.maxAttempts) { + inflightEvents.delete(ctx) + return ctx + } + return client .dispatch( `${remote}/${path}`, From 257db2b24e57b9b17c3ca8e7bf603ced5f7637f2 Mon Sep 17 00:00:00 2001 From: Daniel Jackins Date: Tue, 15 Apr 2025 09:46:35 -0600 Subject: [PATCH 2/4] changeset --- .changeset/nasty-sloths-rhyme.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/nasty-sloths-rhyme.md diff --git a/.changeset/nasty-sloths-rhyme.md b/.changeset/nasty-sloths-rhyme.md new file mode 100644 index 000000000..2f5b05b43 --- /dev/null +++ b/.changeset/nasty-sloths-rhyme.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-next': minor +--- + +Fix retry behavior From b86f93bbd22eeb384561772badad3433a8819f41 Mon Sep 17 00:00:00 2001 From: Daniel Jackins Date: Tue, 15 Apr 2025 10:12:52 -0600 Subject: [PATCH 3/4] fix tests --- .../src/plugins/segmentio/__tests__/retries.test.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts b/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts index 2a9fd370a..b200ed2dd 100644 --- a/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts +++ b/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts @@ -21,7 +21,10 @@ describe('Segment.io retries 500s and 429', () => { jest.restoreAllMocks() options = { apiKey: 'foo' } - analytics = new Analytics({ writeKey: options.apiKey }) + analytics = new Analytics( + { writeKey: options.apiKey }, + { retryQueue: true } + ) segment = await segmentio( analytics, options, @@ -149,6 +152,7 @@ describe('retryQueue', () => { }) it('Only attempts once if retryQueue is false', async () => { + jest.useFakeTimers({ advanceTimers: true }) analytics = new Analytics( { writeKey: options.apiKey }, { retryQueue: false } @@ -161,11 +165,12 @@ describe('retryQueue', () => { await analytics.register(segment, envEnrichment) await analytics.track('foo') - await new Promise((resolve) => setTimeout(resolve, 1000)) + jest.runAllTimers() expect(fetch).toHaveBeenCalledTimes(1) }) - it('Attempts multiple times if retryQueue is not false', async () => { + it('Attempts multiple times if retryQueue is true', async () => { + jest.useFakeTimers({ advanceTimers: true }) analytics = new Analytics( { writeKey: options.apiKey }, { retryQueue: true } @@ -178,7 +183,7 @@ describe('retryQueue', () => { await analytics.register(segment, envEnrichment) await analytics.track('foo') - await new Promise((resolve) => setTimeout(resolve, 1000)) + jest.runAllTimers() expect(fetch.mock.calls.length).toBeGreaterThanOrEqual(2) }) }) From d4580340029784cbd3059e480668c7230c73a4aa Mon Sep 17 00:00:00 2001 From: Daniel Jackins Date: Wed, 16 Apr 2025 10:58:18 -0600 Subject: [PATCH 4/4] test cleanup --- .../src/plugins/segmentio/__tests__/retries.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts b/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts index b200ed2dd..c5c002b95 100644 --- a/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts +++ b/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts @@ -140,7 +140,7 @@ describe('retryQueue', () => { let analytics: Analytics let segment: Plugin beforeEach(async () => { - jest.useRealTimers() + jest.useFakeTimers({ advanceTimers: true }) jest.resetAllMocks() jest.restoreAllMocks() @@ -150,9 +150,11 @@ describe('retryQueue', () => { fetch.mockReturnValue(createError({ status: 500 })) }) + afterEach(() => { + jest.useRealTimers() + }) it('Only attempts once if retryQueue is false', async () => { - jest.useFakeTimers({ advanceTimers: true }) analytics = new Analytics( { writeKey: options.apiKey }, { retryQueue: false } @@ -170,7 +172,6 @@ describe('retryQueue', () => { }) it('Attempts multiple times if retryQueue is true', async () => { - jest.useFakeTimers({ advanceTimers: true }) analytics = new Analytics( { writeKey: options.apiKey }, { retryQueue: true }