Skip to content

Commit db3484d

Browse files
fix(plugin-cloudflare-workers): use request scoped client and run within async context (#2644)
1 parent d58e520 commit db3484d

File tree

3 files changed

+238
-46
lines changed

3 files changed

+238
-46
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
### Added
66

7-
(plugin-cloudflare-workers): Add initial support for Cloudflare Workers [#2643](https://github.com/bugsnag/bugsnag-js/pull/2643)
7+
(plugin-cloudflare-workers): Add initial support for Cloudflare Workers [#2643](https://github.com/bugsnag/bugsnag-js/pull/2643) [#2644](https://github.com/bugsnag/bugsnag-js/pull/2644)
88

99
### Fixed
1010

packages/plugin-cloudflare-workers/src/index.js

Lines changed: 48 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const bugsnagInFlight = require('@bugsnag/in-flight')
22
const BugsnagPluginBrowserSession = require('@bugsnag/plugin-browser-session')
3+
const clone = require('@bugsnag/core/lib/clone-client')
34

45
const SERVER_PLUGIN_NAMES = ['express', 'koa', 'restify', 'hono']
56
const isServerPluginLoaded = client => SERVER_PLUGIN_NAMES.some(name => client.getPlugin(name))
@@ -42,13 +43,6 @@ const BugsnagPluginCloudflareWorkers = {
4243
bugsnagInFlight.trackInFlight(client)
4344
client._loadPlugin(BugsnagPluginBrowserSession)
4445

45-
// Reset the app duration between invocations, if the plugin is loaded
46-
const appDurationPlugin = client.getPlugin('appDuration')
47-
48-
if (appDurationPlugin) {
49-
appDurationPlugin.reset()
50-
}
51-
5246
return {
5347
createHandler ({ flushTimeoutMs = 2000 } = {}) {
5448
return wrapHandler.bind(null, client, flushTimeoutMs)
@@ -59,54 +53,65 @@ const BugsnagPluginCloudflareWorkers = {
5953

6054
function wrapHandler (client, flushTimeoutMs, handler) {
6155
return async function (request, env, ctx) {
62-
// Add request metadata via onError callback so server plugins can override
63-
// Only add metadata if no server plugin is loaded
56+
// Reset the app duration between invocations, if the plugin is loaded
57+
const appDurationPlugin = client.getPlugin('appDuration')
58+
59+
if (appDurationPlugin) {
60+
appDurationPlugin.reset()
61+
}
62+
63+
// clone the client to be scoped to this request.
64+
const requestClient = clone(client)
65+
66+
// only start a session and add request metadata if no server plugin is loaded,
67+
// as those plugins will handle this themselves
6468
if (!isServerPluginLoaded(client)) {
65-
client.addOnError((event) => {
69+
if (requestClient._config.autoTrackSessions) {
70+
requestClient.startSession()
71+
}
72+
73+
requestClient.addOnError((event) => {
6674
const { metadata, request: requestData } = getRequestAndMetadataFromReq(request)
6775
event.request = { ...event.request, ...requestData }
6876
event.addMetadata('request', metadata)
6977
}, true)
7078
}
7179

72-
// Track sessions if autoTrackSessions is enabled and no server plugin is loaded
73-
if (client._config.autoTrackSessions && !isServerPluginLoaded(client)) {
74-
client.startSession()
75-
}
80+
return client._clientContext.run(requestClient, async () => {
81+
try {
82+
return await handler(request, env, ctx)
83+
} catch (err) {
84+
if (requestClient._config.autoDetectErrors && requestClient._config.enabledErrorTypes.unhandledExceptions) {
85+
const handledState = {
86+
severity: 'error',
87+
unhandled: true,
88+
severityReason: { type: 'unhandledException' }
89+
}
7690

77-
try {
78-
return await handler(request, env, ctx)
79-
} catch (err) {
80-
if (client._config.autoDetectErrors && client._config.enabledErrorTypes.unhandledExceptions) {
81-
const handledState = {
82-
severity: 'error',
83-
unhandled: true,
84-
severityReason: { type: 'unhandledException' }
85-
}
86-
87-
const event = client.Event.create(err, true, handledState, 'cloudflare workers plugin', 1)
91+
const event = requestClient.Event.create(err, true, handledState, 'cloudflare workers plugin', 1)
8892

89-
client._notify(event)
90-
}
93+
requestClient._notify(event)
94+
}
9195

92-
throw err
93-
} finally {
94-
// Use ctx.waitUntil to ensure flush completes even after response is returned
95-
// This is critical for Cloudflare Workers as they can terminate immediately
96-
if (ctx && typeof ctx.waitUntil === 'function') {
97-
ctx.waitUntil(
98-
bugsnagInFlight.flush(flushTimeoutMs).catch(err => {
99-
client._logger.error(`Delivery may be unsuccessful: ${err.message}`)
100-
})
101-
)
102-
} else {
103-
try {
104-
await bugsnagInFlight.flush(flushTimeoutMs)
105-
} catch (err) {
106-
client._logger.error(`Delivery may be unsuccessful: ${err.message}`)
96+
throw err
97+
} finally {
98+
// use ctx.waitUntil to ensure flush completes even after response is returned
99+
// this is critical for Cloudflare Workers as they can terminate immediately
100+
if (ctx && typeof ctx.waitUntil === 'function') {
101+
ctx.waitUntil(
102+
bugsnagInFlight.flush(flushTimeoutMs).catch(err => {
103+
requestClient._logger.error(`Delivery may be unsuccessful: ${err.message}`)
104+
})
105+
)
106+
} else {
107+
try {
108+
await bugsnagInFlight.flush(flushTimeoutMs)
109+
} catch (err) {
110+
requestClient._logger.error(`Delivery may be unsuccessful: ${err.message}`)
111+
}
107112
}
108113
}
109-
}
114+
})
110115
}
111116
}
112117

packages/plugin-cloudflare-workers/test/index.test.ts

Lines changed: 189 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ const createMockExecutionContext = (): MockExecutionContext => {
2323
} as unknown as MockExecutionContext
2424
}
2525

26-
const createClient = (events: EventDeliveryPayload[], sessions: SessionDeliveryPayload[], config = {}) => {
27-
const client = new Client({ apiKey: 'AN_API_KEY', plugins: [BugsnagPluginCloudflareWorkers], ...config })
26+
const createClient = (events: EventDeliveryPayload[], sessions: SessionDeliveryPayload[], config = {}, additionalPlugins: any[] = []) => {
27+
const client = new Client({ apiKey: 'AN_API_KEY', plugins: [BugsnagPluginCloudflareWorkers, ...additionalPlugins], ...config })
2828

2929
// @ts-ignore the following property is not defined on the public Event interface
3030
client.Event.__type = 'nodejs'
@@ -40,6 +40,12 @@ const createClient = (events: EventDeliveryPayload[], sessions: SessionDeliveryP
4040
}
4141
}
4242

43+
// Mock _clientContext.run to execute the callback immediately
44+
// This simulates AsyncLocalStorage behavior for testing
45+
client._clientContext = {
46+
run: jest.fn((requestClient, callback) => callback())
47+
}
48+
4349
return client
4450
}
4551

@@ -97,6 +103,13 @@ describe('plugin: cloudflare workers', () => {
97103
// Wait for promises registered with ctx.waitUntil to complete
98104
await ctx._waitForAllPromises()
99105

106+
// Verify _clientContext.run was called with a cloned client
107+
expect(client._clientContext.run).toHaveBeenCalledTimes(1)
108+
expect(client._clientContext.run).toHaveBeenCalledWith(
109+
expect.any(Client),
110+
expect.any(Function)
111+
)
112+
100113
expect(events).toHaveLength(1)
101114

102115
const event = events[0].events[0]
@@ -139,6 +152,11 @@ describe('plugin: cloudflare workers', () => {
139152
}
140153
}
141154

155+
// Mock _clientContext.run to execute the callback immediately
156+
client._clientContext = {
157+
run: jest.fn((requestClient, callback) => callback())
158+
}
159+
142160
const timeoutError = new Error('flush timed out after 20ms')
143161

144162
const plugin = client.getPlugin('cloudflareWorkers')
@@ -200,6 +218,13 @@ describe('plugin: cloudflare workers', () => {
200218
const response = await exportedHandler.fetch?.(request, env, ctx)
201219
expect(await response?.text()).toBe('Hello World! test value')
202220

221+
// Verify _clientContext.run was called
222+
expect(client._clientContext.run).toHaveBeenCalledTimes(1)
223+
expect(client._clientContext.run).toHaveBeenCalledWith(
224+
expect.any(Client),
225+
expect.any(Function)
226+
)
227+
203228
expect(events).toHaveLength(0)
204229
expect(sessions).toHaveLength(1)
205230
})
@@ -235,6 +260,9 @@ describe('plugin: cloudflare workers', () => {
235260
// Wait for promises registered with ctx.waitUntil to complete
236261
await ctx._waitForAllPromises()
237262

263+
// Verify _clientContext.run was called
264+
expect(client._clientContext.run).toHaveBeenCalledTimes(1)
265+
238266
expect(events).toHaveLength(1)
239267

240268
const event = events[0].events[0]
@@ -313,4 +341,163 @@ describe('plugin: cloudflare workers', () => {
313341
expect(events).toHaveLength(0)
314342
expect(sessions).toHaveLength(1)
315343
})
344+
345+
it('clones the client for each request to avoid callback accumulation', async () => {
346+
const events: EventDeliveryPayload[] = []
347+
const sessions: SessionDeliveryPayload[] = []
348+
349+
const client = createClient(events, sessions)
350+
351+
const plugin = client.getPlugin('cloudflareWorkers')
352+
353+
if (!plugin) {
354+
throw new Error('Plugin was not loaded!')
355+
}
356+
357+
const bugsnagHandler = plugin.createHandler()
358+
359+
const exportedHandler: ExportedHandler<Env> = {
360+
fetch: bugsnagHandler(async (request, env, ctx) => {
361+
throw new Error('Test error')
362+
})
363+
}
364+
365+
const request1 = new Request('https://example.com/request1?param1=value1', {
366+
method: 'GET',
367+
headers: {
368+
'X-Custom-Header': 'request1'
369+
}
370+
}) as unknown as CloudflareRequest<unknown, IncomingRequestCfProperties<unknown>>
371+
const request2 = new Request('https://example.com/request2?param2=value2', {
372+
method: 'POST',
373+
headers: {
374+
'X-Custom-Header': 'request2'
375+
}
376+
}) as unknown as CloudflareRequest<unknown, IncomingRequestCfProperties<unknown>>
377+
const env = {}
378+
const ctx1 = createMockExecutionContext()
379+
const ctx2 = createMockExecutionContext()
380+
381+
// Make two requests (both will throw errors)
382+
await expect(exportedHandler.fetch?.(request1, env, ctx1)).rejects.toThrow('Test error')
383+
await expect(exportedHandler.fetch?.(request2, env, ctx2)).rejects.toThrow('Test error')
384+
385+
// Wait for promises registered with ctx.waitUntil to complete
386+
await ctx1._waitForAllPromises()
387+
await ctx2._waitForAllPromises()
388+
389+
// Verify _clientContext.run was called twice with different cloned clients
390+
expect(client._clientContext.run).toHaveBeenCalledTimes(2)
391+
392+
const firstCallClient = (client._clientContext.run as jest.Mock).mock.calls[0][0]
393+
const secondCallClient = (client._clientContext.run as jest.Mock).mock.calls[1][0]
394+
395+
// Both should be Client instances but different instances
396+
expect(firstCallClient).toBeInstanceOf(Client)
397+
expect(secondCallClient).toBeInstanceOf(Client)
398+
expect(firstCallClient).not.toBe(secondCallClient)
399+
expect(firstCallClient).not.toBe(client)
400+
expect(secondCallClient).not.toBe(client)
401+
402+
// Both requests should have started sessions
403+
expect(sessions).toHaveLength(2)
404+
405+
// Verify two events were sent
406+
expect(events).toHaveLength(2)
407+
408+
// Verify first event has correct request metadata
409+
const event1 = events[0].events[0]
410+
expect(event1.request).toMatchObject({
411+
url: 'https://example.com/request1?param1=value1',
412+
httpMethod: 'GET',
413+
headers: expect.objectContaining({
414+
'x-custom-header': 'request1'
415+
})
416+
})
417+
// @ts-ignore
418+
expect(event1._metadata?.request).toMatchObject({
419+
url: 'https://example.com/request1?param1=value1',
420+
path: '/request1',
421+
httpMethod: 'GET',
422+
query: {
423+
param1: 'value1'
424+
},
425+
headers: expect.objectContaining({
426+
'x-custom-header': 'request1'
427+
})
428+
})
429+
430+
// Verify second event has correct request metadata
431+
const event2 = events[1].events[0]
432+
expect(event2.request).toMatchObject({
433+
url: 'https://example.com/request2?param2=value2',
434+
httpMethod: 'POST',
435+
headers: expect.objectContaining({
436+
'x-custom-header': 'request2'
437+
})
438+
})
439+
// @ts-ignore
440+
expect(event2._metadata?.request).toMatchObject({
441+
url: 'https://example.com/request2?param2=value2',
442+
path: '/request2',
443+
httpMethod: 'POST',
444+
query: {
445+
param2: 'value2'
446+
},
447+
headers: expect.objectContaining({
448+
'x-custom-header': 'request2'
449+
})
450+
})
451+
})
452+
453+
it('resets app duration plugin between requests', async () => {
454+
const events: EventDeliveryPayload[] = []
455+
const sessions: SessionDeliveryPayload[] = []
456+
457+
// Mock the app duration plugin
458+
const mockReset = jest.fn()
459+
const mockAppDurationPlugin = {
460+
name: 'appDuration',
461+
load: () => ({
462+
reset: mockReset
463+
})
464+
}
465+
466+
const client = createClient(events, sessions, {}, [mockAppDurationPlugin])
467+
468+
const plugin = client.getPlugin('cloudflareWorkers')
469+
470+
if (!plugin) {
471+
throw new Error('Plugin was not loaded!')
472+
}
473+
474+
const bugsnagHandler = plugin.createHandler()
475+
476+
const exportedHandler: ExportedHandler<Env> = {
477+
fetch: bugsnagHandler(async (request, env, ctx) => {
478+
return new Response('OK') as unknown as CloudflareResponse
479+
})
480+
}
481+
482+
const request1 = new Request('https://example.com/request1') as unknown as CloudflareRequest<unknown, IncomingRequestCfProperties<unknown>>
483+
const request2 = new Request('https://example.com/request2') as unknown as CloudflareRequest<unknown, IncomingRequestCfProperties<unknown>>
484+
const request3 = new Request('https://example.com/request3') as unknown as CloudflareRequest<unknown, IncomingRequestCfProperties<unknown>>
485+
const env = {}
486+
const ctx1 = createMockExecutionContext()
487+
const ctx2 = createMockExecutionContext()
488+
const ctx3 = createMockExecutionContext()
489+
490+
// Make three requests
491+
await exportedHandler.fetch?.(request1, env, ctx1)
492+
await exportedHandler.fetch?.(request2, env, ctx2)
493+
await exportedHandler.fetch?.(request3, env, ctx3)
494+
495+
// Wait for all promises to complete
496+
await ctx1._waitForAllPromises()
497+
await ctx2._waitForAllPromises()
498+
await ctx3._waitForAllPromises()
499+
500+
// Verify reset was called for each request
501+
expect(mockReset).toHaveBeenCalledTimes(3)
502+
})
316503
})

0 commit comments

Comments
 (0)