From 10f0707bb8fae11ec5a1620fab8ce9529143128d Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Thu, 29 Sep 2022 14:37:17 +0100 Subject: [PATCH 01/41] remove js payload limit --- CHANGELOG.md | 6 ++++++ packages/core/lib/json-payload.js | 6 ++---- packages/delivery-electron/delivery.js | 5 +++++ packages/delivery-node/delivery.js | 7 ++++++- packages/delivery-x-domain-request/delivery.js | 10 +++++++++- packages/delivery-xml-http-request/delivery.js | 15 +++++++++++++-- 6 files changed, 41 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26b7ce409b..6106f6a4ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## TBD + +### Changed + +- Attempt to send all event and session payloads, regardless of size + ## v7.18.0 (2022-09-22) ### Changed diff --git a/packages/core/lib/json-payload.js b/packages/core/lib/json-payload.js index 63a6e30962..d543aed646 100644 --- a/packages/core/lib/json-payload.js +++ b/packages/core/lib/json-payload.js @@ -15,13 +15,11 @@ Serialized payload was ${payload.length / 10e5}MB (limit = 1MB) metadata was removed` } payload = jsonStringify(event, null, null, { redactedPaths: EVENT_REDACTION_PATHS, redactedKeys }) - if (payload.length > 10e5) throw new Error('payload exceeded 1MB limit') } return payload } -module.exports.session = (event, redactedKeys) => { - const payload = jsonStringify(event, null, null) - if (payload.length > 10e5) throw new Error('payload exceeded 1MB limit') +module.exports.session = (session, redactedKeys) => { + const payload = jsonStringify(session, null, null) return payload } diff --git a/packages/delivery-electron/delivery.js b/packages/delivery-electron/delivery.js index 8c77d4040f..a9b73e76d8 100644 --- a/packages/delivery-electron/delivery.js +++ b/packages/delivery-electron/delivery.js @@ -18,6 +18,11 @@ const delivery = (client, filestore, net, app) => { } else { const err = new Error(`Bad status code from API: ${response.statusCode}`) err.isRetryable = isRetryable(response.statusCode) + // do not retry oversized payloads regardless of status code + if (body.length > 10e5) { + client._logger.warn(`Discarding over-sized event (${body.length / 10e5}) after failed delivery`) + err.isRetryable = false + } cb(err) } }) diff --git a/packages/delivery-node/delivery.js b/packages/delivery-node/delivery.js index 79d4faff8f..6939a73328 100644 --- a/packages/delivery-node/delivery.js +++ b/packages/delivery-node/delivery.js @@ -3,8 +3,13 @@ const request = require('./request') module.exports = (client) => ({ sendEvent: (event, cb = () => {}) => { + const body = payload.event(event, client._config.redactedKeys) + const _cb = err => { if (err) client._logger.error(`Event failed to send…\n${(err && err.stack) ? err.stack : err}`, err) + if (body.length > 10e5) { + client._logger.warn(`Discarding over-sized event (${body.length / 10e5}) after failed delivery`) + } cb(err) } @@ -17,7 +22,7 @@ module.exports = (client) => ({ 'Bugsnag-Payload-Version': '4', 'Bugsnag-Sent-At': (new Date()).toISOString() }, - body: payload.event(event, client._config.redactedKeys), + body, agent: client._config.agent }, (err, body) => _cb(err)) } catch (e) { diff --git a/packages/delivery-x-domain-request/delivery.js b/packages/delivery-x-domain-request/delivery.js index 2e097f77c0..db16147f80 100644 --- a/packages/delivery-x-domain-request/delivery.js +++ b/packages/delivery-x-domain-request/delivery.js @@ -3,14 +3,22 @@ const payload = require('@bugsnag/core/lib/json-payload') module.exports = (client, win = window) => ({ sendEvent: (event, cb = () => {}) => { const url = getApiUrl(client._config, 'notify', '4', win) + const body = payload.event(event, client._config.redactedKeys) + const req = new win.XDomainRequest() req.onload = function () { cb(null) } + req.onerror = function () { + client._logger.error('Event failed to send…\n') + if (body.length > 10e5) { + client._logger.warning(`Discarding over-sized event (${body.length / 10e5}MB) after failed delivery`) + } + } req.open('POST', url) setTimeout(() => { try { - req.send(payload.event(event, client._config.redactedKeys)) + req.send(body) } catch (e) { client._logger.error(e) cb(e) diff --git a/packages/delivery-xml-http-request/delivery.js b/packages/delivery-xml-http-request/delivery.js index 2c74e61549..dd6f99e14a 100644 --- a/packages/delivery-xml-http-request/delivery.js +++ b/packages/delivery-xml-http-request/delivery.js @@ -5,15 +5,26 @@ module.exports = (client, win = window) => ({ try { const url = client._config.endpoints.notify const req = new win.XMLHttpRequest() + const body = payload.event(event, client._config.redactedKeys) + req.onreadystatechange = function () { - if (req.readyState === win.XMLHttpRequest.DONE) cb(null) + if (req.readyState === win.XMLHttpRequest.DONE) { + const status = req.status + if (status === 0 || status >= 400) { + client._logger.error('Event failed to send…\n') + if (body.length > 10e5) { + client._logger.warning(`Discarding over-sized event (${body.length / 10e5}MB) after failed delivery`) + } + } + cb(null) + } } req.open('POST', url) req.setRequestHeader('Content-Type', 'application/json') req.setRequestHeader('Bugsnag-Api-Key', event.apiKey || client._config.apiKey) req.setRequestHeader('Bugsnag-Payload-Version', '4') req.setRequestHeader('Bugsnag-Sent-At', (new Date()).toISOString()) - req.send(payload.event(event, client._config.redactedKeys)) + req.send(body) } catch (e) { client._logger.error(e) } From e57249c37a372b385807fa8cb3d9f371175f68ae Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Thu, 29 Sep 2022 17:21:26 +0100 Subject: [PATCH 02/41] remove js payload limit --- packages/delivery-electron/delivery.js | 2 +- packages/delivery-node/delivery.js | 2 +- packages/delivery-x-domain-request/delivery.js | 2 +- packages/delivery-xml-http-request/delivery.js | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/delivery-electron/delivery.js b/packages/delivery-electron/delivery.js index a9b73e76d8..a0841d45f6 100644 --- a/packages/delivery-electron/delivery.js +++ b/packages/delivery-electron/delivery.js @@ -20,7 +20,7 @@ const delivery = (client, filestore, net, app) => { err.isRetryable = isRetryable(response.statusCode) // do not retry oversized payloads regardless of status code if (body.length > 10e5) { - client._logger.warn(`Discarding over-sized event (${body.length / 10e5}) after failed delivery`) + client._logger.warn(`Discarding over-sized event (${body.length / 10e5} MB) after failed delivery`) err.isRetryable = false } cb(err) diff --git a/packages/delivery-node/delivery.js b/packages/delivery-node/delivery.js index 6939a73328..561c1b6f55 100644 --- a/packages/delivery-node/delivery.js +++ b/packages/delivery-node/delivery.js @@ -8,7 +8,7 @@ module.exports = (client) => ({ const _cb = err => { if (err) client._logger.error(`Event failed to send…\n${(err && err.stack) ? err.stack : err}`, err) if (body.length > 10e5) { - client._logger.warn(`Discarding over-sized event (${body.length / 10e5}) after failed delivery`) + client._logger.warn(`Event oversized (${body.length / 10e5} MB)`) } cb(err) } diff --git a/packages/delivery-x-domain-request/delivery.js b/packages/delivery-x-domain-request/delivery.js index db16147f80..d40ea29426 100644 --- a/packages/delivery-x-domain-request/delivery.js +++ b/packages/delivery-x-domain-request/delivery.js @@ -12,7 +12,7 @@ module.exports = (client, win = window) => ({ req.onerror = function () { client._logger.error('Event failed to send…\n') if (body.length > 10e5) { - client._logger.warning(`Discarding over-sized event (${body.length / 10e5}MB) after failed delivery`) + client._logger.warning(`Event oversized (${body.length / 10e5} MB)`) } } req.open('POST', url) diff --git a/packages/delivery-xml-http-request/delivery.js b/packages/delivery-xml-http-request/delivery.js index dd6f99e14a..780f46ea01 100644 --- a/packages/delivery-xml-http-request/delivery.js +++ b/packages/delivery-xml-http-request/delivery.js @@ -13,7 +13,7 @@ module.exports = (client, win = window) => ({ if (status === 0 || status >= 400) { client._logger.error('Event failed to send…\n') if (body.length > 10e5) { - client._logger.warning(`Discarding over-sized event (${body.length / 10e5}MB) after failed delivery`) + client._logger.warning(`Event oversized (${body.length / 10e5} MB)`) } } cb(null) From fa453ab2068a2a1c3b34c2fb3a7b78239973055e Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Fri, 30 Sep 2022 10:17:09 +0100 Subject: [PATCH 03/41] remove js payload limit --- .../test/delivery.test-main.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/delivery-electron/test/delivery.test-main.ts b/packages/delivery-electron/test/delivery.test-main.ts index 2a8e4d575f..e7921e3ef5 100644 --- a/packages/delivery-electron/test/delivery.test-main.ts +++ b/packages/delivery-electron/test/delivery.test-main.ts @@ -207,6 +207,39 @@ describe('delivery: electron', () => { }) }) + it('does not attempt to re-send oversized payloads', done => { + // A 401 is considered retryable but this will be override by the payload size check + const { requests, server } = mockServer(401) + server.listen(err => { + expect(err).toBeUndefined() + + const lotsOfEvents: any[] = [] + while (JSON.stringify(lotsOfEvents).length < 10e5) { + lotsOfEvents.push({ errors: [{ errorClass: 'Error', errorMessage: 'long repetitive string'.repeat(1000) }] }) + } + const payload = { + events: lotsOfEvents + } as unknown as EventDeliveryPayload + + const config = { + apiKey: 'aaaaaaaa', + endpoints: { notify: `http://localhost:${(server.address() as AddressInfo).port}/notify/` }, + redactedKeys: [] + } + + let didLog = false + const logger = { error: () => { didLog = true }, info: () => {} } + delivery(filestore, net, app)(makeClient(config, logger)).sendEvent(payload, (err: any) => { + expect(didLog).toBe(true) + expect(enqueueSpy).not.toHaveBeenCalled() + expect(err).toBeTruthy() + expect(requests.length).toBe(999) + server.close() + done() + }) + }) + }) + it('handles errors gracefully for sessions (ECONNREFUSED)', done => { const payload = { events: [{ errors: [{ errorClass: 'Error', errorMessage: 'foo is not a function' }] }] From df3f6b5a457deb14e8ed91e2d9e2934ddcc587b4 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Fri, 30 Sep 2022 13:55:37 +0100 Subject: [PATCH 04/41] add json-payload unit tests --- packages/core/lib/test/json-payload.test.ts | 103 ++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 packages/core/lib/test/json-payload.test.ts diff --git a/packages/core/lib/test/json-payload.test.ts b/packages/core/lib/test/json-payload.test.ts new file mode 100644 index 0000000000..89cc6c5777 --- /dev/null +++ b/packages/core/lib/test/json-payload.test.ts @@ -0,0 +1,103 @@ +import jsonPayload from '../json-payload' + +describe('jsonPayload.event', () => { + it('safe stringifies the payload and redacts values from certain paths of the supplied keys', () => { + expect(jsonPayload.event({ + api_key: 'd145b8e5afb56516423bc4d605e45442', + events: [ + { + errorMessage: 'Failed load tickets', + errorClass: 'CheckoutError', + user: { + name: 'Jim Bug', + email: 'jim@bugsnag.com' + }, + request: { + api_key: '245b39ebd3cd3992e85bffc81c045924' + } + } + ] + }, ['api_key'])).toBe('{"api_key":"d145b8e5afb56516423bc4d605e45442","events":[{"errorMessage":"Failed load tickets","errorClass":"CheckoutError","user":{"name":"Jim Bug","email":"jim@bugsnag.com"},"request":{"api_key":"[REDACTED]"}}]}') + }) + + it('strips the metaData of the first event if the payload is too large', () => { + const payload = { + api_key: 'd145b8e5afb56516423bc4d605e45442', + events: [ + { + errorMessage: 'Failed load tickets', + errorClass: 'CheckoutError', + user: { + name: 'Jim Bug', + email: 'jim@bugsnag.com' + }, + request: { + api_key: '245b39ebd3cd3992e85bffc81c045924' + }, + _metadata: {} + } + ] + } + var big: Record = {} + var i = 0 + while (JSON.stringify(big).length < 5 * 10e5) { + big['entry' + i] = 'long repetitive string'.repeat(1000) + i++ + } + payload.events[0]._metadata = { 'big thing': big } + + expect(jsonPayload.event(payload)).toBe('{"api_key":"d145b8e5afb56516423bc4d605e45442","events":[{"errorMessage":"Failed load tickets","errorClass":"CheckoutError","user":{"name":"Jim Bug","email":"jim@bugsnag.com"},"request":{"api_key":"245b39ebd3cd3992e85bffc81c045924"},"_metadata":{"notifier":"WARNING!\\nSerialized payload was 5.019344MB (limit = 1MB)\\nmetadata was removed"}}]}') + }) + + it('does not attempt to strip any other data paths from the payload to reduce the size', () => { + const payload = { + api_key: 'd145b8e5afb56516423bc4d605e45442', + events: [ + { + errorMessage: 'Failed load tickets', + errorClass: 'CheckoutError', + user: { + name: 'Jim Bug', + email: 'jim@bugsnag.com' + }, + _metadata: {} + }, + { + errorMessage: 'Request failed', + errorClass: 'APIError', + _metadata: {} + } + ] + } + var big: Record = {} + var i = 0 + while (JSON.stringify(big).length < 5 * 10e5) { + big['entry' + i] = 'long repetitive string'.repeat(1000) + i++ + } + payload.events[1]._metadata = { 'big thing': big } + + expect(jsonPayload.event(payload).length).toBeGreaterThan(10e5) + }) +}) + +describe('jsonPayload.session', () => { + it('safe stringifies the payload', () => { + expect(jsonPayload.session({ + api_key: 'd145b8e5afb56516423bc4d605e45442', + events: [ + { + errorMessage: 'Failed load tickets', + errorClass: 'CheckoutError', + user: { + name: 'Jim Bug', + email: 'jim@bugsnag.com' + }, + request: { + api_key: '245b39ebd3cd3992e85bffc81c045924' + } + } + ] + }, ['api_key'])).toBe('{"api_key":"d145b8e5afb56516423bc4d605e45442","events":[{"errorMessage":"Failed load tickets","errorClass":"CheckoutError","user":{"name":"Jim Bug","email":"jim@bugsnag.com"},"request":{"api_key":"245b39ebd3cd3992e85bffc81c045924"}}]}') + }) +}) From aa5aa7e7fd04dc16ad677771ee3aa6385b9403ff Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Mon, 3 Oct 2022 13:14:50 +0100 Subject: [PATCH 05/41] dry up test suite --- packages/core/lib/test/json-payload.test.ts | 27 ++++++++++----------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/core/lib/test/json-payload.test.ts b/packages/core/lib/test/json-payload.test.ts index 89cc6c5777..d15928f88e 100644 --- a/packages/core/lib/test/json-payload.test.ts +++ b/packages/core/lib/test/json-payload.test.ts @@ -1,5 +1,15 @@ import jsonPayload from '../json-payload' +function makeBigObject () { + var big: Record = {} + var i = 0 + while (JSON.stringify(big).length < 5 * 10e5) { + big['entry' + i] = 'long repetitive string'.repeat(1000) + i++ + } + return big +} + describe('jsonPayload.event', () => { it('safe stringifies the payload and redacts values from certain paths of the supplied keys', () => { expect(jsonPayload.event({ @@ -38,13 +48,8 @@ describe('jsonPayload.event', () => { } ] } - var big: Record = {} - var i = 0 - while (JSON.stringify(big).length < 5 * 10e5) { - big['entry' + i] = 'long repetitive string'.repeat(1000) - i++ - } - payload.events[0]._metadata = { 'big thing': big } + + payload.events[0]._metadata = { 'big thing': makeBigObject() } expect(jsonPayload.event(payload)).toBe('{"api_key":"d145b8e5afb56516423bc4d605e45442","events":[{"errorMessage":"Failed load tickets","errorClass":"CheckoutError","user":{"name":"Jim Bug","email":"jim@bugsnag.com"},"request":{"api_key":"245b39ebd3cd3992e85bffc81c045924"},"_metadata":{"notifier":"WARNING!\\nSerialized payload was 5.019344MB (limit = 1MB)\\nmetadata was removed"}}]}') }) @@ -69,13 +74,7 @@ describe('jsonPayload.event', () => { } ] } - var big: Record = {} - var i = 0 - while (JSON.stringify(big).length < 5 * 10e5) { - big['entry' + i] = 'long repetitive string'.repeat(1000) - i++ - } - payload.events[1]._metadata = { 'big thing': big } + payload.events[1]._metadata = { 'big thing': makeBigObject() } expect(jsonPayload.event(payload).length).toBeGreaterThan(10e5) }) From 805cd2977e32b9e2d10b4c71197e85b850b54f99 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Mon, 3 Oct 2022 13:30:15 +0100 Subject: [PATCH 06/41] remove js payload limit --- packages/delivery-electron/delivery.js | 2 +- packages/delivery-node/delivery.js | 2 +- packages/delivery-node/test/delivery.test.ts | 32 +++++++++++++++++++ .../delivery-x-domain-request/delivery.js | 2 +- .../delivery-xml-http-request/delivery.js | 2 +- 5 files changed, 36 insertions(+), 4 deletions(-) diff --git a/packages/delivery-electron/delivery.js b/packages/delivery-electron/delivery.js index a0841d45f6..7216350692 100644 --- a/packages/delivery-electron/delivery.js +++ b/packages/delivery-electron/delivery.js @@ -20,7 +20,7 @@ const delivery = (client, filestore, net, app) => { err.isRetryable = isRetryable(response.statusCode) // do not retry oversized payloads regardless of status code if (body.length > 10e5) { - client._logger.warn(`Discarding over-sized event (${body.length / 10e5} MB) after failed delivery`) + client._logger.warn(`Discarding over-sized event (${(body.length / 10e5).toFixed(2)} MB) after failed delivery`) err.isRetryable = false } cb(err) diff --git a/packages/delivery-node/delivery.js b/packages/delivery-node/delivery.js index 561c1b6f55..d7fffec79f 100644 --- a/packages/delivery-node/delivery.js +++ b/packages/delivery-node/delivery.js @@ -8,7 +8,7 @@ module.exports = (client) => ({ const _cb = err => { if (err) client._logger.error(`Event failed to send…\n${(err && err.stack) ? err.stack : err}`, err) if (body.length > 10e5) { - client._logger.warn(`Event oversized (${body.length / 10e5} MB)`) + client._logger.warn(`Event oversized (${(body.length / 10e5).toFixed(2)} MB)`) } cb(err) } diff --git a/packages/delivery-node/test/delivery.test.ts b/packages/delivery-node/test/delivery.test.ts index fca1339222..42dc7ff24d 100644 --- a/packages/delivery-node/test/delivery.test.ts +++ b/packages/delivery-node/test/delivery.test.ts @@ -61,6 +61,38 @@ describe('delivery:node', () => { }) }) + it('logs failures and large payloads', done => { + const { requests, server } = mockServer(400) + server.listen((err: Error) => { + expect(err).toBeUndefined() + + const lotsOfEvents: any[] = [] + while (JSON.stringify(lotsOfEvents).length < 10e5) { + lotsOfEvents.push({ errors: [{ errorClass: 'Error', errorMessage: 'long repetitive string'.repeat(1000) }] }) + } + const payload = { + events: lotsOfEvents + } as unknown as EventDeliveryPayload + + const config = { + apiKey: 'aaaaaaaa', + endpoints: { notify: `http://0.0.0.0:${(server.address() as AddressInfo).port}/notify/` }, + redactedKeys: [] + } + + const logger = { error: jest.fn(), warn: jest.fn() } + + delivery({ _logger: logger, _config: config } as unknown as Client).sendEvent(payload, (err) => { + expect(err).toStrictEqual(new Error('Bad statusCode from API: 400\nOK')) + expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('Event failed to send…'), expect.any(Error)) + expect(logger.warn).toHaveBeenCalledWith('Event oversized (1.01 MB)') + + server.close() + done() + }) + }) + }) + it('sends sessions successfully', done => { const { requests, server } = mockServer(202) server.listen((err: Error) => { diff --git a/packages/delivery-x-domain-request/delivery.js b/packages/delivery-x-domain-request/delivery.js index d40ea29426..0ce2854ca8 100644 --- a/packages/delivery-x-domain-request/delivery.js +++ b/packages/delivery-x-domain-request/delivery.js @@ -12,7 +12,7 @@ module.exports = (client, win = window) => ({ req.onerror = function () { client._logger.error('Event failed to send…\n') if (body.length > 10e5) { - client._logger.warning(`Event oversized (${body.length / 10e5} MB)`) + client._logger.warn(`Event oversized (${(body.length / 10e5).toFixed(2)} MB)`) } } req.open('POST', url) diff --git a/packages/delivery-xml-http-request/delivery.js b/packages/delivery-xml-http-request/delivery.js index 780f46ea01..824d159ca9 100644 --- a/packages/delivery-xml-http-request/delivery.js +++ b/packages/delivery-xml-http-request/delivery.js @@ -13,7 +13,7 @@ module.exports = (client, win = window) => ({ if (status === 0 || status >= 400) { client._logger.error('Event failed to send…\n') if (body.length > 10e5) { - client._logger.warning(`Event oversized (${body.length / 10e5} MB)`) + client._logger.warn(`Event oversized (${(body.length / 10e5).toFixed(2)} MB)`) } } cb(null) From 051d36bde125b2b02b0c1cea247e64549872be0e Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Mon, 3 Oct 2022 13:45:08 +0100 Subject: [PATCH 07/41] remove js payload limit --- packages/delivery-electron/test/delivery.test-main.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/delivery-electron/test/delivery.test-main.ts b/packages/delivery-electron/test/delivery.test-main.ts index e7921e3ef5..d7a47916d8 100644 --- a/packages/delivery-electron/test/delivery.test-main.ts +++ b/packages/delivery-electron/test/delivery.test-main.ts @@ -227,13 +227,16 @@ describe('delivery: electron', () => { redactedKeys: [] } - let didLog = false - const logger = { error: () => { didLog = true }, info: () => {} } + const logger = { error: jest.fn(), info: () => {}, warn: jest.fn() } delivery(filestore, net, app)(makeClient(config, logger)).sendEvent(payload, (err: any) => { - expect(didLog).toBe(true) + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining('event failed to send…'), + expect.stringContaining('Bad status code from API: 401') + ) + expect(logger.warn).toHaveBeenCalledWith('Discarding over-sized event (1.01 MB) after failed delivery') expect(enqueueSpy).not.toHaveBeenCalled() expect(err).toBeTruthy() - expect(requests.length).toBe(999) + expect(requests.length).toBe(1) server.close() done() }) From 2e4daaefe372d6676316d28dd9540586798f1742 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Mon, 3 Oct 2022 14:35:32 +0100 Subject: [PATCH 08/41] remove js payload limit --- .../delivery-x-domain-request/delivery.js | 4 +- .../test/delivery.test.ts | 36 ++++++++++++ .../delivery-xml-http-request/delivery.js | 2 +- .../test/delivery.test.ts | 55 ++++++++++++++++++- 4 files changed, 93 insertions(+), 4 deletions(-) diff --git a/packages/delivery-x-domain-request/delivery.js b/packages/delivery-x-domain-request/delivery.js index 0ce2854ca8..3c25a1427f 100644 --- a/packages/delivery-x-domain-request/delivery.js +++ b/packages/delivery-x-domain-request/delivery.js @@ -10,10 +10,12 @@ module.exports = (client, win = window) => ({ cb(null) } req.onerror = function () { - client._logger.error('Event failed to send…\n') + const err = new Error('Event failed to send') + client._logger.error('Event failed to send…', err) if (body.length > 10e5) { client._logger.warn(`Event oversized (${(body.length / 10e5).toFixed(2)} MB)`) } + cb(err) } req.open('POST', url) setTimeout(() => { diff --git a/packages/delivery-x-domain-request/test/delivery.test.ts b/packages/delivery-x-domain-request/test/delivery.test.ts index a5f512aeb0..c23bb8b238 100644 --- a/packages/delivery-x-domain-request/test/delivery.test.ts +++ b/packages/delivery-x-domain-request/test/delivery.test.ts @@ -72,6 +72,42 @@ describe('delivery:XDomainRequest', () => { }) }) + it('logs failures and large payloads', done => { + // mock XDomainRequest class + function XDomainRequest () { + } + XDomainRequest.prototype.open = function (method: string, url: string) { + this.method = method + this.url = url + } + XDomainRequest.prototype.send = function (method: string, url: string) { + this.onerror() + } + const window = { XDomainRequest, location: { protocol: 'https://' } } as unknown as Window + + const lotsOfEvents: any[] = [] + while (JSON.stringify(lotsOfEvents).length < 10e5) { + lotsOfEvents.push({ errors: [{ errorClass: 'Error', errorMessage: 'long repetitive string'.repeat(1000) }] }) + } + const payload = { + events: lotsOfEvents + } as unknown as EventDeliveryPayload + + const config = { + apiKey: 'aaaaaaaa', + endpoints: { notify: '/echo/', sessions: '/sessions/' }, + redactedKeys: [] + } + const logger = { error: jest.fn(), warn: jest.fn() } + delivery({ _logger: logger, _config: config } as unknown as Client, window).sendEvent(payload, (err) => { + const expectedError = new Error('Event failed to send') + expect(err).toStrictEqual(expectedError) + expect(logger.error).toHaveBeenCalledWith('Event failed to send…', expectedError) + expect(logger.warn).toHaveBeenCalledWith('Event oversized (1.01 MB)') + done() + }) + }) + it('sends sessions successfully', done => { const requests: XDomainRequest[] = [] diff --git a/packages/delivery-xml-http-request/delivery.js b/packages/delivery-xml-http-request/delivery.js index 824d159ca9..19480d7d76 100644 --- a/packages/delivery-xml-http-request/delivery.js +++ b/packages/delivery-xml-http-request/delivery.js @@ -11,7 +11,7 @@ module.exports = (client, win = window) => ({ if (req.readyState === win.XMLHttpRequest.DONE) { const status = req.status if (status === 0 || status >= 400) { - client._logger.error('Event failed to send…\n') + client._logger.error('Event failed to send…') if (body.length > 10e5) { client._logger.warn(`Event oversized (${(body.length / 10e5).toFixed(2)} MB)`) } diff --git a/packages/delivery-xml-http-request/test/delivery.test.ts b/packages/delivery-xml-http-request/test/delivery.test.ts index 17d2fafafa..e5300482df 100644 --- a/packages/delivery-xml-http-request/test/delivery.test.ts +++ b/packages/delivery-xml-http-request/test/delivery.test.ts @@ -8,6 +8,7 @@ interface MockXMLHttpRequest { data: string | null headers: { [key: string]: string } readyState: string | null + status: number } describe('delivery:XMLHttpRequest', () => { @@ -43,7 +44,7 @@ describe('delivery:XMLHttpRequest', () => { endpoints: { notify: '/echo/' }, redactedKeys: [] } - delivery({ logger: {}, _config: config } as unknown as Client, { XMLHttpRequest } as unknown as Window).sendEvent(payload, (err: any) => { + delivery({ _logger: {}, _config: config } as unknown as Client, { XMLHttpRequest } as unknown as Window).sendEvent(payload, (err: any) => { expect(err).toBe(null) expect(requests.length).toBe(1) expect(requests[0].method).toBe('POST') @@ -57,6 +58,56 @@ describe('delivery:XMLHttpRequest', () => { }) }) + it('logs failures and large payloads', done => { + const requests: MockXMLHttpRequest[] = [] + + // mock XMLHttpRequest class + function XMLHttpRequest (this: MockXMLHttpRequest) { + this.method = null + this.url = null + this.data = null + this.headers = {} + this.readyState = null + this.status = 0 + requests.push(this) + } + XMLHttpRequest.DONE = 4 + XMLHttpRequest.prototype.open = function (method: string, url: string) { + this.method = method + this.url = url + } + XMLHttpRequest.prototype.setRequestHeader = function (key: string, val: string) { + this.headers[key] = val + } + XMLHttpRequest.prototype.send = function (data: string) { + this.data = data + this.readyState = XMLHttpRequest.DONE + this.status = 400 + this.onreadystatechange() + } + + const lotsOfEvents: any[] = [] + while (JSON.stringify(lotsOfEvents).length < 10e5) { + lotsOfEvents.push({ errors: [{ errorClass: 'Error', errorMessage: 'long repetitive string'.repeat(1000) }] }) + } + const payload = { + events: lotsOfEvents + } as unknown as EventDeliveryPayload + const config = { + apiKey: 'aaaaaaaa', + endpoints: { notify: '/echo/' }, + redactedKeys: [] + } + const logger = { error: jest.fn(), warn: jest.fn() } + + delivery({ _logger: logger, _config: config } as unknown as Client, { XMLHttpRequest } as unknown as Window).sendEvent(payload, (err: any) => { + expect(err).toBe(null) + expect(logger.error).toHaveBeenCalledWith('Event failed to send…') + expect(logger.warn).toHaveBeenCalledWith('Event oversized (1.01 MB)') + done() + }) + }) + it('sends sessions successfully', done => { const requests: MockXMLHttpRequest[] = [] @@ -89,7 +140,7 @@ describe('delivery:XMLHttpRequest', () => { endpoints: { notify: '/', sessions: '/echo/' }, redactedKeys: [] } - delivery({ _config: config, logger: {} } as unknown as Client, { XMLHttpRequest } as unknown as Window).sendSession(payload, (err) => { + delivery({ _config: config, _logger: {} } as unknown as Client, { XMLHttpRequest } as unknown as Window).sendSession(payload, (err) => { expect(err).toBe(null) expect(requests.length).toBe(1) expect(requests[0].method).toBe('POST') From 32481fade6d978df48e6549d607d4510ab6e91cb Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Mon, 3 Oct 2022 15:14:06 +0100 Subject: [PATCH 09/41] remove js payload limit --- packages/delivery-node/test/delivery.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/delivery-node/test/delivery.test.ts b/packages/delivery-node/test/delivery.test.ts index 42dc7ff24d..96fb62a19d 100644 --- a/packages/delivery-node/test/delivery.test.ts +++ b/packages/delivery-node/test/delivery.test.ts @@ -62,7 +62,7 @@ describe('delivery:node', () => { }) it('logs failures and large payloads', done => { - const { requests, server } = mockServer(400) + const { server } = mockServer(400) server.listen((err: Error) => { expect(err).toBeUndefined() From 6d6b459324c7d50a9fa23e20f79d0fc60a7e1dd6 Mon Sep 17 00:00:00 2001 From: djskinner Date: Fri, 7 Oct 2022 03:52:02 +1100 Subject: [PATCH 10/41] Update CHANGELOG.md Co-authored-by: Gareth Thackeray --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6106f6a4ae..85ccf6f93a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Changed -- Attempt to send all event and session payloads, regardless of size +- After trimming, attempt to send all event and session payloads, even if believed oversize ## v7.18.0 (2022-09-22) From 1c8d1350eb2daf0cfc9ea50f45a58b15a5814394 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Thu, 13 Oct 2022 13:31:01 +0100 Subject: [PATCH 11/41] add e2e coverage --- test/browser/features/delivery.feature | 12 +++++++ .../features/fixtures/delivery/script/a.html | 31 +++++++++++++++++++ .../fixtures/delivery/script/package.json | 8 +++++ 3 files changed, 51 insertions(+) create mode 100644 test/browser/features/delivery.feature create mode 100644 test/browser/features/fixtures/delivery/script/a.html create mode 100644 test/browser/features/fixtures/delivery/script/package.json diff --git a/test/browser/features/delivery.feature b/test/browser/features/delivery.feature new file mode 100644 index 0000000000..a0e7383fc1 --- /dev/null +++ b/test/browser/features/delivery.feature @@ -0,0 +1,12 @@ +Feature: Delivery of errors + + Scenario: Delivery is attempted oversized payloads + Given I set the HTTP status code to 400 + When I navigate to the test URL "/delivery/script/a.html" + And I wait to receive an error + And I wait for the fixture to process the response + + # Check that Bugsnag is discarding the event + And I wait to receive a log + And the log payload field "level" equals "warning" + Then the log payload field "message" matches the regex "Discarding over-sized event \(from.*\) after failed delivery" \ No newline at end of file diff --git a/test/browser/features/fixtures/delivery/script/a.html b/test/browser/features/fixtures/delivery/script/a.html new file mode 100644 index 0000000000..e24c018d05 --- /dev/null +++ b/test/browser/features/fixtures/delivery/script/a.html @@ -0,0 +1,31 @@ + + + + + + + + + + + diff --git a/test/browser/features/fixtures/delivery/script/package.json b/test/browser/features/fixtures/delivery/script/package.json new file mode 100644 index 0000000000..dfd1a94a76 --- /dev/null +++ b/test/browser/features/fixtures/delivery/script/package.json @@ -0,0 +1,8 @@ +{ + "name": "bugsnag-js-fixtures-delivery", + "private": true, + "scripts": { + "build": "echo 'Done'" + }, + "dependencies": {} +} From 0240d643963449faaa242ace587fd7de90a4ce68 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Tue, 1 Nov 2022 09:40:54 +0000 Subject: [PATCH 12/41] add e2e coverage --- test/browser/features/delivery.feature | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/browser/features/delivery.feature b/test/browser/features/delivery.feature index d00d43d63b..2942443ddb 100644 --- a/test/browser/features/delivery.feature +++ b/test/browser/features/delivery.feature @@ -1,9 +1,8 @@ Feature: Delivery of errors Scenario: Delivery is attempted oversized payloads - When I navigate to the test URL "/delivery/script/a.html" - And I wait for 20 seconds - And I set the HTTP status code to 500 + When I set the HTTP status code to 400 + And I navigate to the test URL "/delivery/script/a.html" And I wait to receive an error # Check that Bugsnag is discarding the event From 6812fb7119ad7fb487a53202ff443b0074f0757a Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Tue, 1 Nov 2022 10:10:07 +0000 Subject: [PATCH 13/41] add e2e coverage --- test/browser/features/delivery.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/browser/features/delivery.feature b/test/browser/features/delivery.feature index 2942443ddb..b80cd1743e 100644 --- a/test/browser/features/delivery.feature +++ b/test/browser/features/delivery.feature @@ -1,8 +1,8 @@ Feature: Delivery of errors Scenario: Delivery is attempted oversized payloads - When I set the HTTP status code to 400 - And I navigate to the test URL "/delivery/script/a.html" + When I navigate to the test URL "/delivery/script/a.html" + And I set the HTTP status code to 400 And I wait to receive an error # Check that Bugsnag is discarding the event From 8e6b15ca8eb8caca4d52782abeb5352736dea16d Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Thu, 3 Nov 2022 09:31:20 +0000 Subject: [PATCH 14/41] Test if 400 status has any influence --- test/browser/features/delivery.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/browser/features/delivery.feature b/test/browser/features/delivery.feature index b80cd1743e..699ff917d8 100644 --- a/test/browser/features/delivery.feature +++ b/test/browser/features/delivery.feature @@ -2,7 +2,7 @@ Feature: Delivery of errors Scenario: Delivery is attempted oversized payloads When I navigate to the test URL "/delivery/script/a.html" - And I set the HTTP status code to 400 + # And I set the HTTP status code to 400 And I wait to receive an error # Check that Bugsnag is discarding the event From 98e3eb11ffcfbbaa34c817dd0d1ec66c492097b0 Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Thu, 3 Nov 2022 11:35:43 +0000 Subject: [PATCH 15/41] Use options only response code for failing scenario --- test/browser/features/delivery.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/browser/features/delivery.feature b/test/browser/features/delivery.feature index 699ff917d8..87432f81fb 100644 --- a/test/browser/features/delivery.feature +++ b/test/browser/features/delivery.feature @@ -2,7 +2,7 @@ Feature: Delivery of errors Scenario: Delivery is attempted oversized payloads When I navigate to the test URL "/delivery/script/a.html" - # And I set the HTTP status code to 400 + And I set the HTTP status code for the next "OPTIONS" request to 400 And I wait to receive an error # Check that Bugsnag is discarding the event From 08ea26149303ef454cc283a499177b841e3770ac Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Thu, 3 Nov 2022 11:41:34 +0000 Subject: [PATCH 16/41] Use the post response instead of options --- test/browser/features/delivery.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/browser/features/delivery.feature b/test/browser/features/delivery.feature index 87432f81fb..9b6cc48252 100644 --- a/test/browser/features/delivery.feature +++ b/test/browser/features/delivery.feature @@ -2,7 +2,7 @@ Feature: Delivery of errors Scenario: Delivery is attempted oversized payloads When I navigate to the test URL "/delivery/script/a.html" - And I set the HTTP status code for the next "OPTIONS" request to 400 + And I set the HTTP status code for the next "POST" request to 400 And I wait to receive an error # Check that Bugsnag is discarding the event From 01479ea180be9b52fa59edb418c256ed6ebaddef Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Fri, 4 Nov 2022 11:25:50 +0000 Subject: [PATCH 17/41] Log feedback inplementation --- test/browser/features/delivery.feature | 10 +++++----- .../browser/features/fixtures/delivery/script/a.html | 12 +++++++++++- test/browser/features/support/env.rb | 3 ++- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/test/browser/features/delivery.feature b/test/browser/features/delivery.feature index 9b6cc48252..49f687a830 100644 --- a/test/browser/features/delivery.feature +++ b/test/browser/features/delivery.feature @@ -3,9 +3,9 @@ Feature: Delivery of errors Scenario: Delivery is attempted oversized payloads When I navigate to the test URL "/delivery/script/a.html" And I set the HTTP status code for the next "POST" request to 400 - And I wait to receive an error - + And I wait 5 seconds + Then I expect to receive an error + # Check that Bugsnag is discarding the event - And I wait to receive a log - And the log payload field "level" equals "warning" - Then the log payload field "message" matches the regex "Discarding over-sized event \(from.*\) after failed delivery" \ No newline at end of file + And I expect to receive a log + And the log payload field "response" equals 400 diff --git a/test/browser/features/fixtures/delivery/script/a.html b/test/browser/features/fixtures/delivery/script/a.html index 948b3f5274..392077981c 100644 --- a/test/browser/features/fixtures/delivery/script/a.html +++ b/test/browser/features/fixtures/delivery/script/a.html @@ -6,6 +6,7 @@ diff --git a/test/browser/features/support/env.rb b/test/browser/features/support/env.rb index 558ca2afd7..4e32c2e40c 100644 --- a/test/browser/features/support/env.rb +++ b/test/browser/features/support/env.rb @@ -4,7 +4,8 @@ def get_test_url(path) host = ENV['HOST'] notify = "http://#{ENV['API_HOST']}:#{Maze.config.port}/notify" sessions = "http://#{ENV['API_HOST']}:#{Maze.config.port}/sessions" - config_query_string = "NOTIFY=#{notify}&SESSIONS=#{sessions}&API_KEY=#{$api_key}" + logs = "http://#{ENV['API_HOST']}:#{Maze.config.port}/logs" + config_query_string = "NOTIFY=#{notify}&SESSIONS=#{sessions}&API_KEY=#{$api_key}&LOGS=#{logs}" uri = URI("http://#{host}:#{FIXTURES_SERVER_PORT}#{path}") From e1767a7e36e2b22844d97f50a5964354edbdab3d Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Fri, 4 Nov 2022 13:38:50 +0000 Subject: [PATCH 18/41] Use actual steps --- test/browser/features/delivery.feature | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/browser/features/delivery.feature b/test/browser/features/delivery.feature index 49f687a830..6e0146e7ff 100644 --- a/test/browser/features/delivery.feature +++ b/test/browser/features/delivery.feature @@ -3,9 +3,9 @@ Feature: Delivery of errors Scenario: Delivery is attempted oversized payloads When I navigate to the test URL "/delivery/script/a.html" And I set the HTTP status code for the next "POST" request to 400 - And I wait 5 seconds - Then I expect to receive an error + And I wait for 5 seconds + Then I wait to receive an error # Check that Bugsnag is discarding the event - And I expect to receive a log + And I wait to receive a log And the log payload field "response" equals 400 From e1797bb3ca6b456295b22db7f5ea4f60586eb582 Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Fri, 4 Nov 2022 14:41:06 +0000 Subject: [PATCH 19/41] Return the event instead of err --- test/browser/features/fixtures/delivery/script/a.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/browser/features/fixtures/delivery/script/a.html b/test/browser/features/fixtures/delivery/script/a.html index 392077981c..0de0c19b6b 100644 --- a/test/browser/features/fixtures/delivery/script/a.html +++ b/test/browser/features/fixtures/delivery/script/a.html @@ -38,7 +38,7 @@ xhr.setRequestHeader('Content-Type', 'application/json') xhr.send(JSON.stringify({ - "response": err.status + "response": event })) }); From 945a5a61e650aac103784f84ebf2d79f232fd3ce Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Fri, 4 Nov 2022 15:36:54 +0000 Subject: [PATCH 20/41] Return the full err --- test/browser/features/fixtures/delivery/script/a.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/browser/features/fixtures/delivery/script/a.html b/test/browser/features/fixtures/delivery/script/a.html index 0de0c19b6b..0bb9596a36 100644 --- a/test/browser/features/fixtures/delivery/script/a.html +++ b/test/browser/features/fixtures/delivery/script/a.html @@ -38,7 +38,7 @@ xhr.setRequestHeader('Content-Type', 'application/json') xhr.send(JSON.stringify({ - "response": event + "response": err })) }); From c3203fce29913872c8fc2e460cb88953cfa644e2 Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Fri, 4 Nov 2022 16:22:43 +0000 Subject: [PATCH 21/41] Remove feedback from log and rely on signle response only --- test/browser/features/delivery.feature | 2 +- test/browser/features/fixtures/delivery/script/a.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/browser/features/delivery.feature b/test/browser/features/delivery.feature index 6e0146e7ff..2edc2258a8 100644 --- a/test/browser/features/delivery.feature +++ b/test/browser/features/delivery.feature @@ -8,4 +8,4 @@ Feature: Delivery of errors # Check that Bugsnag is discarding the event And I wait to receive a log - And the log payload field "response" equals 400 + And the log payload field "response" equals "Notify complete" diff --git a/test/browser/features/fixtures/delivery/script/a.html b/test/browser/features/fixtures/delivery/script/a.html index 0bb9596a36..b5621087d2 100644 --- a/test/browser/features/fixtures/delivery/script/a.html +++ b/test/browser/features/fixtures/delivery/script/a.html @@ -38,7 +38,7 @@ xhr.setRequestHeader('Content-Type', 'application/json') xhr.send(JSON.stringify({ - "response": err + "response": "Notify complete" })) }); From fae0eda2fbfc5704c2325017f794e57ec620faef Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Tue, 8 Nov 2022 16:07:42 +0000 Subject: [PATCH 22/41] Attempt cross-platform xhr response --- test/browser/features/fixtures/delivery/script/a.html | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/browser/features/fixtures/delivery/script/a.html b/test/browser/features/fixtures/delivery/script/a.html index b5621087d2..5cca59062b 100644 --- a/test/browser/features/fixtures/delivery/script/a.html +++ b/test/browser/features/fixtures/delivery/script/a.html @@ -33,7 +33,13 @@ Bugsnag.leaveBreadcrumb('big thing', big); Bugsnag.notify(new Error('big things'), null, function (err, event) { var LOGS = decodeURIComponent(window.location.search.match(/LOGS=([^&]+)/)[1]) - const xhr = new XMLHttpRequest(); + var xhr; + if (typeof window.XMLHttpRequest === 'function') { + xhr = new XMLHttpRequest(); + } + else { + xhr = new ActiveXObject("Microsoft.XMLHTTP"); + } xhr.open("POST", LOGS); xhr.setRequestHeader('Content-Type', 'application/json') From a694dc667eeeaa8c863c1037636d11cb560b3d26 Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Tue, 8 Nov 2022 17:09:01 +0000 Subject: [PATCH 23/41] Add basic node log implementation and extra logs to ie10 --- .buildkite/browser-pipeline.yml | 1 + test/node/features/delivery.feature | 19 +++++++++++ .../fixtures/express/scenarios/app.js | 33 +++++++++++++++++++ 3 files changed, 53 insertions(+) create mode 100644 test/node/features/delivery.feature diff --git a/.buildkite/browser-pipeline.yml b/.buildkite/browser-pipeline.yml index 18f72c50ee..4cc79f54e9 100644 --- a/.buildkite/browser-pipeline.yml +++ b/.buildkite/browser-pipeline.yml @@ -193,6 +193,7 @@ steps: command: - --farm=bs - --browser=ie_10 + - --capabilities={"browserstack.consoleLogs":"error"} concurrency: 2 concurrency_group: "browserstack" diff --git a/test/node/features/delivery.feature b/test/node/features/delivery.feature new file mode 100644 index 0000000000..f0dd8df1b6 --- /dev/null +++ b/test/node/features/delivery.feature @@ -0,0 +1,19 @@ +Feature: Delivery of errors + +Background: + Given I store the api key in the environment variable "BUGSNAG_API_KEY" + And I store the notify endpoint in the environment variable "BUGSNAG_NOTIFY_ENDPOINT" + And I store the sessions endpoint in the environment variable "BUGSNAG_SESSIONS_ENDPOINT" + And I store the logs endpoint in the environment variable "BUGSNAG_LOG_ENDPOINT" + +Scenario: adding feature flags for an unhandled error + Given I start the service "express" + And I wait for the host "express" to open port "80" + And I set the HTTP status code for the next "POST" request to 400 + When I open the URL "http://express/features/oversized" + And I wait for 5 seconds + Then I wait to receive an error + + # Check that Bugsnag is discarding the event + And I wait to receive a log + And the log payload field "response" equals "Notify complete" \ No newline at end of file diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index 00e139864c..4bb57cc188 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -2,6 +2,7 @@ var Bugsnag = require('@bugsnag/node') var bugsnagExpress = require('@bugsnag/plugin-express') var express = require('express') var bodyParser = require('body-parser') +var http = require('node:http') Bugsnag.start({ apiKey: process.env.BUGSNAG_API_KEY, @@ -22,6 +23,29 @@ var middleware = Bugsnag.getPlugin('express') var app = express() +function sendLog(body) { + const postData = JSON.stringify(body) + const logUrl = new URL(process.env.BUGSNAG_LOG_ENDPOINT) + const options = { + hostname: logUrl.hostname, + path: logUrl.pathname, + port: logUrl.port, + method: 'POST', + headers: { + 'Content-Type': 'application/json' + } + } + + const req = http.request(options, (res) => { + res.on('end', () => { + console.log('Send complete') + }) + + req.write(postData) + req.end() + }) +} + app.use(middleware.requestHandler) // If the server hasn't started sending something within 2 seconds @@ -70,6 +94,15 @@ app.get('/throw-non-error', function (req, res, next) { throw 1 // eslint-disable-line }) +app.get('/oversized', function (req, res, next) { + req.bugsnag.notify(new Error('handled', null, function (err, event) { + sendLog({ + "response": "Notify complete" + }) + })); + res.end('OK') +}) + app.get('/handled', function (req, res, next) { req.bugsnag.notify(new Error('handled')) res.end('OK') From e8d94f36c698221e7d8d463b4eddf97935be67aa Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Tue, 8 Nov 2022 17:23:53 +0000 Subject: [PATCH 24/41] Update node to maze-runner v7 --- dockerfiles/Dockerfile.node | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dockerfiles/Dockerfile.node b/dockerfiles/Dockerfile.node index ce48bc8315..b29044d015 100644 --- a/dockerfiles/Dockerfile.node +++ b/dockerfiles/Dockerfile.node @@ -21,7 +21,7 @@ RUN npm pack --verbose packages/plugin-koa/ RUN npm pack --verbose packages/plugin-restify/ # The maze-runner node tests -FROM 855461928731.dkr.ecr.us-west-1.amazonaws.com/maze-runner-releases:latest-v6-cli as node-maze-runner +FROM 855461928731.dkr.ecr.us-west-1.amazonaws.com/maze-runner-releases:latest-v7-cli as node-maze-runner WORKDIR /app/ COPY packages/node/ . COPY test/node/features test/node/features From cd749709b22f5db6c39e9ca125659ffd02586d61 Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Tue, 8 Nov 2022 17:44:51 +0000 Subject: [PATCH 25/41] Update to ensure errors are logged correctly --- .buildkite/browser-pipeline.yml | 2 +- test/node/features/delivery.feature | 4 ++-- test/node/features/fixtures/docker-compose.yml | 1 + test/node/features/fixtures/express/scenarios/app.js | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.buildkite/browser-pipeline.yml b/.buildkite/browser-pipeline.yml index 4cc79f54e9..edfbb05c61 100644 --- a/.buildkite/browser-pipeline.yml +++ b/.buildkite/browser-pipeline.yml @@ -193,7 +193,7 @@ steps: command: - --farm=bs - --browser=ie_10 - - --capabilities={"browserstack.consoleLogs":"error"} + - --capabilities={"browserstack.consoleLogs":"errors"} concurrency: 2 concurrency_group: "browserstack" diff --git a/test/node/features/delivery.feature b/test/node/features/delivery.feature index f0dd8df1b6..d5e5df4533 100644 --- a/test/node/features/delivery.feature +++ b/test/node/features/delivery.feature @@ -4,9 +4,9 @@ Background: Given I store the api key in the environment variable "BUGSNAG_API_KEY" And I store the notify endpoint in the environment variable "BUGSNAG_NOTIFY_ENDPOINT" And I store the sessions endpoint in the environment variable "BUGSNAG_SESSIONS_ENDPOINT" - And I store the logs endpoint in the environment variable "BUGSNAG_LOG_ENDPOINT" + And I store the logs endpoint in the environment variable "BUGSNAG_LOGS_ENDPOINT" -Scenario: adding feature flags for an unhandled error +Scenario: Delivery for an oversized error is not retried Given I start the service "express" And I wait for the host "express" to open port "80" And I set the HTTP status code for the next "POST" request to 400 diff --git a/test/node/features/fixtures/docker-compose.yml b/test/node/features/fixtures/docker-compose.yml index c20b3be735..7cf0ceb10e 100644 --- a/test/node/features/fixtures/docker-compose.yml +++ b/test/node/features/fixtures/docker-compose.yml @@ -99,6 +99,7 @@ services: environment: - BUGSNAG_API_KEY - BUGSNAG_NOTIFY_ENDPOINT + - BUGSNAG_LOGS_ENDPOINT - BUGSNAG_SESSIONS_ENDPOINT networks: default: diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index 4bb57cc188..3ea5f4d4b9 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -25,7 +25,7 @@ var app = express() function sendLog(body) { const postData = JSON.stringify(body) - const logUrl = new URL(process.env.BUGSNAG_LOG_ENDPOINT) + const logUrl = new URL(process.env.BUGSNAG_LOGS_ENDPOINT) const options = { hostname: logUrl.hostname, path: logUrl.pathname, From 7e88ff0a9f8d647d721594af4cd4bdb20b90606b Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Wed, 9 Nov 2022 09:15:57 +0000 Subject: [PATCH 26/41] Temporarily disable delivery test on node and on ie10 --- .buildkite/browser-pipeline.yml | 1 - dockerfiles/Dockerfile.node | 2 +- test/browser/features/delivery.feature | 1 + test/node/features/delivery.feature | 1 + 4 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.buildkite/browser-pipeline.yml b/.buildkite/browser-pipeline.yml index edfbb05c61..18f72c50ee 100644 --- a/.buildkite/browser-pipeline.yml +++ b/.buildkite/browser-pipeline.yml @@ -193,7 +193,6 @@ steps: command: - --farm=bs - --browser=ie_10 - - --capabilities={"browserstack.consoleLogs":"errors"} concurrency: 2 concurrency_group: "browserstack" diff --git a/dockerfiles/Dockerfile.node b/dockerfiles/Dockerfile.node index b29044d015..ce48bc8315 100644 --- a/dockerfiles/Dockerfile.node +++ b/dockerfiles/Dockerfile.node @@ -21,7 +21,7 @@ RUN npm pack --verbose packages/plugin-koa/ RUN npm pack --verbose packages/plugin-restify/ # The maze-runner node tests -FROM 855461928731.dkr.ecr.us-west-1.amazonaws.com/maze-runner-releases:latest-v7-cli as node-maze-runner +FROM 855461928731.dkr.ecr.us-west-1.amazonaws.com/maze-runner-releases:latest-v6-cli as node-maze-runner WORKDIR /app/ COPY packages/node/ . COPY test/node/features test/node/features diff --git a/test/browser/features/delivery.feature b/test/browser/features/delivery.feature index 2edc2258a8..305ea41557 100644 --- a/test/browser/features/delivery.feature +++ b/test/browser/features/delivery.feature @@ -1,5 +1,6 @@ Feature: Delivery of errors + @skip_ie_10 Scenario: Delivery is attempted oversized payloads When I navigate to the test URL "/delivery/script/a.html" And I set the HTTP status code for the next "POST" request to 400 diff --git a/test/node/features/delivery.feature b/test/node/features/delivery.feature index d5e5df4533..540f4b3b10 100644 --- a/test/node/features/delivery.feature +++ b/test/node/features/delivery.feature @@ -6,6 +6,7 @@ Background: And I store the sessions endpoint in the environment variable "BUGSNAG_SESSIONS_ENDPOINT" And I store the logs endpoint in the environment variable "BUGSNAG_LOGS_ENDPOINT" +@skip Scenario: Delivery for an oversized error is not retried Given I start the service "express" And I wait for the host "express" to open port "80" From 37d53ef0dba51a243f083a0c29f8f74e2f41bbe1 Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Thu, 17 Nov 2022 11:24:46 +0000 Subject: [PATCH 27/41] Update to Node V7 and add express oversized scenario --- docker-compose.yml | 2 +- dockerfiles/Dockerfile.node | 2 +- .../fixtures/express/scenarios/app.js | 32 ++++++++++++++----- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index eaac4e50eb..29a81c8ae8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -104,7 +104,7 @@ services: context: . dockerfile: dockerfiles/Dockerfile.node target: node-maze-runner - command: --fail-fast --retry 2 + command: --fail-fast --retry 2 --verbose environment: BUILDKITE: BUILDKITE_BRANCH: diff --git a/dockerfiles/Dockerfile.node b/dockerfiles/Dockerfile.node index ce48bc8315..b29044d015 100644 --- a/dockerfiles/Dockerfile.node +++ b/dockerfiles/Dockerfile.node @@ -21,7 +21,7 @@ RUN npm pack --verbose packages/plugin-koa/ RUN npm pack --verbose packages/plugin-restify/ # The maze-runner node tests -FROM 855461928731.dkr.ecr.us-west-1.amazonaws.com/maze-runner-releases:latest-v6-cli as node-maze-runner +FROM 855461928731.dkr.ecr.us-west-1.amazonaws.com/maze-runner-releases:latest-v7-cli as node-maze-runner WORKDIR /app/ COPY packages/node/ . COPY test/node/features test/node/features diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index 3ea5f4d4b9..158e4673a1 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -40,10 +40,9 @@ function sendLog(body) { res.on('end', () => { console.log('Send complete') }) - - req.write(postData) - req.end() }) + req.write(postData) + req.end() } app.use(middleware.requestHandler) @@ -95,11 +94,28 @@ app.get('/throw-non-error', function (req, res, next) { }) app.get('/oversized', function (req, res, next) { - req.bugsnag.notify(new Error('handled', null, function (err, event) { - sendLog({ - "response": "Notify complete" - }) - })); + function repeat(s, n){ + var a = []; + while(a.length < n){ + a.push(s); + } + return a.join(''); + } + + var big = {}; + var i = 0; + while (JSON.stringify(big).length < 5*10e5) { + big['entry'+i] = repeat('long repetitive string', 1000); + i++; + } + req.bugsnag.addMetadata('big data', big) + req.bugsnag.notify(new Error('oversized'), null, function (err, event) { + setTimeout(() => { + sendLog({ + "response": "Notify complete" + }) + }, 1000) + }); res.end('OK') }) From e0a77dd49d7a65c2d7c6532373a350d0601765a3 Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Fri, 18 Nov 2022 16:12:55 +0000 Subject: [PATCH 28/41] Ensure logs uri is passed through to containers and use http lib in older node versions --- test/node/features/delivery.feature | 1 - test/node/features/express.feature | 1 + test/node/features/feature_flags.feature | 1 + test/node/features/fixtures/express/scenarios/app.js | 8 +++++++- 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/test/node/features/delivery.feature b/test/node/features/delivery.feature index 540f4b3b10..d5e5df4533 100644 --- a/test/node/features/delivery.feature +++ b/test/node/features/delivery.feature @@ -6,7 +6,6 @@ Background: And I store the sessions endpoint in the environment variable "BUGSNAG_SESSIONS_ENDPOINT" And I store the logs endpoint in the environment variable "BUGSNAG_LOGS_ENDPOINT" -@skip Scenario: Delivery for an oversized error is not retried Given I start the service "express" And I wait for the host "express" to open port "80" diff --git a/test/node/features/express.feature b/test/node/features/express.feature index 093e7da144..9106753ead 100644 --- a/test/node/features/express.feature +++ b/test/node/features/express.feature @@ -4,6 +4,7 @@ Background: Given I store the api key in the environment variable "BUGSNAG_API_KEY" And I store the notify endpoint in the environment variable "BUGSNAG_NOTIFY_ENDPOINT" And I store the sessions endpoint in the environment variable "BUGSNAG_SESSIONS_ENDPOINT" + And I store the logs endpoint in the environment variable "BUGSNAG_LOGS_ENDPOINT" And I start the service "express" And I wait for the host "express" to open port "80" diff --git a/test/node/features/feature_flags.feature b/test/node/features/feature_flags.feature index 418281ad96..d865ac3199 100644 --- a/test/node/features/feature_flags.feature +++ b/test/node/features/feature_flags.feature @@ -4,6 +4,7 @@ Background: Given I store the api key in the environment variable "BUGSNAG_API_KEY" And I store the notify endpoint in the environment variable "BUGSNAG_NOTIFY_ENDPOINT" And I store the sessions endpoint in the environment variable "BUGSNAG_SESSIONS_ENDPOINT" + And I store the logs endpoint in the environment variable "BUGSNAG_LOGS_ENDPOINT" Scenario: adding feature flags for an unhandled error Given I start the service "express" diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index 158e4673a1..a068879728 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -2,7 +2,13 @@ var Bugsnag = require('@bugsnag/node') var bugsnagExpress = require('@bugsnag/plugin-express') var express = require('express') var bodyParser = require('body-parser') -var http = require('node:http') + +var node_version = process.version.match(/^v(\d+\.\d+)/)[1] +if (parseFloat(node_version) > 12) { + var http = require('node:http') +} else { + var http = require('http') +} Bugsnag.start({ apiKey: process.env.BUGSNAG_API_KEY, From e7c5056f87dfdc09140ca80a4eb104466e7fab43 Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Mon, 21 Nov 2022 09:14:12 +0000 Subject: [PATCH 29/41] Remove problematic HTTP code --- test/node/features/fixtures/express/scenarios/app.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index a068879728..c3271b2d62 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -4,7 +4,7 @@ var express = require('express') var bodyParser = require('body-parser') var node_version = process.version.match(/^v(\d+\.\d+)/)[1] -if (parseFloat(node_version) > 12) { +if (parseFloat(node_version) > 14) { var http = require('node:http') } else { var http = require('http') @@ -42,13 +42,10 @@ function sendLog(body) { } } - const req = http.request(options, (res) => { - res.on('end', () => { - console.log('Send complete') - }) - }) + const req = http.request(options) req.write(postData) req.end() + console.log('Log delivered') } app.use(middleware.requestHandler) From f0556b861aeccc8ff260c801e3290cf3945aa925 Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Mon, 21 Nov 2022 09:29:57 +0000 Subject: [PATCH 30/41] Remove potentially problematic big-data generation --- .../fixtures/express/scenarios/app.js | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index c3271b2d62..a6549d4a3d 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -97,21 +97,21 @@ app.get('/throw-non-error', function (req, res, next) { }) app.get('/oversized', function (req, res, next) { - function repeat(s, n){ - var a = []; - while(a.length < n){ - a.push(s); - } - return a.join(''); - } - - var big = {}; - var i = 0; - while (JSON.stringify(big).length < 5*10e5) { - big['entry'+i] = repeat('long repetitive string', 1000); - i++; - } - req.bugsnag.addMetadata('big data', big) + // function repeat(s, n){ + // var a = []; + // while(a.length < n){ + // a.push(s); + // } + // return a.join(''); + // } + + // var big = {}; + // var i = 0; + // while (JSON.stringify(big).length < 5*10e5) { + // big['entry'+i] = repeat('long repetitive string', 1000); + // i++; + // } + // req.bugsnag.addMetadata('big data', big) req.bugsnag.notify(new Error('oversized'), null, function (err, event) { setTimeout(() => { sendLog({ From 168bd62b80ed42ab0cfdcaff7eddc85165c3eafd Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Mon, 21 Nov 2022 09:41:42 +0000 Subject: [PATCH 31/41] Crop even more out --- .../features/fixtures/express/scenarios/app.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index a6549d4a3d..d8d0fb62fd 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -112,13 +112,14 @@ app.get('/oversized', function (req, res, next) { // i++; // } // req.bugsnag.addMetadata('big data', big) - req.bugsnag.notify(new Error('oversized'), null, function (err, event) { - setTimeout(() => { - sendLog({ - "response": "Notify complete" - }) - }, 1000) - }); + req.bugsnag.notify(new Error('oversized')) + // , null, function (err, event) { + // setTimeout(() => { + // sendLog({ + // "response": "Notify complete" + // }) + // }, 1000) + // }); res.end('OK') }) From 24a0e2642e1ccaf048441d3de0ae52b0f35b217e Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Mon, 21 Nov 2022 10:23:11 +0000 Subject: [PATCH 32/41] Fix incorrect URL --- test/node/features/delivery.feature | 2 +- .../fixtures/express/scenarios/app.js | 45 +++++++++---------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/test/node/features/delivery.feature b/test/node/features/delivery.feature index d5e5df4533..ed15a127e5 100644 --- a/test/node/features/delivery.feature +++ b/test/node/features/delivery.feature @@ -10,7 +10,7 @@ Scenario: Delivery for an oversized error is not retried Given I start the service "express" And I wait for the host "express" to open port "80" And I set the HTTP status code for the next "POST" request to 400 - When I open the URL "http://express/features/oversized" + When I open the URL "http://express/oversized" And I wait for 5 seconds Then I wait to receive an error diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index d8d0fb62fd..c3271b2d62 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -97,29 +97,28 @@ app.get('/throw-non-error', function (req, res, next) { }) app.get('/oversized', function (req, res, next) { - // function repeat(s, n){ - // var a = []; - // while(a.length < n){ - // a.push(s); - // } - // return a.join(''); - // } - - // var big = {}; - // var i = 0; - // while (JSON.stringify(big).length < 5*10e5) { - // big['entry'+i] = repeat('long repetitive string', 1000); - // i++; - // } - // req.bugsnag.addMetadata('big data', big) - req.bugsnag.notify(new Error('oversized')) - // , null, function (err, event) { - // setTimeout(() => { - // sendLog({ - // "response": "Notify complete" - // }) - // }, 1000) - // }); + function repeat(s, n){ + var a = []; + while(a.length < n){ + a.push(s); + } + return a.join(''); + } + + var big = {}; + var i = 0; + while (JSON.stringify(big).length < 5*10e5) { + big['entry'+i] = repeat('long repetitive string', 1000); + i++; + } + req.bugsnag.addMetadata('big data', big) + req.bugsnag.notify(new Error('oversized'), null, function (err, event) { + setTimeout(() => { + sendLog({ + "response": "Notify complete" + }) + }, 1000) + }); res.end('OK') }) From a5aeabd453b1d3ec48059d85f3f8c3a2787270c0 Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Mon, 21 Nov 2022 14:31:42 +0000 Subject: [PATCH 33/41] Ensure the correct version of url is present --- test/node/features/fixtures/express/scenarios/app.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index c3271b2d62..879f4b55fb 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -10,6 +10,12 @@ if (parseFloat(node_version) > 14) { var http = require('http') } +if (parseFloat(node_version) > 12) { + var url = URL +} else { + var url = require('url').url +} + Bugsnag.start({ apiKey: process.env.BUGSNAG_API_KEY, endpoints: { @@ -31,7 +37,7 @@ var app = express() function sendLog(body) { const postData = JSON.stringify(body) - const logUrl = new URL(process.env.BUGSNAG_LOGS_ENDPOINT) + const logUrl = new url(process.env.BUGSNAG_LOGS_ENDPOINT) const options = { hostname: logUrl.hostname, path: logUrl.pathname, From 50caa80415926d0dd17ecedb1320a5c048c94dd1 Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Mon, 21 Nov 2022 15:12:27 +0000 Subject: [PATCH 34/41] Ensure the correct version of url is present --- test/node/features/fixtures/express/scenarios/app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index 879f4b55fb..1f1478de21 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -13,7 +13,7 @@ if (parseFloat(node_version) > 14) { if (parseFloat(node_version) > 12) { var url = URL } else { - var url = require('url').url + var url = require('url').Url } Bugsnag.start({ From 140aef4419184f1bbd26eaf2f1e6e1c1a72a022b Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Wed, 23 Nov 2022 23:22:16 +0000 Subject: [PATCH 35/41] Use correct parse method --- .../node/features/fixtures/express/scenarios/app.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index 1f1478de21..b697a4d707 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -10,12 +10,6 @@ if (parseFloat(node_version) > 14) { var http = require('http') } -if (parseFloat(node_version) > 12) { - var url = URL -} else { - var url = require('url').Url -} - Bugsnag.start({ apiKey: process.env.BUGSNAG_API_KEY, endpoints: { @@ -37,7 +31,12 @@ var app = express() function sendLog(body) { const postData = JSON.stringify(body) - const logUrl = new url(process.env.BUGSNAG_LOGS_ENDPOINT) + if (parseFloat(node_version) > 12) { + var logUrl = new URL(process.env.BUGSNAG_LOGS_ENDPOINT) + } else { + var url = require('url') + var logUrl = url.parse(process.env.BUGSNAG_LOGS_ENDPOINT) + } const options = { hostname: logUrl.hostname, path: logUrl.pathname, From 879587a495b89be76a2f32f2d876a21338e4d189 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Tue, 29 Nov 2022 13:28:37 +0000 Subject: [PATCH 36/41] use remote logging to assert event oversized log message --- test/node/features/delivery.feature | 6 +- .../fixtures/express/scenarios/app.js | 73 +++++++++---------- 2 files changed, 39 insertions(+), 40 deletions(-) diff --git a/test/node/features/delivery.feature b/test/node/features/delivery.feature index ed15a127e5..33880666f9 100644 --- a/test/node/features/delivery.feature +++ b/test/node/features/delivery.feature @@ -15,5 +15,7 @@ Scenario: Delivery for an oversized error is not retried Then I wait to receive an error # Check that Bugsnag is discarding the event - And I wait to receive a log - And the log payload field "response" equals "Notify complete" \ No newline at end of file + And I wait to receive 3 logs + Then I discard the oldest log + Then I discard the oldest log + And the log payload field "message" equals "Event oversized (5.02 MB)" \ No newline at end of file diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index b697a4d707..85980a80b3 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -3,11 +3,37 @@ var bugsnagExpress = require('@bugsnag/plugin-express') var express = require('express') var bodyParser = require('body-parser') -var node_version = process.version.match(/^v(\d+\.\d+)/)[1] -if (parseFloat(node_version) > 14) { - var http = require('node:http') -} else { - var http = require('http') +const nodeVersion = process.version.match(/^v(\d+\.\d+)/)[1] + +const http = parseFloat(nodeVersion) > 14 ? require('node:http') : require('http') + +const logUrl = parseFloat(nodeVersion) > 12 + ? new URL(process.env.BUGSNAG_LOGS_ENDPOINT) + : require('url').parse(process.env.BUGSNAG_LOGS_ENDPOINT) + +function sendLog(level, message) { + const postData = JSON.stringify({ level, message }) + + const options = { + hostname: logUrl.hostname, + path: logUrl.pathname, + port: logUrl.port, + method: 'POST', + headers: { + 'Content-Type': 'application/json' + } + } + + const req = http.request(options) + req.write(postData) + req.end() +} + +const remoteLogger = { + debug: (message) => sendLog('debug', message), + info: (message) => sendLog('info', message), + warn: (message) => sendLog('warn', message), + error: (message) => sendLog('error', message) } Bugsnag.start({ @@ -22,37 +48,14 @@ Bugsnag.start({ { name: 'from config 3', variant: 'SHOULD BE REMOVED' } ], autoTrackSessions: false, - plugins: [bugsnagExpress] + plugins: [bugsnagExpress], + logger: remoteLogger, }) var middleware = Bugsnag.getPlugin('express') var app = express() -function sendLog(body) { - const postData = JSON.stringify(body) - if (parseFloat(node_version) > 12) { - var logUrl = new URL(process.env.BUGSNAG_LOGS_ENDPOINT) - } else { - var url = require('url') - var logUrl = url.parse(process.env.BUGSNAG_LOGS_ENDPOINT) - } - const options = { - hostname: logUrl.hostname, - path: logUrl.pathname, - port: logUrl.port, - method: 'POST', - headers: { - 'Content-Type': 'application/json' - } - } - - const req = http.request(options) - req.write(postData) - req.end() - console.log('Log delivered') -} - app.use(middleware.requestHandler) // If the server hasn't started sending something within 2 seconds @@ -116,14 +119,8 @@ app.get('/oversized', function (req, res, next) { big['entry'+i] = repeat('long repetitive string', 1000); i++; } - req.bugsnag.addMetadata('big data', big) - req.bugsnag.notify(new Error('oversized'), null, function (err, event) { - setTimeout(() => { - sendLog({ - "response": "Notify complete" - }) - }, 1000) - }); + req.bugsnag.leaveBreadcrumb('big thing', big); + req.bugsnag.notify(new Error('oversized')); res.end('OK') }) From b1a23c8d38f6ab1413868e70fef9f715727c9636 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Tue, 29 Nov 2022 13:39:58 +0000 Subject: [PATCH 37/41] use remote logging to assert event oversized log message --- docker-compose.yml | 2 +- packages/core/lib/test/json-payload.test.ts | 4 +- test/browser/features/delivery.feature | 9 +-- .../features/fixtures/delivery/script/a.html | 69 ++++++++++++++----- test/electron/features/delivery.feature | 15 ++++ test/electron/fixtures/app/index.html | 3 +- test/electron/fixtures/app/main.js | 5 +- .../electron/fixtures/app/preloads/default.js | 3 + test/electron/fixtures/app/src/errors.js | 19 +++++ test/node/features/delivery.feature | 2 +- .../fixtures/express/scenarios/app.js | 2 +- 11 files changed, 103 insertions(+), 30 deletions(-) create mode 100644 test/electron/features/delivery.feature diff --git a/docker-compose.yml b/docker-compose.yml index 29a81c8ae8..eaac4e50eb 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -104,7 +104,7 @@ services: context: . dockerfile: dockerfiles/Dockerfile.node target: node-maze-runner - command: --fail-fast --retry 2 --verbose + command: --fail-fast --retry 2 environment: BUILDKITE: BUILDKITE_BRANCH: diff --git a/packages/core/lib/test/json-payload.test.ts b/packages/core/lib/test/json-payload.test.ts index d15928f88e..44cf86c760 100644 --- a/packages/core/lib/test/json-payload.test.ts +++ b/packages/core/lib/test/json-payload.test.ts @@ -3,7 +3,7 @@ import jsonPayload from '../json-payload' function makeBigObject () { var big: Record = {} var i = 0 - while (JSON.stringify(big).length < 5 * 10e5) { + while (JSON.stringify(big).length < 2 * 10e5) { big['entry' + i] = 'long repetitive string'.repeat(1000) i++ } @@ -51,7 +51,7 @@ describe('jsonPayload.event', () => { payload.events[0]._metadata = { 'big thing': makeBigObject() } - expect(jsonPayload.event(payload)).toBe('{"api_key":"d145b8e5afb56516423bc4d605e45442","events":[{"errorMessage":"Failed load tickets","errorClass":"CheckoutError","user":{"name":"Jim Bug","email":"jim@bugsnag.com"},"request":{"api_key":"245b39ebd3cd3992e85bffc81c045924"},"_metadata":{"notifier":"WARNING!\\nSerialized payload was 5.019344MB (limit = 1MB)\\nmetadata was removed"}}]}') + expect(jsonPayload.event(payload)).toBe('{"api_key":"d145b8e5afb56516423bc4d605e45442","events":[{"errorMessage":"Failed load tickets","errorClass":"CheckoutError","user":{"name":"Jim Bug","email":"jim@bugsnag.com"},"request":{"api_key":"245b39ebd3cd3992e85bffc81c045924"},"_metadata":{"notifier":"WARNING!\\nSerialized payload was 2.003435MB (limit = 1MB)\\nmetadata was removed"}}]}') }) it('does not attempt to strip any other data paths from the payload to reduce the size', () => { diff --git a/test/browser/features/delivery.feature b/test/browser/features/delivery.feature index 305ea41557..a452d2fafa 100644 --- a/test/browser/features/delivery.feature +++ b/test/browser/features/delivery.feature @@ -2,11 +2,12 @@ Feature: Delivery of errors @skip_ie_10 Scenario: Delivery is attempted oversized payloads - When I navigate to the test URL "/delivery/script/a.html" - And I set the HTTP status code for the next "POST" request to 400 + When I set the HTTP status code for the next "POST" request to 400 + And I navigate to the test URL "/delivery/script/a.html" And I wait for 5 seconds Then I wait to receive an error # Check that Bugsnag is discarding the event - And I wait to receive a log - And the log payload field "response" equals "Notify complete" + And I wait to receive 2 logs + Then I discard the oldest log + And the log payload field "message" equals "Event oversized (2.00 MB)" diff --git a/test/browser/features/fixtures/delivery/script/a.html b/test/browser/features/fixtures/delivery/script/a.html index 5cca59062b..56437ddf0f 100644 --- a/test/browser/features/fixtures/delivery/script/a.html +++ b/test/browser/features/fixtures/delivery/script/a.html @@ -6,11 +6,57 @@ @@ -26,27 +72,12 @@ var big = {}; var i = 0; - while (JSON.stringify(big).length < 5*10e5) { + while (JSON.stringify(big).length < 2*10e5) { big['entry'+i] = repeat('long repetitive string', 1000); i++; } Bugsnag.leaveBreadcrumb('big thing', big); - Bugsnag.notify(new Error('big things'), null, function (err, event) { - var LOGS = decodeURIComponent(window.location.search.match(/LOGS=([^&]+)/)[1]) - var xhr; - if (typeof window.XMLHttpRequest === 'function') { - xhr = new XMLHttpRequest(); - } - else { - xhr = new ActiveXObject("Microsoft.XMLHTTP"); - } - xhr.open("POST", LOGS); - xhr.setRequestHeader('Content-Type', 'application/json') - - xhr.send(JSON.stringify({ - "response": "Notify complete" - })) - }); + Bugsnag.notify(new Error('big things')); diff --git a/test/electron/features/delivery.feature b/test/electron/features/delivery.feature new file mode 100644 index 0000000000..9281a93c44 --- /dev/null +++ b/test/electron/features/delivery.feature @@ -0,0 +1,15 @@ +Feature: Delivery of errors + + Scenario: Delivery for an oversized error is not retried + Given I launch an app with configuration: + | bugsnag | default | + And I set the HTTP status code for the next "POST" request to 400 + And I click "oversized-notify" + Then I wait to receive an error + + # Check that Bugsnag is discarding the event + And I wait to receive 3 logs + Then I discard the oldest log + Then I discard the oldest log + And the log payload field "message" equals "Event oversized (2.00 MB)" + diff --git a/test/electron/fixtures/app/index.html b/test/electron/fixtures/app/index.html index 4ca6f03f6a..9d35c9d50c 100644 --- a/test/electron/fixtures/app/index.html +++ b/test/electron/fixtures/app/index.html @@ -14,7 +14,8 @@

