From 5e8a92b4c78193d94bb7d85339484cc1a886fe8c Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Wed, 4 Dec 2019 10:24:59 +0000 Subject: [PATCH 1/5] fix(plugin-network-breadcrumbs): fetch accepts Request instance as an argument --- .../network-breadcrumbs.js | 16 +++++- .../test/network-breadcrumbs.test.js | 55 +++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/packages/plugin-network-breadcrumbs/network-breadcrumbs.js b/packages/plugin-network-breadcrumbs/network-breadcrumbs.js index 1915b05ec2..706c1b527b 100644 --- a/packages/plugin-network-breadcrumbs/network-breadcrumbs.js +++ b/packages/plugin-network-breadcrumbs/network-breadcrumbs.js @@ -120,11 +120,21 @@ const monkeyPatchFetch = () => { const oldFetch = win.fetch win.fetch = function fetch (...args) { - let [url, options] = args + const [urlOrRequest, options] = args + let method = 'GET' - if (options && options.method) { - method = options.method + let url + + if (typeof urlOrRequest === 'string') { + url = urlOrRequest + if (options && options.method) { + method = options.method + } + } else { + url = urlOrRequest.url + method = urlOrRequest.method } + return new Promise((resolve, reject) => { // pass through to native fetch oldFetch(...args) diff --git a/packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.js b/packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.js index 9b7074daf6..08c0a43bd2 100644 --- a/packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.js +++ b/packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.js @@ -27,6 +27,7 @@ XMLHttpRequest.prototype.removeEventListener = function (evt, listener) { if (listener === this._listeners[evt]) delete this._listeners[evt] } +// mock fetch function fetch (url, options, fail, status) { return new Promise((resolve, reject) => { if (fail) { @@ -37,6 +38,12 @@ function fetch (url, options, fail, status) { }) } +// mock (fetch) Request +function Request (url, opts) { + this.url = url + this.method = (opts && opts.method) || 'GET' +} + describe('plugin: network breadcrumbs', () => { afterEach(() => { // undo the global side effects @@ -179,6 +186,54 @@ describe('plugin: network breadcrumbs', () => { }) }) + it('should handle a fetch() request supplied with a Request object', (done) => { + const window = { XMLHttpRequest, fetch } + + const client = new Client(VALID_NOTIFIER) + client.setOptions({ apiKey: 'aaaa-aaaa-aaaa-aaaa' }) + client.configure() + client.use(plugin, () => [], window) + + const request = new Request('/') + + window.fetch(request, {}, false, 200).then(() => { + expect(client.breadcrumbs.length).toBe(1) + expect(client.breadcrumbs[0]).toEqual(jasmine.objectContaining({ + type: 'request', + name: 'fetch() succeeded', + metaData: { + status: 200, + request: 'GET /' + } + })) + done() + }) + }) + + it('should handle a fetch() request supplied with a Request object that has a method specified', (done) => { + const window = { XMLHttpRequest, fetch } + + const client = new Client(VALID_NOTIFIER) + client.setOptions({ apiKey: 'aaaa-aaaa-aaaa-aaaa' }) + client.configure() + client.use(plugin, () => [], window) + + const request = new Request('/', { method: 'PUT' }) + + window.fetch(request, {}, false, 200).then(() => { + expect(client.breadcrumbs.length).toBe(1) + expect(client.breadcrumbs[0]).toEqual(jasmine.objectContaining({ + type: 'request', + name: 'fetch() succeeded', + metaData: { + status: 200, + request: 'PUT /' + } + })) + done() + }) + }) + it('should leave a breadcrumb when a fetch() has a failed response', (done) => { const window = { XMLHttpRequest, fetch } From e2b822d90299c0543fa7af59d8cbba8433612ebf Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Thu, 5 Dec 2019 16:42:28 +0000 Subject: [PATCH 2/5] fix(plugin-network-breadcrumbs): improve robustness of fetch patch --- .../network-breadcrumbs.js | 22 ++- .../test/network-breadcrumbs.test.js | 132 ++++++++++++++++++ 2 files changed, 147 insertions(+), 7 deletions(-) diff --git a/packages/plugin-network-breadcrumbs/network-breadcrumbs.js b/packages/plugin-network-breadcrumbs/network-breadcrumbs.js index 706c1b527b..8ea88fd554 100644 --- a/packages/plugin-network-breadcrumbs/network-breadcrumbs.js +++ b/packages/plugin-network-breadcrumbs/network-breadcrumbs.js @@ -122,17 +122,25 @@ const monkeyPatchFetch = () => { win.fetch = function fetch (...args) { const [urlOrRequest, options] = args - let method = 'GET' - let url + let method + let url = null - if (typeof urlOrRequest === 'string') { - url = urlOrRequest - if (options && options.method) { + if (typeof urlOrRequest === 'object' && urlOrRequest !== null) { + url = urlOrRequest.url + if (options && 'method' in options) { method = options.method + } else if ('method' in urlOrRequest) { + method = urlOrRequest.method } } else { - url = urlOrRequest.url - method = urlOrRequest.method + url = urlOrRequest + if (options && 'method' in options) { + method = options.method + } + } + + if (method === undefined) { + method = 'GET' } return new Promise((resolve, reject) => { diff --git a/packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.js b/packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.js index 08c0a43bd2..f4633a842e 100644 --- a/packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.js +++ b/packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.js @@ -186,6 +186,28 @@ describe('plugin: network breadcrumbs', () => { }) }) + it('should handle a fetch(url, { method: null })', (done) => { + const window = { XMLHttpRequest, fetch } + + const client = new Client(VALID_NOTIFIER) + client.setOptions({ apiKey: 'aaaa-aaaa-aaaa-aaaa' }) + client.configure() + client.use(plugin, () => [], window) + + window.fetch('/', { method: null }, false, 405).then(() => { + expect(client.breadcrumbs.length).toBe(1) + expect(client.breadcrumbs[0]).toEqual(jasmine.objectContaining({ + type: 'request', + name: 'fetch() failed', + metaData: { + status: 405, + request: 'null /' + } + })) + done() + }) + }) + it('should handle a fetch() request supplied with a Request object', (done) => { const window = { XMLHttpRequest, fetch } @@ -234,6 +256,116 @@ describe('plugin: network breadcrumbs', () => { }) }) + it('should handle fetch(null)', (done) => { + const window = { XMLHttpRequest, fetch } + + const client = new Client(VALID_NOTIFIER) + client.setOptions({ apiKey: 'aaaa-aaaa-aaaa-aaaa' }) + client.configure() + client.use(plugin, () => [], window) + + window.fetch(null, {}, false, 404).then(() => { + expect(client.breadcrumbs.length).toBe(1) + expect(client.breadcrumbs[0]).toEqual(jasmine.objectContaining({ + type: 'request', + name: 'fetch() failed', + metaData: { + status: 404, + request: 'GET null' + } + })) + done() + }) + }) + + it('should handle fetch(undefined)', (done) => { + const window = { XMLHttpRequest, fetch } + + const client = new Client(VALID_NOTIFIER) + client.setOptions({ apiKey: 'aaaa-aaaa-aaaa-aaaa' }) + client.configure() + client.use(plugin, () => [], window) + + window.fetch(undefined, {}, false, 404).then(() => { + expect(client.breadcrumbs.length).toBe(1) + expect(client.breadcrumbs[0]).toEqual(jasmine.objectContaining({ + type: 'request', + name: 'fetch() failed', + metaData: { + status: 404, + request: 'GET undefined' + } + })) + done() + }) + }) + + it('should handle a fetch(request, { method })', (done) => { + const window = { XMLHttpRequest, fetch } + + const client = new Client(VALID_NOTIFIER) + client.setOptions({ apiKey: 'aaaa-aaaa-aaaa-aaaa' }) + client.configure() + client.use(plugin, () => [], window) + + window.fetch(new Request('/foo', { method: 'GET' }), { method: 'PUT' }, false, 200).then(() => { + expect(client.breadcrumbs.length).toBe(1) + expect(client.breadcrumbs[0]).toEqual(jasmine.objectContaining({ + type: 'request', + name: 'fetch() succeeded', + metaData: { + status: 200, + request: 'PUT /foo' + } + })) + done() + }) + }) + + it('should handle a fetch(request, { method: null })', (done) => { + const window = { XMLHttpRequest, fetch } + + const client = new Client(VALID_NOTIFIER) + client.setOptions({ apiKey: 'aaaa-aaaa-aaaa-aaaa' }) + client.configure() + client.use(plugin, () => [], window) + + window.fetch(new Request('/foo'), { method: null }, false, 405).then(() => { + expect(client.breadcrumbs.length).toBe(1) + expect(client.breadcrumbs[0]).toEqual(jasmine.objectContaining({ + type: 'request', + name: 'fetch() failed', + metaData: { + status: 405, + request: 'null /foo' + } + })) + done() + }) + }) + + it('should handle a fetch(request, { method: undefined })', (done) => { + const window = { XMLHttpRequest, fetch } + + const client = new Client(VALID_NOTIFIER) + client.setOptions({ apiKey: 'aaaa-aaaa-aaaa-aaaa' }) + client.configure() + client.use(plugin, () => [], window) + + window.fetch(new Request('/foo'), { method: undefined }, false, 200).then(() => { + expect(client.breadcrumbs.length).toBe(1) + expect(client.breadcrumbs[0]).toEqual(jasmine.objectContaining({ + type: 'request', + name: 'fetch() succeeded', + metaData: { + status: 200, + request: 'GET /foo' + } + })) + done() + }) + }) + it('should leave a breadcrumb when a fetch() has a failed response', (done) => { const window = { XMLHttpRequest, fetch } From 3f081c46eb2c25407454e970d2a60f2eb916b155 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Thu, 5 Dec 2019 16:46:20 +0000 Subject: [PATCH 3/5] chore: update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3df8a6ea11..9d7e88c3d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## TBD +- (plugin-network-breadcrumbs): Fixes the `window.fetch` monkey-patch to also accept `Request`. [#662](https://github.com/bugsnag/bugsnag-js/pull/662) + ## 6.4.3 (2019-10-21) ### Fixed From 316439b4253bced2b462a740a7c1afafab970566 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Thu, 5 Dec 2019 17:32:31 +0000 Subject: [PATCH 4/5] fix(plugin-network-breadcrumbs): improve robustness of fetch patch --- .../network-breadcrumbs.js | 4 ++-- .../test/network-breadcrumbs.test.js | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/plugin-network-breadcrumbs/network-breadcrumbs.js b/packages/plugin-network-breadcrumbs/network-breadcrumbs.js index 8ea88fd554..1c8689558e 100644 --- a/packages/plugin-network-breadcrumbs/network-breadcrumbs.js +++ b/packages/plugin-network-breadcrumbs/network-breadcrumbs.js @@ -125,11 +125,11 @@ const monkeyPatchFetch = () => { let method let url = null - if (typeof urlOrRequest === 'object' && urlOrRequest !== null) { + if (urlOrRequest && typeof urlOrRequest === 'object') { url = urlOrRequest.url if (options && 'method' in options) { method = options.method - } else if ('method' in urlOrRequest) { + } else if (urlOrRequest && 'method' in urlOrRequest) { method = urlOrRequest.method } } else { diff --git a/packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.js b/packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.js index f4633a842e..e3fb7e35bb 100644 --- a/packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.js +++ b/packages/plugin-network-breadcrumbs/test/network-breadcrumbs.test.js @@ -278,6 +278,28 @@ describe('plugin: network breadcrumbs', () => { }) }) + it('should handle fetch(url, null)', (done) => { + const window = { XMLHttpRequest, fetch } + + const client = new Client(VALID_NOTIFIER) + client.setOptions({ apiKey: 'aaaa-aaaa-aaaa-aaaa' }) + client.configure() + client.use(plugin, () => [], window) + + window.fetch('/', null, false, 200).then(() => { + expect(client.breadcrumbs.length).toBe(1) + expect(client.breadcrumbs[0]).toEqual(jasmine.objectContaining({ + type: 'request', + name: 'fetch() succeeded', + metaData: { + status: 200, + request: 'GET /' + } + })) + done() + }) + }) + it('should handle fetch(undefined)', (done) => { const window = { XMLHttpRequest, fetch } From 43f5d3a4952d6afd59c38ff37dbe17ab42a480b5 Mon Sep 17 00:00:00 2001 From: Ben Gourley Date: Fri, 6 Dec 2019 11:39:32 +0000 Subject: [PATCH 5/5] refactor(plugin-network-breadcrumbs): Minimise increase on bundle size --- packages/plugin-network-breadcrumbs/network-breadcrumbs.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/plugin-network-breadcrumbs/network-breadcrumbs.js b/packages/plugin-network-breadcrumbs/network-breadcrumbs.js index 1c8689558e..e20836074a 100644 --- a/packages/plugin-network-breadcrumbs/network-breadcrumbs.js +++ b/packages/plugin-network-breadcrumbs/network-breadcrumbs.js @@ -119,8 +119,9 @@ const monkeyPatchFetch = () => { if (!('fetch' in win) || win.fetch.polyfill) return const oldFetch = win.fetch - win.fetch = function fetch (...args) { - const [urlOrRequest, options] = args + win.fetch = function fetch () { + const urlOrRequest = arguments[0] + const options = arguments[1] let method let url = null @@ -145,7 +146,7 @@ const monkeyPatchFetch = () => { return new Promise((resolve, reject) => { // pass through to native fetch - oldFetch(...args) + oldFetch(...arguments) .then(response => { handleFetchSuccess(response, method, url) resolve(response)