Skip to content

Commit

Permalink
Merge pull request #2055 from bugsnag/PLAT-11296/fix-xhr-monkey-patch
Browse files Browse the repository at this point in the history
  • Loading branch information
@yousif-bugsnag
yousif-bugsnag committed Dec 14, 2023
2 parents 7892f3d + fe52193 commit c9ccb54
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 63 deletions.
56 changes: 28 additions & 28 deletions packages/plugin-network-breadcrumbs/network-breadcrumbs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,45 +21,45 @@ module.exports = (_ignoredUrls = [], win = window) => {

// XMLHttpRequest monkey patch
function monkeyPatchXMLHttpRequest () {
if (!('addEventListener' in win.XMLHttpRequest.prototype)) return
const nativeOpen = win.XMLHttpRequest.prototype.open
if (!('addEventListener' in win.XMLHttpRequest.prototype) || !('WeakMap' in win)) return

// override native open()
win.XMLHttpRequest.prototype.open = function open (method, url) {
let requestSetupKey = false

const error = () => handleXHRError(method, url, getDuration(requestStart))
const load = () => handleXHRLoad(method, url, this.status, getDuration(requestStart))

// if we have already setup listeners, it means open() was called twice, we need to remove
// the listeners and recreate them
if (requestSetupKey) {
this.removeEventListener('load', load)
this.removeEventListener('error', error)
}
const trackedRequests = new WeakMap()
const requestHandlers = new WeakMap()

// attach load event listener
this.addEventListener('load', load)
// attach error event listener
this.addEventListener('error', error)
const originalOpen = win.XMLHttpRequest.prototype.open
win.XMLHttpRequest.prototype.open = function open (method, url) {
trackedRequests.set(this, { method, url })
originalOpen.apply(this, arguments)
}

requestSetupKey = true
const originalSend = win.XMLHttpRequest.prototype.send
win.XMLHttpRequest.prototype.send = function send (body) {
const requestData = trackedRequests.get(this)
if (requestData) {
// if we have already setup listeners then this request instance is being reused,
// so we need to remove the listeners from the previous send
const listeners = requestHandlers.get(this)
if (listeners) {
this.removeEventListener('load', listeners.load)
this.removeEventListener('error', listeners.error)
}

const oldSend = this.send
let requestStart
const requestStart = new Date()
const error = () => handleXHRError(requestData.method, requestData.url, getDuration(requestStart))
const load = () => handleXHRLoad(requestData.method, requestData.url, this.status, getDuration(requestStart))

// override send() for this XMLHttpRequest instance
this.send = function send () {
requestStart = new Date()
oldSend.apply(this, arguments)
this.addEventListener('load', load)
this.addEventListener('error', error)
requestHandlers.set(this, { load, error })
}

nativeOpen.apply(this, arguments)
originalSend.apply(this, arguments)
}

if (process.env.NODE_ENV !== 'production') {
restoreFunctions.push(() => {
win.XMLHttpRequest.prototype.open = nativeOpen
win.XMLHttpRequest.prototype.open = originalOpen
win.XMLHttpRequest.prototype.send = originalSend
})
}
}
Expand Down
124 changes: 91 additions & 33 deletions packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import Client from '@bugsnag/core/client'
import { Config } from '@bugsnag/core'

class XMLHttpRequest {
_listeners: { load: () => void, error: () => void }
_listeners: { load: Array<() => void>, error: Array<() => void> }
status: number | null;

constructor () {
this._listeners = { load: () => {}, error: () => {} }
this._listeners = { load: [], error: [] }
this.status = null
}

Expand All @@ -18,20 +18,22 @@ class XMLHttpRequest {

send (fail: boolean, status: number | null = null) {
if (fail) {
this._listeners.error.call(this)
this._listeners.error.map(fn => fn())
} else {
this.status = status
this._listeners.load.call(this)
this._listeners.load.map(fn => fn())
}
}

addEventListener (evt: 'load'| 'error', listener: () => void) {
this._listeners[evt] = listener
this._listeners[evt].push(listener)
}

removeEventListener (evt: 'load'| 'error', listener: () => void) {
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
if (listener === this._listeners[evt]) delete this._listeners[evt]
for (let i = this._listeners[evt].length - 1; i >= 0; i--) {
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
if (listener.name === this._listeners[evt][i].name) delete this._listeners[evt][i]
}
}
}

Expand Down Expand Up @@ -65,7 +67,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should leave a breadcrumb when an XMLHTTPRequest resolves', () => {
const window = { XMLHttpRequest } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -88,21 +90,77 @@ describe('plugin: network breadcrumbs', () => {
}))
})

it('should not leave duplicate breadcrumbs if open() is called twice', () => {
const window = { XMLHttpRequest } as unknown as Window & typeof globalThis
it('should not leave duplicate breadcrumbs if open() is called twice (open -> open -> send)', () => {
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })

const request = new window.XMLHttpRequest() as unknown as XMLHttpRequest
request.open('GET', '/')
request.open('GET', '/')
request.open('POST', 'https://example.com')
request.send(false, 200)
expect(client._breadcrumbs.length).toBe(1)
expect(client._breadcrumbs[0]).toEqual(expect.objectContaining({
type: 'request',
message: 'XMLHttpRequest succeeded',
metadata: {
status: 200,
method: 'POST',
url: 'https://example.com',
duration: expect.any(Number)
}
}))
})

it('should not leave duplicate breadcrumbs if send() is called twice (open -> send -> open -> send)', () => {
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })

const request = new window.XMLHttpRequest() as unknown as XMLHttpRequest
jest.spyOn(request, 'addEventListener')
jest.spyOn(request, 'removeEventListener')

request.open('GET', '/')
request.send(false, 200)

expect(request.addEventListener).toHaveBeenCalledTimes(2)
expect(request.removeEventListener).not.toHaveBeenCalled()

request.open('POST', 'https://example.com')
request.send(false, 200)

expect(request.removeEventListener).toHaveBeenCalledTimes(2)
expect(request.addEventListener).toHaveBeenCalledTimes(4)

expect(client._breadcrumbs.length).toBe(2)
expect(client._breadcrumbs[0]).toEqual(expect.objectContaining({
type: 'request',
message: 'XMLHttpRequest succeeded',
metadata: {
status: 200,
method: 'GET',
url: '/',
duration: expect.any(Number)
}
}))

expect(client._breadcrumbs[1]).toEqual(expect.objectContaining({
type: 'request',
message: 'XMLHttpRequest succeeded',
metadata: {
status: 200,
method: 'POST',
url: 'https://example.com',
duration: expect.any(Number)
}
}))
})

