From a096e37b125d8c641fe857a035bbc6ca1e1dca0f Mon Sep 17 00:00:00 2001 From: Ben Wilson Date: Tue, 17 May 2022 16:14:38 +0100 Subject: [PATCH 1/2] add maxEvents error message and update tests --- CHANGELOG.md | 1 + .../plugin-simple-throttle/test/throttle.test.ts | 16 +++++++++++++--- packages/plugin-simple-throttle/throttle.js | 5 ++++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60477e3541..667916d765 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - Small performance improvements to `Bugnag.start` [bugsnag-android#1680](https://github.com/bugsnag/bugsnag-android/pull/1680) - (plugin-react|plugin-vue|plugin-react-navigation|plugin-react-native-navigation) Set `@bugsnag/core` to be an optional peer dependency to avoid unmet peer dependency warnings [#1735](https://github.com/bugsnag/bugsnag-js/pull/1735) +- (plugin-simple-throttle) Warning message added when error handler has exceeded `maxEvents` [#1739](https://github.com/bugsnag/bugsnag-js/pull/1739) ## v7.16.4 (2022-05-03) diff --git a/packages/plugin-simple-throttle/test/throttle.test.ts b/packages/plugin-simple-throttle/test/throttle.test.ts index db1329fc86..4619463f5e 100644 --- a/packages/plugin-simple-throttle/test/throttle.test.ts +++ b/packages/plugin-simple-throttle/test/throttle.test.ts @@ -3,11 +3,21 @@ import plugin from '../' import Client from '@bugsnag/core/client' describe('plugin: throttle', () => { - it('prevents more than maxEvents being sent', () => { + describe('addOnError', () => { const payloads = [] const c = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa' }, undefined, [plugin]) + const mockWarn = jest.fn(message => message) + c._logger.warn = mockWarn c._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload), sendSession: () => {} })) - for (let i = 0; i < 100; i++) c.notify(new Error('This is fail')) - expect(payloads.length).toBe(10) + + it('prevents more than maxEvents being sent', () => { + for (let i = 0; i < 100; i++) c.notify(new Error('This is fail')) + expect(payloads.length).toBe(10) + expect(mockWarn).toHaveBeenCalledTimes(90) + }) + + it('throws an appropriate error message', () => { + expect(mockWarn.mock.results[11].value).toBe('Cancelling event send due to maxEvents per session limit of 10 being reached') + }) }) }) diff --git a/packages/plugin-simple-throttle/throttle.js b/packages/plugin-simple-throttle/throttle.js index d1333b8b28..3de502ec61 100644 --- a/packages/plugin-simple-throttle/throttle.js +++ b/packages/plugin-simple-throttle/throttle.js @@ -12,7 +12,10 @@ module.exports = { // add onError hook client.addOnError((event) => { // have max events been sent already? - if (n >= client._config.maxEvents) return false + if (n >= client._config.maxEvents) { + client._logger.warn(`Cancelling event send due to maxEvents per session limit of ${client._config.maxEvents} being reached`) + return false + } n++ }) From cc27e07d1d2c6d1a9bb40ee3276060cf80d90ae2 Mon Sep 17 00:00:00 2001 From: Ben Wilson Date: Tue, 17 May 2022 17:02:50 +0100 Subject: [PATCH 2/2] clean up throttle test --- .../test/throttle.test.ts | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/plugin-simple-throttle/test/throttle.test.ts b/packages/plugin-simple-throttle/test/throttle.test.ts index 4619463f5e..32589456f4 100644 --- a/packages/plugin-simple-throttle/test/throttle.test.ts +++ b/packages/plugin-simple-throttle/test/throttle.test.ts @@ -3,21 +3,19 @@ import plugin from '../' import Client from '@bugsnag/core/client' describe('plugin: throttle', () => { - describe('addOnError', () => { - const payloads = [] - const c = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa' }, undefined, [plugin]) - const mockWarn = jest.fn(message => message) - c._logger.warn = mockWarn - c._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload), sendSession: () => {} })) + const payloads = [] + const c = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa' }, undefined, [plugin]) + const mockWarn = jest.fn() + c._logger.warn = mockWarn + c._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload), sendSession: () => {} })) - it('prevents more than maxEvents being sent', () => { - for (let i = 0; i < 100; i++) c.notify(new Error('This is fail')) - expect(payloads.length).toBe(10) - expect(mockWarn).toHaveBeenCalledTimes(90) - }) + it('prevents more than maxEvents being sent', () => { + for (let i = 0; i < 100; i++) c.notify(new Error('This is fail')) + expect(payloads.length).toBe(10) + expect(mockWarn).toHaveBeenCalledTimes(90) + }) - it('throws an appropriate error message', () => { - expect(mockWarn.mock.results[11].value).toBe('Cancelling event send due to maxEvents per session limit of 10 being reached') - }) + it('logs an appropriate error message', () => { + expect(mockWarn).toHaveBeenCalledWith('Cancelling event send due to maxEvents per session limit of 10 being reached') }) })