actions

  • Unhandled promise rejection in main process
  • Handled error in main process
  • Crash in child process
    • - +
  • Oversized event payload in main process
    • +
    • Uncaught exception in renderer
    • Unhandled promise rejection in renderer
      • diff --git a/test/electron/fixtures/app/main.js b/test/electron/fixtures/app/main.js index d3fdaa2ac3..159afba4a7 100644 --- a/test/electron/fixtures/app/main.js +++ b/test/electron/fixtures/app/main.js @@ -3,7 +3,8 @@ const { uncaughtException, unhandledRejection, crash, - notify + notify, + oversizedNotify } = require('./src/errors') const Bugsnag = require('@bugsnag/electron') const configFile = process.env.BUGSNAG_CONFIG || 'default' @@ -93,3 +94,5 @@ ipcMain.on('main-process-clear-feature-flags', () => { ipcMain.on('main-process-clear-feature-flags-now', () => { Bugsnag.clearFeatureFlags() }) + +ipcMain.on('oversized-notify', oversizedNotify) diff --git a/test/electron/fixtures/app/preloads/default.js b/test/electron/fixtures/app/preloads/default.js index 175917c95d..a8255eab9e 100644 --- a/test/electron/fixtures/app/preloads/default.js +++ b/test/electron/fixtures/app/preloads/default.js @@ -39,5 +39,8 @@ contextBridge.exposeInMainWorld('RunnerAPI', { mainProcessClearFeatureFlagsNow: () => { ipcRenderer.send('main-process-clear-feature-flags-now') }, + oversizedNotify: () => { + ipcRenderer.send('oversized-notify') + }, preloadStart: Date.now() }) diff --git a/test/electron/fixtures/app/src/errors.js b/test/electron/fixtures/app/src/errors.js index b51abe75c4..0eb913946b 100644 --- a/test/electron/fixtures/app/src/errors.js +++ b/test/electron/fixtures/app/src/errors.js @@ -16,5 +16,24 @@ module.exports = { notify () { Bugsnag.notify(new ReferenceError('something bad')) + }, + + oversizedNotify () { + function repeat (s, n) { + var a = [] + while (a.length < n) { + a.push(s) + } + return a.join('') + } + + var big = {} + var i = 0 + while (JSON.stringify(big).length < 2 * 10e5) { + big['entry' + i] = repeat('long repetitive string', 1000) + i++ + } + Bugsnag.leaveBreadcrumb('big thing', big) + Bugsnag.notify(new Error('oversized')) } } diff --git a/test/node/features/delivery.feature b/test/node/features/delivery.feature index 33880666f9..ad82f106ef 100644 --- a/test/node/features/delivery.feature +++ b/test/node/features/delivery.feature @@ -18,4 +18,4 @@ Scenario: Delivery for an oversized error is not retried And I wait to receive 3 logs Then I discard the oldest log Then I discard the oldest log - And the log payload field "message" equals "Event oversized (5.02 MB)" \ No newline at end of file + And the log payload field "message" equals "Event oversized (2.01 MB)" \ No newline at end of file diff --git a/test/node/features/fixtures/express/scenarios/app.js b/test/node/features/fixtures/express/scenarios/app.js index 85980a80b3..692096d50a 100644 --- a/test/node/features/fixtures/express/scenarios/app.js +++ b/test/node/features/fixtures/express/scenarios/app.js @@ -115,7 +115,7 @@ app.get('/oversized', function (req, res, next) { var big = {}; var i = 0; - while (JSON.stringify(big).length < 5*10e5) { + while (JSON.stringify(big).length < 2*10e5) { big['entry'+i] = repeat('long repetitive string', 1000); i++; } From af22e4c774cc9d5d59956e6c11955068a011c965 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Thu, 1 Dec 2022 10:19:21 +0000 Subject: [PATCH 38/41] enhance node delivery test for future persistence functionality --- test/node/features/delivery.feature | 11 +++++++++-- .../features/steps/server_fixture_request_steps.rb | 4 ++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/test/node/features/delivery.feature b/test/node/features/delivery.feature index ad82f106ef..7cdbea1f9f 100644 --- a/test/node/features/delivery.feature +++ b/test/node/features/delivery.feature @@ -14,8 +14,15 @@ Scenario: Delivery for an oversized error is not retried And I wait for 5 seconds Then I wait to receive an error - # Check that Bugsnag is discarding the event + # Check that Bugsnag is discarding the event based on the log output And I wait to receive 3 logs Then I discard the oldest log Then I discard the oldest log - And the log payload field "message" equals "Event oversized (2.01 MB)" \ No newline at end of file + And the log payload field "message" equals "Event oversized (2.01 MB)" + + # Check that resend is not attempted next load (e.g. for when persistence/retry is supported in node) + Then I stop all docker services + And I discard the oldest error + And I start the service "express" + And I wait for the host "express" to open port "80" + Then I should receive no errors \ No newline at end of file diff --git a/test/node/features/steps/server_fixture_request_steps.rb b/test/node/features/steps/server_fixture_request_steps.rb index a1da9e1d9e..c0a94df156 100644 --- a/test/node/features/steps/server_fixture_request_steps.rb +++ b/test/node/features/steps/server_fixture_request_steps.rb @@ -33,3 +33,7 @@ TEXT ) end + +When('I stop all docker services') do + Maze::Docker.down_all_services +end \ No newline at end of file From b9ed23a611a01347c1a37c3cb2d28affd25683ed Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Thu, 1 Dec 2022 15:35:45 +0000 Subject: [PATCH 39/41] revert e2e tests for oversized delivery on electron --- test/electron/features/delivery.feature | 15 --------------- test/electron/fixtures/app/index.html | 3 +-- test/electron/fixtures/app/main.js | 5 +---- .../electron/fixtures/app/preloads/default.js | 3 --- test/electron/fixtures/app/src/errors.js | 19 ------------------- 5 files changed, 2 insertions(+), 43 deletions(-) delete mode 100644 test/electron/features/delivery.feature diff --git a/test/electron/features/delivery.feature b/test/electron/features/delivery.feature deleted file mode 100644 index 9281a93c44..0000000000 --- a/test/electron/features/delivery.feature +++ /dev/null @@ -1,15 +0,0 @@ -Feature: Delivery of errors - - Scenario: Delivery for an oversized error is not retried - Given I launch an app with configuration: - | bugsnag | default | - And I set the HTTP status code for the next "POST" request to 400 - And I click "oversized-notify" - Then I wait to receive an error - - # Check that Bugsnag is discarding the event - And I wait to receive 3 logs - Then I discard the oldest log - Then I discard the oldest log - And the log payload field "message" equals "Event oversized (2.00 MB)" - diff --git a/test/electron/fixtures/app/index.html b/test/electron/fixtures/app/index.html index 9d35c9d50c..4ca6f03f6a 100644 --- a/test/electron/fixtures/app/index.html +++ b/test/electron/fixtures/app/index.html @@ -14,8 +14,7 @@

      actions

    • Unhandled promise rejection in main process
    • Handled error in main process
    • Crash in child process
      • -
    • Oversized event payload in main process
      • -
    +
    • Uncaught exception in renderer
    • Unhandled promise rejection in renderer
      • diff --git a/test/electron/fixtures/app/main.js b/test/electron/fixtures/app/main.js index 159afba4a7..d3fdaa2ac3 100644 --- a/test/electron/fixtures/app/main.js +++ b/test/electron/fixtures/app/main.js @@ -3,8 +3,7 @@ const { uncaughtException, unhandledRejection, crash, - notify, - oversizedNotify + notify } = require('./src/errors') const Bugsnag = require('@bugsnag/electron') const configFile = process.env.BUGSNAG_CONFIG || 'default' @@ -94,5 +93,3 @@ ipcMain.on('main-process-clear-feature-flags', () => { ipcMain.on('main-process-clear-feature-flags-now', () => { Bugsnag.clearFeatureFlags() }) - -ipcMain.on('oversized-notify', oversizedNotify) diff --git a/test/electron/fixtures/app/preloads/default.js b/test/electron/fixtures/app/preloads/default.js index a8255eab9e..175917c95d 100644 --- a/test/electron/fixtures/app/preloads/default.js +++ b/test/electron/fixtures/app/preloads/default.js @@ -39,8 +39,5 @@ contextBridge.exposeInMainWorld('RunnerAPI', { mainProcessClearFeatureFlagsNow: () => { ipcRenderer.send('main-process-clear-feature-flags-now') }, - oversizedNotify: () => { - ipcRenderer.send('oversized-notify') - }, preloadStart: Date.now() }) diff --git a/test/electron/fixtures/app/src/errors.js b/test/electron/fixtures/app/src/errors.js index 0eb913946b..b51abe75c4 100644 --- a/test/electron/fixtures/app/src/errors.js +++ b/test/electron/fixtures/app/src/errors.js @@ -16,24 +16,5 @@ module.exports = { notify () { Bugsnag.notify(new ReferenceError('something bad')) - }, - - oversizedNotify () { - function repeat (s, n) { - var a = [] - while (a.length < n) { - a.push(s) - } - return a.join('') - } - - var big = {} - var i = 0 - while (JSON.stringify(big).length < 2 * 10e5) { - big['entry' + i] = repeat('long repetitive string', 1000) - i++ - } - Bugsnag.leaveBreadcrumb('big thing', big) - Bugsnag.notify(new Error('oversized')) } } From 094c21a866a2f2f0a77fd6eec53b3e6ef79a0ab5 Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Thu, 15 Dec 2022 10:31:42 +0000 Subject: [PATCH 40/41] update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab0d4e02ee..f11e8b906e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Changed -- After trimming, attempt to send all event and session payloads, even if believed oversize +- After trimming, attempt to send all event and session payloads, even if believed oversize [#1823](https://github.com/bugsnag/bugsnag-js/pull/1823) - (react-native) Update bugsnag-android from v5.28.1 to [v5.28.3](https://github.com/bugsnag/bugsnag-android/blob/master/CHANGELOG.md#5283-2022-11-16) ### Fixed From c455ae4b6b7afa40bf52d59751faa0558992326c Mon Sep 17 00:00:00 2001 From: Dan Skinner Date: Mon, 9 Jan 2023 13:56:39 +0000 Subject: [PATCH 41/41] revert to running Safari 16 tests on browser stack --- .buildkite/browser-pipeline.yml | 34 ++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/.buildkite/browser-pipeline.yml b/.buildkite/browser-pipeline.yml index 03597b21c5..da1ca846ff 100644 --- a/.buildkite/browser-pipeline.yml +++ b/.buildkite/browser-pipeline.yml @@ -107,23 +107,6 @@ steps: concurrency: 5 concurrency_group: "bitbar-web" - - label: ":safari: 16 Browser tests" - depends_on: "browser-maze-runner" - timeout_in_minutes: 30 - plugins: - docker-compose#v3.9.0: - pull: browser-maze-runner - run: browser-maze-runner - use-aliases: true - command: - - --farm=bb - - --browser=safari_16 - artifacts#v1.5.0: - upload: - - "./test/browser/maze_output/failed/**/*" - concurrency: 5 - concurrency_group: "bitbar-web" - # # BrowserStack tests # @@ -263,6 +246,23 @@ steps: concurrency: 2 concurrency_group: "browserstack" + - label: ":safari: 16 Browser tests" + depends_on: "browser-maze-runner" + timeout_in_minutes: 30 + plugins: + docker-compose#v3.9.0: + pull: browser-maze-runner + run: browser-maze-runner + use-aliases: true + command: + - --farm=bs + - --browser=safari_16 + artifacts#v1.5.0: + upload: + - "./test/browser/maze_output/failed/**/*" + concurrency: 2 + concurrency_group: "browserstack" + - label: ":iphone: iOS 10.3 Browser tests" depends_on: "browser-maze-runner" timeout_in_minutes: 30