it('should leave a breadcrumb when an XMLHTTPRequest has a failed response', () => {
const window = { XMLHttpRequest } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -125,7 +183,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should leave a breadcrumb when an XMLHTTPRequest has a network error', () => {
const window = { XMLHttpRequest } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -148,7 +206,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should not leave a breadcrumb for request to bugsnag notify endpoint', () => {
const window = { XMLHttpRequest } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] } as unknown as Config)
Expand All @@ -161,7 +219,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should not leave a breadcrumb for session tracking requests', () => {
const window = { XMLHttpRequest } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] } as unknown as Config)
Expand All @@ -173,7 +231,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should leave a breadcrumb when the request URL is not a string', () => {
const window = { XMLHttpRequest } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

const logger = {
debug: jest.fn(),
Expand Down Expand Up @@ -203,7 +261,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should leave a breadcrumb when the request URL is not a string for a request that errors', () => {
const window = { XMLHttpRequest } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

const logger = {
debug: jest.fn(),
Expand Down Expand Up @@ -232,7 +290,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should leave a breadcrumb when a fetch() resolves', (done) => {
const window = { XMLHttpRequest, fetch } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap, fetch } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -255,7 +313,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should handle a fetch(url, { method: null })', (done) => {
const window = { XMLHttpRequest, fetch } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap, fetch } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -278,7 +336,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should handle a fetch() request supplied with a Request object', (done) => {
const window = { XMLHttpRequest, fetch } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap, fetch } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -303,7 +361,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should handle a fetch() request supplied with a Request object that has a method specified', (done) => {
const window = { XMLHttpRequest, fetch } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap, fetch } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -328,7 +386,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should handle fetch(null)', (done) => {
const window = { XMLHttpRequest, fetch } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap, fetch } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -351,7 +409,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should handle fetch(url, null)', (done) => {
const window = { XMLHttpRequest, fetch } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap, fetch } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -374,7 +432,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should handle fetch(undefined)', (done) => {
const window = { XMLHttpRequest, fetch } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap, fetch } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -397,7 +455,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should handle a fetch(request, { method })', (done) => {
const window = { XMLHttpRequest, fetch } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap, fetch } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -420,7 +478,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should handle a fetch(request, { method: null })', (done) => {
const window = { XMLHttpRequest, fetch } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap, fetch } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -443,7 +501,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should handle a fetch(request, { method: undefined })', (done) => {
const window = { XMLHttpRequest, fetch } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap, fetch } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -466,7 +524,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should leave a breadcrumb when a fetch() has a failed response', (done) => {
const window = { XMLHttpRequest, fetch } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap, fetch } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -489,7 +547,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should leave a breadcrumb when a fetch() has a network error', (done) => {
const window = { XMLHttpRequest, fetch } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap, fetch } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand All @@ -511,7 +569,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should not be enabled when enabledBreadcrumbTypes=[]', () => {
const window = { XMLHttpRequest } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', enabledBreadcrumbTypes: [], plugins: [p] })
Expand All @@ -524,7 +582,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should be enabled when enabledBreadcrumbTypes=["request"]', () => {
const window = { XMLHttpRequest } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', enabledBreadcrumbTypes: ['request'], plugins: [p] })
Expand All @@ -537,7 +595,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should be enabled when enabledBreadcrumbTypes=null', () => {
const window = { XMLHttpRequest } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

p = plugin([], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', enabledBreadcrumbTypes: null, plugins: [p] })
Expand All @@ -550,7 +608,7 @@ describe('plugin: network breadcrumbs', () => {
})

it('should strip query string data before checking a url is ignored', () => {
const window = { XMLHttpRequest } as unknown as Window & typeof globalThis
const window = { XMLHttpRequest, WeakMap } as unknown as Window & typeof globalThis

p = plugin(['/ignoreme'], window)
const client = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [p] })
Expand Down
Loading

0 comments on commit c9ccb54

Please sign in to comment.