Skip to content

Commit b9c6356

Browse files
chrisradekchrisradek
authored
Updates analytics error handling to support checking delivery failures on Context (#505)
* add tests to ensure plugin errors do not bubble to analytics API calls * fix vscode launch configurations * add ContextCancelation retry tests and improve plugin error behavior integration tests * add ContextCancelation support to before plugins and prevent bubbling before plugin errors to analytics API calls * add Context.failedDelivery() to check if an event failed to be delivered * only set failed delivery on context on final attempt * add changeset
1 parent 908f47d commit b9c6356

File tree

8 files changed

+394
-26
lines changed

8 files changed

+394
-26
lines changed

‎.changeset/odd-flies-protect.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@segment/analytics-next': minor
3+
---
4+
5+
Adds `context.failedDelivery()` to improve detection of events that could not be delivered due to plugin errors.

‎.vscode/launch.json‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,15 @@
66
{
77
"type": "node",
88
"request": "launch",
9-
"name": "Jest Current File",
9+
"name": "(packages/browser) Jest Current File",
1010
"program": "${workspaceFolder}/node_modules/.bin/jest",
11-
"args": ["--testTimeout=100000", "--runTestsByPath", "${relativeFile}"],
11+
"args": [
12+
"--testTimeout=100000",
13+
"-c",
14+
"${workspaceFolder}/packages/browser/jest.config.js",
15+
"--runTestsByPath",
16+
"${relativeFile}"
17+
],
1218
"console": "integratedTerminal",
1319
"internalConsoleOptions": "neverOpen",
1420
"skipFiles": [
@@ -18,9 +24,13 @@
1824
},
1925
{
2026
"type": "node",
21-
"name": "Jest All",
27+
"name": "(packages/browser) Jest All",
2228
"request": "launch",
23-
"args": ["--runInBand"],
29+
"args": [
30+
"--runInBand",
31+
"-c",
32+
"${workspaceFolder}/packages/browser/jest.config.js"
33+
],
2434
"cwd": "${workspaceFolder}",
2535
"console": "integratedTerminal",
2636
"internalConsoleOptions": "neverOpen",

‎packages/browser/src/core/analytics/__tests__/integration.test.ts‎

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
import { PriorityQueue } from '../../../lib/priority-queue'
2+
import { MiddlewareParams } from '../../../plugins/middleware'
3+
import { Context } from '../../context'
4+
import { Plugin } from '../../plugin'
5+
import { EventQueue } from '../../queue/event-queue'
6+
import { Analytics } from '../index'
7+
import {
8+
AfterPlugin,
9+
BeforePlugin,
10+
DestinationPlugin,
11+
EnrichmentPlugin,
12+
} from './test-plugins'
13+
14+
describe('Analytics', () => {
15+
describe('plugin error behavior', () => {
16+
const retriesEnabledScenarios = [true, false]
17+
retriesEnabledScenarios.forEach((retriesEnabled) => {
18+
describe(`with retries ${
19+
retriesEnabled ? 'enabled' : 'disabled'
20+
}`, () => {
21+
let analytics: Analytics
22+
const attemptCount = retriesEnabled ? 2 : 1
23+
24+
beforeEach(() => {
25+
const queue = new EventQueue(new PriorityQueue(attemptCount, []))
26+
analytics = new Analytics({ writeKey: 'writeKey' }, {}, queue)
27+
})
28+
29+
// Indicates plugins should throw
30+
const shouldThrow = true
31+
32+
it(`"before" plugin errors should not throw (single dispatched event)`, async () => {
33+
const plugin = new BeforePlugin({ shouldThrow })
34+
const trackSpy = jest.spyOn(plugin, 'track')
35+
36+
await analytics.register(plugin)
37+
const context = await analytics.track('test')
38+
expect(trackSpy).toHaveBeenCalledTimes(attemptCount)
39+
expect(context).toBeInstanceOf(Context)
40+
expect(context.failedDelivery()).toBeTruthy()
41+
})
42+
43+
it(`"before" plugin errors should not throw (multiple dispatched events)`, async () => {
44+
const plugin = new BeforePlugin({ shouldThrow })
45+
const trackSpy = jest.spyOn(plugin, 'track')
46+
47+
await analytics.register(plugin)
48+
const contexts = await Promise.all([
49+
analytics.track('test1'),
50+
analytics.track('test2'),
51+
analytics.track('test3'),
52+
])
53+
expect(trackSpy).toHaveBeenCalledTimes(3 * attemptCount)
54+
55+
for (const context of contexts) {
56+
expect(context).toBeInstanceOf(Context)
57+
expect(context.failedDelivery()).toBeTruthy()
58+
}
59+
})
60+
61+
it(`"before" plugin errors should not impact callbacks`, async () => {
62+
const plugin = new BeforePlugin({ shouldThrow })
63+
const trackSpy = jest.spyOn(plugin, 'track')
64+
65+
await analytics.register(plugin)
66+
67+
const context = await new Promise<Context>((resolve) => {
68+
void analytics.track('test', resolve)
69+
})
70+
71+
expect(trackSpy).toHaveBeenCalledTimes(attemptCount)
72+
expect(context).toBeInstanceOf(Context)
73+
expect(context.failedDelivery()).toBeTruthy()
74+
})
75+
76+
if (retriesEnabled) {
77+
it(`will not mark delivery failed if retry succeeds`, async () => {
78+
const plugin: Plugin = {
79+
name: 'Test Retries Plugin',
80+
type: 'before',
81+
version: '1.0.0',
82+
isLoaded: () => true,
83+
load: () => Promise.resolve(),
84+
track: jest.fn((ctx) => {
85+
if (ctx.attempts < attemptCount) {
86+
throw new Error(`Plugin failed!`)
87+
}
88+
return ctx
89+
}),
90+
}
91+
await analytics.register(plugin)
92+
93+
const context = await analytics.track('test')
94+
expect(plugin.track).toHaveBeenCalledTimes(attemptCount)
95+
expect(context).toBeInstanceOf(Context)
96+
expect(context.failedDelivery()).toBeFalsy()
97+
})
98+
}
99+
100+
const testPlugins = [
101+
new EnrichmentPlugin({ shouldThrow }),
102+
new DestinationPlugin({ shouldThrow }),
103+
new AfterPlugin({ shouldThrow }),
104+
]
105+
testPlugins.forEach((plugin) => {
106+
it(`"${plugin.type}" plugin errors should not throw (single dispatched event)`, async () => {
107+
const trackSpy = jest.spyOn(plugin, 'track')
108+
109+
await analytics.register(plugin)
110+
111+
const context = await analytics.track('test')
112+
expect(trackSpy).toHaveBeenCalledTimes(1)
113+
expect(context).toBeInstanceOf(Context)
114+
expect(context.failedDelivery()).toBeFalsy()
115+
})
116+
117+
it(`"${plugin.type}" plugin errors should not throw (multiple dispatched events)`, async () => {
118+
const trackSpy = jest.spyOn(plugin, 'track')
119+
120+
await analytics.register(plugin)
121+
122+
const contexts = await Promise.all([
123+
analytics.track('test1'),
124+
analytics.track('test2'),
125+
analytics.track('test3'),
126+
])
127+
128+
expect(trackSpy).toHaveBeenCalledTimes(3)
129+
for (const context of contexts) {
130+
expect(context).toBeInstanceOf(Context)
131+
expect(context.failedDelivery()).toBeFalsy()
132+
}
133+
})
134+
135+
it(`"${plugin.type}" plugin errors should not impact callbacks`, async () => {
136+
const trackSpy = jest.spyOn(plugin, 'track')
137+
138+
await analytics.register(plugin)
139+
140+
const context = await new Promise<Context>((resolve) => {
141+
void analytics.track('test', resolve)
142+
})
143+
144+
expect(trackSpy).toHaveBeenCalledTimes(1)
145+
expect(context).toBeInstanceOf(Context)
146+
expect(context.failedDelivery()).toBeFalsy()
147+
})
148+
})
149+
150+
it('"before" plugin supports cancelation (single dispatched event)', async () => {
151+
const plugin = new BeforePlugin({ shouldCancel: true })
152+
const trackSpy = jest.spyOn(plugin, 'track')
153+
154+
await analytics.register(plugin)
155+
156+
const context = await analytics.track('test')
157+
expect(trackSpy).toHaveBeenCalledTimes(1)
158+
expect(context).toBeInstanceOf(Context)
159+
expect(context.failedDelivery()).toBeTruthy()
160+
})
161+
162+
it('"before" plugin supports cancelation (multiple dispatched events)', async () => {
163+
const plugin = new BeforePlugin({ shouldCancel: true })
164+
const trackSpy = jest.spyOn(plugin, 'track')
165+
166+
await analytics.register(plugin)
167+
const contexts = await Promise.all([
168+
analytics.track('test1'),
169+
analytics.track('test2'),
170+
analytics.track('test3'),
171+
])
172+
expect(trackSpy).toHaveBeenCalledTimes(3)
173+
174+
for (const context of contexts) {
175+
expect(context).toBeInstanceOf(Context)
176+
expect(context.failedDelivery()).toBeTruthy()
177+
}
178+
})
179+
180+
it(`source middleware should not throw when "next" not called`, async () => {
181+
const sourceMiddleware = jest.fn<void, MiddlewareParams[]>(() => {})
182+
183+
await analytics.addSourceMiddleware(sourceMiddleware)
184+
185+
const context = await analytics.track('test')
186+
187+
expect(sourceMiddleware).toHaveBeenCalledTimes(1)
188+
expect(context).toBeInstanceOf(Context)
189+
expect(context.failedDelivery()).toBeTruthy()
190+
})
191+
192+
it(`source middleware errors should not throw`, async () => {
193+
const sourceMiddleware = jest.fn<void, MiddlewareParams[]>(() => {
194+
throw new Error('Source middleware error')
195+
})
196+
197+
await analytics.addSourceMiddleware(sourceMiddleware)
198+
199+
const context = await analytics.track('test')
200+
201+
expect(sourceMiddleware).toHaveBeenCalledTimes(1 * attemptCount)
202+
expect(context).toBeInstanceOf(Context)
203+
expect(context.failedDelivery()).toBeTruthy()
204+
})
205+
206+
it(`source middleware errors should not impact callbacks`, async () => {
207+
const sourceMiddleware = jest.fn<void, MiddlewareParams[]>(() => {
208+
throw new Error('Source middleware error')
209+
})
210+
211+
await analytics.addSourceMiddleware(sourceMiddleware)
212+
213+
const context = await new Promise<Context>((resolve) => {
214+
void analytics.track('test', resolve)
215+
})
216+
217+
expect(sourceMiddleware).toHaveBeenCalledTimes(1 * attemptCount)
218+
expect(context).toBeInstanceOf(Context)
219+
expect(context.failedDelivery()).toBeTruthy()
220+
})
221+
})
222+
})
223+
})
224+
})

‎packages/browser/src/core/analytics/__tests__/test-plugins.ts‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
import { Context, ContextCancelation, Plugin } from '../../../index'
2+
3+
export interface BasePluginOptions {
4+
shouldThrow?: boolean
5+
shouldCancel?: boolean
6+
}
7+
8+
class BasePlugin implements Partial<Plugin> {
9+
public version = '1.0.0'
10+
private shouldCancel: boolean
11+
private shouldThrow: boolean
12+
13+
constructor({
14+
shouldCancel = false,
15+
shouldThrow = false,
16+
}: BasePluginOptions) {
17+
this.shouldCancel = shouldCancel
18+
this.shouldThrow = shouldThrow
19+
}
20+
21+
isLoaded() {
22+
return true
23+
}
24+
25+
load() {
26+
return Promise.resolve()
27+
}
28+
29+
alias(ctx: Context): Context {
30+
return this.task(ctx)
31+
}
32+
33+
group(ctx: Context): Context {
34+
return this.task(ctx)
35+
}
36+
37+
identify(ctx: Context): Context {
38+
return this.task(ctx)
39+
}
40+
41+
page(ctx: Context): Context {
42+
return this.task(ctx)
43+
}
44+
45+
screen(ctx: Context): Context {
46+
return this.task(ctx)
47+
}
48+
49+
track(ctx: Context): Context {
50+
return this.task(ctx)
51+
}
52+
53+
private task(ctx: Context): Context {
54+
if (this.shouldCancel) {
55+
ctx.cancel(
56+
new ContextCancelation({
57+
retry: false,
58+
})
59+
)
60+
} else if (this.shouldThrow) {
61+
throw new Error(`Error thrown in task`)
62+
}
63+
return ctx
64+
}
65+
}
66+
67+
export class BeforePlugin extends BasePlugin implements Plugin {
68+
public name = 'Test Before Error'
69+
public type = 'before' as const
70+
}
71+
72+
export class EnrichmentPlugin extends BasePlugin implements Plugin {
73+
public name = 'Test Enrichment Error'
74+
public type = 'enrichment' as const
75+
}
76+
77+
export class DestinationPlugin extends BasePlugin implements Plugin {
78+
public name = 'Test Destination Error'
79+
public type = 'destination' as const
80+
81+
public ready() {
82+
return Promise.resolve(true)
83+
}
84+
}
85+
86+
export class AfterPlugin extends BasePlugin implements Plugin {
87+
public name = 'Test After Error'
88+
public type = 'after' as const
89+
}

‎packages/browser/src/core/context/index.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ interface CancelationOptions {
2424
type?: string
2525
}
2626

27+
export interface ContextFailedDelivery {
28+
reason: unknown
29+
}
30+
2731
export class ContextCancelation {
2832
retry: boolean
2933
type: string
@@ -44,6 +48,7 @@ export class Context implements AbstractContext {
4448
public logger = new Logger()
4549
public stats: Stats
4650
private _id: string
51+
private _failedDelivery?: ContextFailedDelivery
4752

4853
constructor(event: SegmentEvent, id?: string) {
4954
this._attempts = 0
@@ -110,6 +115,14 @@ export class Context implements AbstractContext {
110115
return this._event
111116
}
112117

118+
public failedDelivery(): ContextFailedDelivery | undefined {
119+
return this._failedDelivery
120+
}
121+
122+
public setFailedDelivery(options: ContextFailedDelivery) {
123+
this._failedDelivery = options
124+
}
125+
113126
public logs(): LogMessage[] {
114127
return this.logger.logs
115128
}

0 commit comments

Comments
 (0)
⚡