From d64ed8764f38d0b31fdc317dd7fb288617611233 Mon Sep 17 00:00:00 2001 From: Yousif Ahmed Date: Mon, 20 Nov 2023 13:49:08 +0000 Subject: [PATCH 1/7] fix(electron): do not sync to the NativeClient if nativeCrashes or autoDetectErrors are disabled --- packages/plugin-electron-app/app.js | 38 ++++++++++---- .../client-state-persistence.js | 6 +++ packages/plugin-electron-device/device.js | 20 ++++--- packages/plugin-electron-session/session.js | 52 ++++++++++--------- 4 files changed, 75 insertions(+), 41 deletions(-) diff --git a/packages/plugin-electron-app/app.js b/packages/plugin-electron-app/app.js index 7a329d9386..d01b68dbf5 100644 --- a/packages/plugin-electron-app/app.js +++ b/packages/plugin-electron-app/app.js @@ -2,27 +2,43 @@ const native = require('bindings')('bugsnag_plugin_electron_app_bindings') const { schema } = require('@bugsnag/core/config') const intRange = require('@bugsnag/core/lib/validators/int-range') +const isNativeClientEnabled = client => client._config.autoDetectErrors && client._config.enabledErrorTypes.nativeCrashes + +const noop = () => {} + const osToAppType = new Map([ ['darwin', 'macOS'], ['linux', 'Linux'], ['win32', 'Windows'] ]) -const createAppUpdater = (client, NativeClient, app) => newProperties => { - Object.assign(app, newProperties) +const createAppUpdater = (client, NativeClient, app) => { + if (!isNativeClientEnabled(client)) { + return newProperties => Object.assign(app, newProperties) + } + + return newProperties => { + Object.assign(app, newProperties) - try { - NativeClient.setApp(app) - } catch (err) { - client._logger.error(err) + try { + NativeClient.setApp(app) + } catch (err) { + client._logger.error(err) + } } } -const createLastRunInfoUpdater = (client, NativeClient) => lastRunInfo => { - try { - NativeClient.setLastRunInfo(JSON.stringify(lastRunInfo)) - } catch (err) { - client._logger.error(err) +const createLastRunInfoUpdater = (client, NativeClient) => { + if (!isNativeClientEnabled(client)) { + return noop + } + + return lastRunInfo => { + try { + NativeClient.setLastRunInfo(JSON.stringify(lastRunInfo)) + } catch (err) { + client._logger.error(err) + } } } diff --git a/packages/plugin-electron-client-state-persistence/client-state-persistence.js b/packages/plugin-electron-client-state-persistence/client-state-persistence.js index 9324171b19..42186b4a59 100644 --- a/packages/plugin-electron-client-state-persistence/client-state-persistence.js +++ b/packages/plugin-electron-client-state-persistence/client-state-persistence.js @@ -1,9 +1,15 @@ const featureFlagDelegate = require('@bugsnag/core/lib/feature-flag-delegate') +const isEnabledFor = client => client._config.autoDetectErrors && client._config.enabledErrorTypes.nativeCrashes + module.exports = { NativeClient: require('bindings')('bugsnag_pecsp_bindings'), plugin: (NativeClient) => ({ load: (client) => { + if (!isEnabledFor(client)) { + return + } + client.addOnBreadcrumb(breadcrumb => { try { NativeClient.leaveBreadcrumb(breadcrumb) diff --git a/packages/plugin-electron-device/device.js b/packages/plugin-electron-device/device.js index 16f73cd5c6..1a55413f5d 100644 --- a/packages/plugin-electron-device/device.js +++ b/packages/plugin-electron-device/device.js @@ -7,13 +7,21 @@ const platformToOs = new Map([ // electron memory APIs are documented as KB but are actually KiB const kibibytesToBytes = kibibytes => kibibytes * 1024 -const createDeviceUpdater = (client, NativeClient, device) => newProperties => { - Object.assign(device, newProperties) +const isNativeClientEnabled = client => client._config.autoDetectErrors && client._config.enabledErrorTypes.nativeCrashes - try { - NativeClient.setDevice(device) - } catch (err) { - client._logger.error(err) +const createDeviceUpdater = (client, NativeClient, device) => { + if (!isNativeClientEnabled(client)) { + return newProperties => Object.assign(device, newProperties) + } + + return newProperties => { + Object.assign(device, newProperties) + + try { + NativeClient.setDevice(device) + } catch (err) { + client._logger.error(err) + } } } diff --git a/packages/plugin-electron-session/session.js b/packages/plugin-electron-session/session.js index a415c247ed..f5440fe251 100644 --- a/packages/plugin-electron-session/session.js +++ b/packages/plugin-electron-session/session.js @@ -3,37 +3,41 @@ const Session = require('@bugsnag/core/session') const SESSION_TIMEOUT_MS = 60 * 1000 +const isNativeClientEnabled = client => client._config.autoDetectErrors && client._config.enabledErrorTypes.nativeCrashes + module.exports = (app, BrowserWindow, NativeClient) => ({ load (client) { - // Ensure native session kept in sync with session changes - const oldTrack = Session.prototype._track - Session.prototype._track = function (...args) { - const result = oldTrack.apply(this, args) - NativeClient.setSession(serializeForNativeEvent(this)) - return result - } - // load the actual session delegate from plugin-browser-session sessionDelegate.load(client) - // Override the delegate to perform electron-specific synchronization - const defaultDelegate = client._sessionDelegate - client._sessionDelegate = { - startSession: (client, session) => { - const result = defaultDelegate.startSession(client, session) - NativeClient.setSession(serializeForNativeEvent(client._session)) - return result - }, - resumeSession: (client) => { - const result = defaultDelegate.resumeSession(client) - NativeClient.setSession(serializeForNativeEvent(client._session)) - return result - }, - pauseSession: (client) => { - const result = defaultDelegate.pauseSession(client) - NativeClient.setSession(serializeForNativeEvent(client._session)) + if (isNativeClientEnabled(client)) { + // Ensure native session kept in sync with session changes + const oldTrack = Session.prototype._track + Session.prototype._track = function (...args) { + const result = oldTrack.apply(this, args) + NativeClient.setSession(serializeForNativeEvent(this)) return result } + + // Override the delegate to perform electron-specific synchronization + const defaultDelegate = client._sessionDelegate + client._sessionDelegate = { + startSession: (client, session) => { + const result = defaultDelegate.startSession(client, session) + NativeClient.setSession(serializeForNativeEvent(client._session)) + return result + }, + resumeSession: (client) => { + const result = defaultDelegate.resumeSession(client) + NativeClient.setSession(serializeForNativeEvent(client._session)) + return result + }, + pauseSession: (client) => { + const result = defaultDelegate.pauseSession(client) + NativeClient.setSession(serializeForNativeEvent(client._session)) + return result + } + } } if (!client._config.autoTrackSessions) { From 2d38fb91b5396e562a6e901a0334776f8f58d816 Mon Sep 17 00:00:00 2001 From: Yousif Ahmed Date: Mon, 20 Nov 2023 13:50:11 +0000 Subject: [PATCH 2/7] (plugin-electron-session) Add unit tests for disabled NativeClient --- .../test/session.test.ts | 117 ++++++++++++++++-- 1 file changed, 104 insertions(+), 13 deletions(-) diff --git a/packages/plugin-electron-session/test/session.test.ts b/packages/plugin-electron-session/test/session.test.ts index 569529e3c2..fa7abb8778 100644 --- a/packages/plugin-electron-session/test/session.test.ts +++ b/packages/plugin-electron-session/test/session.test.ts @@ -1,10 +1,28 @@ import Client, { EventDeliveryPayload } from '@bugsnag/core/client' +import { schema as defaultSchema } from '@bugsnag/core/config' import { SessionPayload } from '@bugsnag/core' import { makeApp, makeBrowserWindow } from '@bugsnag/electron-test-helpers' import plugin from '../' const expectCuid = expect.stringMatching(/^c[a-z0-9]{20,30}$/) +const config = { + apiKey: 'abcabcabcabcabcabcabc1234567890f' +} + +const schema = { + ...defaultSchema, + enabledErrorTypes: { + defaultValue: () => ({ + unhandledExceptions: true, + unhandledRejections: true, + nativeCrashes: true + }), + allowPartialObject: true, + validate: value => true + } +} + describe('plugin: electron sessions', () => { let NativeClient beforeEach(() => { @@ -17,8 +35,8 @@ describe('plugin: electron sessions', () => { const app = makeApp({ BrowserWindow }) const client = new Client( - { apiKey: 'abcabcabcabcabcabcabc1234567890f' }, - undefined, + config, + schema, [plugin(app, BrowserWindow, NativeClient)] ) @@ -67,8 +85,8 @@ describe('plugin: electron sessions', () => { const app = makeApp({ BrowserWindow }) const client = new Client( - { apiKey: 'abcabcabcabcabcabcabc1234567890f' }, - undefined, + config, + schema, [plugin(app, BrowserWindow, NativeClient)] ) @@ -117,8 +135,8 @@ describe('plugin: electron sessions', () => { const app = makeApp({ BrowserWindow }) const client = new Client( - { apiKey: 'abcabcabcabcabcabcabc1234567890f' }, - undefined, + config, + schema, [plugin(app, BrowserWindow, NativeClient)] ) @@ -174,7 +192,7 @@ describe('plugin: electron sessions', () => { const client = new Client( { apiKey: 'abcabcabcabcabcabcabc1234567890f', autoTrackSessions: false }, - undefined, + schema, [plugin(app, BrowserWindow, NativeClient)] ) @@ -204,8 +222,8 @@ describe('plugin: electron sessions', () => { const app = makeApp({ BrowserWindow }) const client = new Client( - { apiKey: 'abcabcabcabcabcabcabc1234567890f' }, - undefined, + config, + schema, [plugin(app, BrowserWindow, NativeClient)] ) @@ -228,8 +246,8 @@ describe('plugin: electron sessions', () => { const app = makeApp({ BrowserWindow }) const client = new Client( - { apiKey: 'abcabcabcabcabcabcabc1234567890f' }, - undefined, + config, + schema, [plugin(app, BrowserWindow, NativeClient)] ) @@ -243,8 +261,8 @@ describe('plugin: electron sessions', () => { const app = makeApp({ BrowserWindow }) const client = new Client( - { apiKey: 'abcabcabcabcabcabcabc1234567890f' }, - undefined, + config, + schema, [plugin(app, BrowserWindow, NativeClient)] ) @@ -259,6 +277,79 @@ describe('plugin: electron sessions', () => { events: { handled: 0, unhandled: 1 } }) }) + + it('does not update the native session when nativeCrashes is disabled', async () => { + jest.useFakeTimers('modern') + const BrowserWindow = makeBrowserWindow() + const app = makeApp({ BrowserWindow }) + + const client = new Client( + // @ts-expect-error enabledErrorTypes.nativeCrashes is not part of the core schema + { ...config, enabledErrorTypes: { nativeCrashes: false } }, + schema, + [plugin(app, BrowserWindow, NativeClient)] + ) + + const window = new BrowserWindow(123, 'hello world') + const payloads: SessionPayload[] = [] + + client._setDelivery(() => ({ + sendEvent (payload: EventDeliveryPayload, cb = () => {}) { + }, + sendSession (payload: SessionPayload, cb = () => {}) { + payloads.push(payload) + cb() + } + })) + + client.startSession() + client.pauseSession() + client.resumeSession() + client.notify(new Error('oh no')) + + app._emit('browser-window-blur', window) + jest.advanceTimersByTime(120_000) + app._emit('browser-window-focus', window) + + expect(payloads).toHaveLength(2) + expect(NativeClient.setSession).not.toHaveBeenCalled() + }) + + it('does not update the native session when autoDetectErrors is disabled', async () => { + jest.useFakeTimers('modern') + const BrowserWindow = makeBrowserWindow() + const app = makeApp({ BrowserWindow }) + + const client = new Client( + { ...config, autoDetectErrors: false }, + schema, + [plugin(app, BrowserWindow, NativeClient)] + ) + + const window = new BrowserWindow(123, 'hello world') + const payloads: SessionPayload[] = [] + + client._setDelivery(() => ({ + sendEvent (payload: EventDeliveryPayload, cb = () => {}) { + }, + sendSession (payload: SessionPayload, cb = () => {}) { + payloads.push(payload) + cb() + } + })) + + client.startSession() + client.pauseSession() + client.resumeSession() + client.notify(new Error('oh no')) + + app._emit('browser-window-blur', window) + jest.advanceTimersByTime(120_000) + app._emit('browser-window-focus', window) + + expect(payloads).toHaveLength(2) + expect(NativeClient.setSession).not.toHaveBeenCalled() + }) }) async function createSession (client: Client): Promise { From dac7b6a656461088822e7a548c259c1b2c23b765 Mon Sep 17 00:00:00 2001 From: Yousif Ahmed Date: Mon, 20 Nov 2023 13:54:08 +0000 Subject: [PATCH 3/7] Update makeClientForPlugin test helper to support an array plugins --- packages/electron-test-helpers/src/client.ts | 6 +++--- .../test/net-breadcrumbs.test-main.ts | 2 +- .../test/power-monitor-breadcrumbs.test.ts | 6 +++--- .../test/preload-error.test-main.ts | 2 +- .../test/renderer-event-data.test.ts | 2 +- .../test/strip-project-root.test.ts | 2 +- .../test/screen-breadcrumbs.test.ts | 12 ++++++------ 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/electron-test-helpers/src/client.ts b/packages/electron-test-helpers/src/client.ts index a645dc0427..9199fb3ee2 100644 --- a/packages/electron-test-helpers/src/client.ts +++ b/packages/electron-test-helpers/src/client.ts @@ -11,8 +11,8 @@ interface ClientTestHelpers { export function makeClientForPlugin ({ config = {}, schema = {}, - plugin = undefined -}: { config?: object, schema?: object, plugin?: Plugin } = {}): ClientTestHelpers { + plugins = [] +}: { config?: object, schema?: object, plugins?: Plugin[] } = {}): ClientTestHelpers { const client = new Client( { apiKey: 'abcabcabcabcabcabcabc1234567890f', @@ -20,7 +20,7 @@ export function makeClientForPlugin ({ ...config }, { ...defaultSchema, ...schema }, - plugin !== undefined ? [plugin] : [] + plugins ) let lastSession: SessionPayload diff --git a/packages/plugin-electron-net-breadcrumbs/test/net-breadcrumbs.test-main.ts b/packages/plugin-electron-net-breadcrumbs/test/net-breadcrumbs.test-main.ts index 06c6a17c99..c84548d1a4 100644 --- a/packages/plugin-electron-net-breadcrumbs/test/net-breadcrumbs.test-main.ts +++ b/packages/plugin-electron-net-breadcrumbs/test/net-breadcrumbs.test-main.ts @@ -283,7 +283,7 @@ describe.skip('plugin: electron net breadcrumbs', () => { }) function makeClient ({ config = {}, schema = {} } = {}) { - return makeClientForPlugin({ config, schema, plugin: plugin(net) }).client + return makeClientForPlugin({ config, schema, plugins: [plugin(net)] }).client } const defaultRequestHandler = (statusCode: number) => (req: IncomingMessage, res: ServerResponse) => { diff --git a/packages/plugin-electron-power-monitor-breadcrumbs/test/power-monitor-breadcrumbs.test.ts b/packages/plugin-electron-power-monitor-breadcrumbs/test/power-monitor-breadcrumbs.test.ts index 81f241ab96..73e7adbbd0 100644 --- a/packages/plugin-electron-power-monitor-breadcrumbs/test/power-monitor-breadcrumbs.test.ts +++ b/packages/plugin-electron-power-monitor-breadcrumbs/test/power-monitor-breadcrumbs.test.ts @@ -16,7 +16,7 @@ describe('plugin: electron power monitor breadcrumbs', () => { it.each(events)('leaves a breadcrumb for the "%s" event', (event, expectedMessage) => { const powerMonitor = makePowerMonitor() - const { client } = makeClientForPlugin({ plugin: plugin(powerMonitor) }) + const { client } = makeClientForPlugin({ plugins: [plugin(powerMonitor)] }) powerMonitor._emit(event) @@ -28,7 +28,7 @@ describe('plugin: electron power monitor breadcrumbs', () => { it.each(events)('honours enabledBreadcrumbTypes for "%s"', (event, expectedMessage) => { const powerMonitor = makePowerMonitor() const { client } = makeClientForPlugin({ - plugin: plugin(powerMonitor), + plugins: [plugin(powerMonitor)], config: { enabledBreadcrumbTypes: [] } }) @@ -40,7 +40,7 @@ describe('plugin: electron power monitor breadcrumbs', () => { it.each(events)('works when enabledBreadcrumbTypes=null for "%s"', (event, expectedMessage) => { const powerMonitor = makePowerMonitor() const { client } = makeClientForPlugin({ - plugin: plugin(powerMonitor), + plugins: [plugin(powerMonitor)], config: { enabledBreadcrumbTypes: null } }) diff --git a/packages/plugin-electron-preload-error/test/preload-error.test-main.ts b/packages/plugin-electron-preload-error/test/preload-error.test-main.ts index dfbca272ea..8942ff55fc 100644 --- a/packages/plugin-electron-preload-error/test/preload-error.test-main.ts +++ b/packages/plugin-electron-preload-error/test/preload-error.test-main.ts @@ -117,7 +117,7 @@ describe('plugin: preload-error', () => { }) function makeClient ({ config = {}, schema = {}, _app = app } = {}) { - const { client } = makeClientForPlugin({ config, schema, plugin: plugin(_app) }) + const { client } = makeClientForPlugin({ config, schema, plugins: [plugin(_app)] }) client._setDelivery(() => ({ sendEvent: jest.fn(), sendSession: () => {} })) return client diff --git a/packages/plugin-electron-renderer-event-data/test/renderer-event-data.test.ts b/packages/plugin-electron-renderer-event-data/test/renderer-event-data.test.ts index 339e3d997f..723a39646c 100644 --- a/packages/plugin-electron-renderer-event-data/test/renderer-event-data.test.ts +++ b/packages/plugin-electron-renderer-event-data/test/renderer-event-data.test.ts @@ -56,5 +56,5 @@ describe('plugin: electron renderer event data', () => { }) }) -const makeClient = payloadInfo => makeClientForPlugin({ plugin: plugin(makeIpcRenderer(payloadInfo)) }) +const makeClient = payloadInfo => makeClientForPlugin({ plugins: [plugin(makeIpcRenderer(payloadInfo))] }) const makeIpcRenderer = payloadInfo => ({ getPayloadInfo: async () => payloadInfo }) diff --git a/packages/plugin-electron-renderer-strip-project-root/test/strip-project-root.test.ts b/packages/plugin-electron-renderer-strip-project-root/test/strip-project-root.test.ts index eee836aeb6..7562da07db 100644 --- a/packages/plugin-electron-renderer-strip-project-root/test/strip-project-root.test.ts +++ b/packages/plugin-electron-renderer-strip-project-root/test/strip-project-root.test.ts @@ -55,7 +55,7 @@ describe('plugin: stack frame file trimmer', () => { async function sendEvent (initialStackframe: any, projectRoot: string|null = null) { const config = { projectRoot } const schema = { projectRoot: { defaultValue: () => null, validate: () => true } } - const { client, sendEvent } = makeClientForPlugin({ config, schema, plugin }) + const { client, sendEvent } = makeClientForPlugin({ config, schema, plugins: [plugin] }) client.addOnError((event: Event) => { event.errors[0].stacktrace = [initialStackframe] }, true) diff --git a/packages/plugin-electron-screen-breadcrumbs/test/screen-breadcrumbs.test.ts b/packages/plugin-electron-screen-breadcrumbs/test/screen-breadcrumbs.test.ts index 7dd5015348..584c51028e 100644 --- a/packages/plugin-electron-screen-breadcrumbs/test/screen-breadcrumbs.test.ts +++ b/packages/plugin-electron-screen-breadcrumbs/test/screen-breadcrumbs.test.ts @@ -5,7 +5,7 @@ import plugin from '../' describe('plugin: electron screen breadcrumbs', () => { it('leaves a breadcrumb for the "display-added" event', () => { const screen = makeScreen() - const { client } = makeClientForPlugin({ plugin: plugin(screen) }) + const { client } = makeClientForPlugin({ plugins: [plugin(screen)] }) const display = makeDisplay({ id: 1234 }) @@ -21,7 +21,7 @@ describe('plugin: electron screen breadcrumbs', () => { it('leaves a breadcrumb for the "display-removed" event', () => { const screen = makeScreen() - const { client } = makeClientForPlugin({ plugin: plugin(screen) }) + const { client } = makeClientForPlugin({ plugins: [plugin(screen)] }) const display = makeDisplay({ id: 1234 }) @@ -46,7 +46,7 @@ describe('plugin: electron screen breadcrumbs', () => { ] ])('leaves a breadcrumb for the "display-metrics-changed" event with changedMetrics = %p', (changedMetrics, message) => { const screen = makeScreen() - const { client } = makeClientForPlugin({ plugin: plugin(screen) }) + const { client } = makeClientForPlugin({ plugins: [plugin(screen)] }) const display = makeDisplay({ id: 23456 }) @@ -61,7 +61,7 @@ describe('plugin: electron screen breadcrumbs', () => { it('honours enabledBreadcrumbTypes', () => { const screen = makeScreen() const { client } = makeClientForPlugin({ - plugin: plugin(screen), + plugins: [plugin(screen)], config: { enabledBreadcrumbTypes: [] } }) @@ -77,7 +77,7 @@ describe('plugin: electron screen breadcrumbs', () => { it('works when enabledBreadcrumbTypes=null', () => { const screen = makeScreen() const { client } = makeClientForPlugin({ - plugin: plugin(screen), + plugins: [plugin(screen)], config: { enabledBreadcrumbTypes: null } }) @@ -92,7 +92,7 @@ describe('plugin: electron screen breadcrumbs', () => { it('anonymises IDs correctly', () => { const screen = makeScreen() - const { client } = makeClientForPlugin({ plugin: plugin(screen) }) + const { client } = makeClientForPlugin({ plugins: [plugin(screen)] }) const display = makeDisplay({ id: 1234 }) From c218a10f722a5f21cd932eb38b038b8ef86aedbe Mon Sep 17 00:00:00 2001 From: Yousif Ahmed Date: Mon, 20 Nov 2023 13:54:25 +0000 Subject: [PATCH 4/7] (plugin-electron-app) Add unit tests for disabled NativeClient --- packages/plugin-electron-app/test/app.test.ts | 51 +++++++++++++++++-- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/packages/plugin-electron-app/test/app.test.ts b/packages/plugin-electron-app/test/app.test.ts index 52e9169e16..3ca4e56633 100644 --- a/packages/plugin-electron-app/test/app.test.ts +++ b/packages/plugin-electron-app/test/app.test.ts @@ -700,7 +700,7 @@ describe('plugin: electron app info', () => { expect(NativeClient.setApp).toHaveBeenCalledTimes(2) }) - it('does not sync "launchDurationMillis" elapsing after "markLaunchComplete" has been caled', async () => { + it('does not sync "launchDurationMillis" elapsing after "markLaunchComplete" has been called', async () => { jest.useFakeTimers('modern') const now = Date.now() @@ -748,6 +748,38 @@ describe('plugin: electron app info', () => { 'Invalid configuration\n - launchDurationMillis should be an integer ≥0, got -1234567890' )) }) + + it('does not sync data to NativeClient when enabledErrorTypes.nativeCrashes is disabled', async () => { + const NativeClient = makeNativeClient() + const config = { launchDurationMillis: 0, enabledErrorTypes: { nativeCrashes: false } } + const process = makeProcess({ platform: 'darwin' }) + const { sendEvent, sendSession } = makeClient({ NativeClient, config, process }) + + // app info should still be updated in the JS client + const event = await sendEvent() + expect(event.app).toEqual(makeExpectedEventApp({ type: 'macOS' })) + + const session = await sendSession() + expect(session.app).toEqual(makeExpectedSessionApp({ type: 'macOS' })) + + expect(NativeClient.setApp).not.toHaveBeenCalled() + }) + + it('does not sync data to NativeClient when autoDetectErrors is disabled', async () => { + const NativeClient = makeNativeClient() + const config = { launchDurationMillis: 0, autoDetectErrors: false } + const process = makeProcess({ platform: 'darwin' }) + const { sendEvent, sendSession } = makeClient({ NativeClient, config, process }) + + // app info should still be updated in the JS client + const event = await sendEvent() + expect(event.app).toEqual(makeExpectedEventApp({ type: 'macOS' })) + + const session = await sendSession() + expect(session.app).toEqual(makeExpectedSessionApp({ type: 'macOS' })) + + expect(NativeClient.setApp).not.toHaveBeenCalled() + }) }) interface MakeClientOptions { @@ -756,10 +788,22 @@ interface MakeClientOptions { NativeClient?: any NativeApp?: any process?: any - config?: { launchDurationMillis: number|undefined } + config?: object filestore?: any } +const schema = { + enabledErrorTypes: { + defaultValue: () => ({ + unhandledExceptions: true, + unhandledRejections: true, + nativeCrashes: true + }), + allowPartialObject: true, + validate: value => true + } +} + function makeClient ({ BrowserWindow = makeBrowserWindow(), electronApp = makeElectronApp({ BrowserWindow }), @@ -771,7 +815,8 @@ function makeClient ({ }: MakeClientOptions = {}): ReturnType { return makeClientForPlugin({ config, - plugin: plugin(NativeClient, process, electronApp, BrowserWindow, filestore, NativeApp) + schema, + plugins: [plugin(NativeClient, process, electronApp, BrowserWindow, filestore, NativeApp)] }) } From 71e2d6bc175c81892ec67257362477cdf66640ea Mon Sep 17 00:00:00 2001 From: Yousif Ahmed Date: Mon, 20 Nov 2023 13:55:00 +0000 Subject: [PATCH 5/7] (plugin-electron-device) Add unit tests for disabled NativeClient --- .../test/device.test.ts | 64 +++++++++++++++++-- 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/packages/plugin-electron-device/test/device.test.ts b/packages/plugin-electron-device/test/device.test.ts index 82356bb453..0747b53483 100644 --- a/packages/plugin-electron-device/test/device.test.ts +++ b/packages/plugin-electron-device/test/device.test.ts @@ -3,15 +3,39 @@ import plugin from '../' const nextTick = async () => new Promise(process.nextTick) +const schema = { + enabledErrorTypes: { + defaultValue: () => ({ + unhandledExceptions: true, + unhandledRejections: true, + nativeCrashes: true + }), + allowPartialObject: true, + validate: value => true + } +} + +interface MakeClientOptions { + app?: any + screen?: any + process?: any + fileStore?: any + NativeClient?: any + powerMonitor?: any + config?: object + filestore?: any +} + function makeClient ({ app = makeApp(), screen = makeScreen(), process = makeProcess(), filestore = makeFilestore(), NativeClient = makeNativeClient(), - powerMonitor = makePowerMonitor() -} = {}) { - return makeClientForPlugin({ plugin: plugin(app, screen, process, filestore, NativeClient, powerMonitor) }) + powerMonitor = makePowerMonitor(), + config = undefined +}: MakeClientOptions = {}) { + return makeClientForPlugin({ plugins: [plugin(app, screen, process, filestore, NativeClient, powerMonitor)], schema, config }) } const DEFAULTS = { @@ -101,6 +125,38 @@ describe('plugin: electron device info', () => { expect(session.device).toEqual(makeExpectedSessionDevice()) }) + it('does not sync device information to the NativeClient when enabledErrorTypes.nativeCrashes is disabled', async () => { + const NativeClient = makeNativeClient() + + const { sendEvent, sendSession } = makeClient({ NativeClient, config: { enabledErrorTypes: { nativeCrashes: false } } }) + + await nextTick() + + const event = await sendEvent() + expect(event.device).toEqual(makeExpectedEventDevice()) + expect(event.getMetadata('device')).toEqual(makeExpectedMetadataDevice()) + + const session = await sendSession() + expect(session.device).toEqual(makeExpectedSessionDevice()) + expect(NativeClient.setDevice).not.toHaveBeenCalled() + }) + + it('does not sync device information to the NativeClient when autoDetectErrors is disabled', async () => { + const NativeClient = makeNativeClient() + + const { sendEvent, sendSession } = makeClient({ NativeClient, config: { autoDetectErrors: false } }) + + await nextTick() + + const event = await sendEvent() + expect(event.device).toEqual(makeExpectedEventDevice()) + expect(event.getMetadata('device')).toEqual(makeExpectedMetadataDevice()) + + const session = await sendSession() + expect(session.device).toEqual(makeExpectedSessionDevice()) + expect(NativeClient.setDevice).not.toHaveBeenCalled() + }) + it('reports correct locale and OS for macOS', async () => { const process = makeProcess({ platform: 'darwin' }) @@ -170,7 +226,6 @@ describe('plugin: electron device info', () => { it('does not report screen information if there is no primary display', async () => { const screen = { getPrimaryDisplay: () => undefined, on: () => {} } - // @ts-expect-error const { sendEvent, sendSession } = makeClient({ screen }) await nextTick() @@ -283,7 +338,6 @@ describe('plugin: electron device info', () => { it('does not add device.id when one is not created', async () => { const filestore = { getDeviceInfo: jest.fn().mockReturnValue({}) } - // @ts-expect-error const { sendEvent, sendSession } = makeClient({ filestore }) await nextTick() From 0182817b24cf3e016d2befecc19e2fab85b0e713 Mon Sep 17 00:00:00 2001 From: Yousif Ahmed Date: Mon, 20 Nov 2023 13:56:00 +0000 Subject: [PATCH 6/7] (plugin-electron-client-state-persistence) Add unit tests for disabled NativeClient --- .../package.json | 3 +- .../test/client-state-persistence.test.ts | 254 +++++++++--------- 2 files changed, 126 insertions(+), 131 deletions(-) diff --git a/packages/plugin-electron-client-state-persistence/package.json b/packages/plugin-electron-client-state-persistence/package.json index 908f07fc54..2b92c476a3 100644 --- a/packages/plugin-electron-client-state-persistence/package.json +++ b/packages/plugin-electron-client-state-persistence/package.json @@ -41,7 +41,8 @@ "devDependencies": { "@bugsnag/core": "^7.19.0", "@bugsnag/plugin-electron-client-state-manager": "^7.19.0", - "@types/bindings": "^1.5.0" + "@types/bindings": "^1.5.0", + "@bugsnag/electron-test-helpers": "^7.19.0" }, "peerDependencies": { "@bugsnag/core": "^7.9.2" diff --git a/packages/plugin-electron-client-state-persistence/test/client-state-persistence.test.ts b/packages/plugin-electron-client-state-persistence/test/client-state-persistence.test.ts index 1e4cdb1fe8..c9a9bca231 100644 --- a/packages/plugin-electron-client-state-persistence/test/client-state-persistence.test.ts +++ b/packages/plugin-electron-client-state-persistence/test/client-state-persistence.test.ts @@ -1,114 +1,94 @@ -import Client from '@bugsnag/core/client' import { plugin } from '../' -import { Breadcrumb, Logger } from '@bugsnag/core' +import { Breadcrumb } from '@bugsnag/core' import stateManager from '@bugsnag/plugin-electron-client-state-manager' +import { makeClientForPlugin } from '@bugsnag/electron-test-helpers' + +const schema = { + enabledErrorTypes: { + defaultValue: () => ({ + unhandledExceptions: true, + unhandledRejections: true, + nativeCrashes: true + }), + allowPartialObject: true, + validate: value => true + } +} + +function makeClient (NativeClient: object, config?: object) { + return makeClientForPlugin({ plugins: [stateManager, plugin(NativeClient)], schema, config }) +} describe('plugin: electron client sync', () => { it('updates context', done => { - const c = new Client({ - apiKey: 'api_key', - plugins: [ - stateManager, - plugin({ - updateContext: (update: any) => { - expect(update).toBe('1234') - done() - } - }) - ] + const { client } = makeClient({ + updateContext: (update: any) => { + expect(update).toBe('1234') + done() + } }) - c.setContext('1234') + client.setContext('1234') }) it('updates metadata', done => { - const c = new Client({ - apiKey: 'api_key', - plugins: [ - stateManager, - plugin({ - updateMetadata: (key: string, updates: any) => { - expect(key).toBe('widget') - expect(updates).toEqual({ - id: '14', - count: 340 - }) - done() - } + const { client } = makeClient({ + updateMetadata: (key: string, updates: any) => { + expect(key).toBe('widget') + expect(updates).toEqual({ + id: '14', + count: 340 }) - ] + done() + } }) - c.addMetadata('widget', { id: '14', count: 340 }) - expect(c.getMetadata('widget')).toEqual({ id: '14', count: 340 }) + client.addMetadata('widget', { id: '14', count: 340 }) + expect(client.getMetadata('widget')).toEqual({ id: '14', count: 340 }) }) it('clears metadata', done => { - const c = new Client({ - apiKey: 'api_key', - plugins: [ - stateManager, - plugin({ - addMetadata: () => {}, - clearMetadata: () => {} - }) - ] + const { client } = makeClient({ + addMetadata: () => {}, + clearMetadata: () => {} }) - c.addMetadata('widget', { id: '14', count: 340 }) - expect(c.getMetadata('widget')).toEqual({ id: '14', count: 340 }) - c.clearMetadata('widget', 'count') - expect(c.getMetadata('widget', 'count')).toBeUndefined() - c.clearMetadata('widget') - expect(c.getMetadata('widget')).toBeUndefined() + client.addMetadata('widget', { id: '14', count: 340 }) + expect(client.getMetadata('widget')).toEqual({ id: '14', count: 340 }) + client.clearMetadata('widget', 'count') + expect(client.getMetadata('widget', 'count')).toBeUndefined() + client.clearMetadata('widget') + expect(client.getMetadata('widget')).toBeUndefined() done() }) it('updates user', done => { - const c = new Client({ - apiKey: 'api_key', - plugins: [ - stateManager, - plugin({ - updateUser: (id: string, email: string, name: string) => { - expect(id).toBe('1234') - expect(name).toBe('Ben') - expect(email).toBe('user@example.com') - done() - } - }) - ] + const { client } = makeClient({ + updateUser: (id: string, email: string, name: string) => { + expect(id).toBe('1234') + expect(name).toBe('Ben') + expect(email).toBe('user@example.com') + done() + } }) - c.setUser('1234', 'user@example.com', 'Ben') - expect(c.getUser()).toEqual({ id: '1234', name: 'Ben', email: 'user@example.com' }) + client.setUser('1234', 'user@example.com', 'Ben') + expect(client.getUser()).toEqual({ id: '1234', name: 'Ben', email: 'user@example.com' }) }) it('syncs breadcrumbs', (done) => { - const c = new Client({ - apiKey: 'api_key', - plugins: [ - stateManager, - plugin({ - leaveBreadcrumb: ({ message, metadata, type, timestamp }: Breadcrumb) => { - expect(message).toBe('Spin') - expect(type).toBe('manual') - expect(metadata).toEqual({ direction: 'ccw', deg: '90' }) - expect(timestamp).toBeTruthy() - done() - } - }) - ] + const { client } = makeClient({ + leaveBreadcrumb: ({ message, metadata, type, timestamp }: Breadcrumb) => { + expect(message).toBe('Spin') + expect(type).toBe('manual') + expect(metadata).toEqual({ direction: 'ccw', deg: '90' }) + expect(timestamp).toBeTruthy() + done() + } }) - c.leaveBreadcrumb('Spin', { direction: 'ccw', deg: '90' }) + client.leaveBreadcrumb('Spin', { direction: 'ccw', deg: '90' }) }) it('does not sync breadcrumbs that are cancelled by an onBreadcrumb callback', () => { const NativeClient = { leaveBreadcrumb: jest.fn() } - const client = new Client({ - apiKey: 'api_key', - plugins: [ - stateManager, - plugin(NativeClient) - ] - }) + const { client } = makeClient(NativeClient) client.addOnBreadcrumb(breadcrumb => { if (breadcrumb.message === 'skip me') { @@ -138,13 +118,7 @@ describe('plugin: electron client sync', () => { it('updates feature flags', () => { const updateFeatureFlags = jest.fn() - const client = new Client({ - apiKey: 'api_key', - plugins: [ - stateManager, - plugin({ updateFeatureFlags }) - ] - }) + const { client } = makeClient({ updateFeatureFlags }) client.addFeatureFlag('a', 'b') client.addFeatureFlags([{ name: 'c', variant: null }, { name: 'd', variant: 'e' }]) @@ -163,13 +137,7 @@ describe('plugin: electron client sync', () => { it('clears a single feature flag', () => { const updateFeatureFlags = jest.fn() - const client = new Client({ - apiKey: 'api_key', - plugins: [ - stateManager, - plugin({ updateFeatureFlags }) - ] - }) + const { client } = makeClient({ updateFeatureFlags }) client.addFeatureFlag('a', 'b') client.addFeatureFlags([{ name: 'c', variant: null }, { name: 'd', variant: 'e' }]) @@ -193,13 +161,7 @@ describe('plugin: electron client sync', () => { it('clears all feature flags', () => { const updateFeatureFlags = jest.fn() - const client = new Client({ - apiKey: 'api_key', - plugins: [ - stateManager, - plugin({ updateFeatureFlags }) - ] - }) + const { client } = makeClient({ updateFeatureFlags }) client.addFeatureFlag('a', 'b') client.addFeatureFlags([{ name: 'c', variant: null }, { name: 'd', variant: 'e' }]) @@ -217,81 +179,113 @@ describe('plugin: electron client sync', () => { expect(updateFeatureFlags).toHaveBeenNthCalledWith(3, []) }) - function loggingClient (NativeClient: object): [Client, Logger] { - const logger = { - debug: jest.fn(), - warn: jest.fn(), - info: jest.fn(), - error: jest.fn() - } - const client = new Client({ - apiKey: 'api_key', - plugins: [stateManager, plugin(NativeClient)], - logger - }) - return [client, logger] - } - it('logs errors thrown from updating context', () => { - const [client, logger] = loggingClient({ + const { client } = makeClient({ updateContext: () => { throw new Error('wrong thing') } }) + client.setContext('some invalid context') - const error = logger.error as jest.Mock + const error = client._logger.error as jest.Mock expect(error.mock.calls.length).toBe(1) expect(error.mock.calls[0][0].message).toContain('wrong thing') }) it('logs errors thrown from adding breadcrumbs', () => { - const [client, logger] = loggingClient({ + const { client } = makeClient({ leaveBreadcrumb: () => { throw new Error('wrong thing') } }) client.leaveBreadcrumb('Spin', { direction: 'ccw', deg: '90' }) - const error = logger.error as jest.Mock + const error = client._logger.error as jest.Mock expect(error.mock.calls.length).toBe(1) expect(error.mock.calls[0][0].message).toContain('wrong thing') }) it('logs errors thrown from adding metadata', () => { - const [client, logger] = loggingClient({ + const { client } = makeClient({ updateMetadata: () => { throw new Error('wrong thing') } }) client.addMetadata('widget', { id: '14', count: 340 }) - const error = logger.error as jest.Mock + const error = client._logger.error as jest.Mock expect(error.mock.calls.length).toBe(1) expect(error.mock.calls[0][0].message).toContain('wrong thing') }) it('logs errors thrown from clearing metadata', () => { - const [client, logger] = loggingClient({ + const { client } = makeClient({ updateMetadata: () => { throw new Error('wrong thing') } }) client.clearMetadata('widget') - const error = logger.error as jest.Mock + const error = client._logger.error as jest.Mock expect(error.mock.calls.length).toBe(1) expect(error.mock.calls[0][0].message).toContain('wrong thing') }) it('logs errors thrown from updating user info', () => { - const [client, logger] = loggingClient({ + const { client } = makeClient({ updateUser: () => { throw new Error('wrong thing') } }) client.setUser('404', 'tim@example.com', undefined) - const error = logger.error as jest.Mock + const error = client._logger.error as jest.Mock expect(error.mock.calls.length).toBe(1) expect(error.mock.calls[0][0].message).toContain('wrong thing') }) it('logs errors thrown from updating feature flags', () => { - const [client, logger] = loggingClient({ + const { client } = makeClient({ updateFeatureFlags: () => { throw new Error('wrong thing') } }) client.addFeatureFlag('a', 'b') - const error = logger.error as jest.Mock + const error = client._logger.error as jest.Mock expect(error.mock.calls.length).toBe(1) expect(error.mock.calls[0][0].message).toContain('wrong thing') }) + + it('does not sync data to the NativeClient if enabledErrorTypes.nativeCrashes is disabled', () => { + const NativeClient = { + leaveBreadcrumb: jest.fn(), + updateUser: jest.fn(), + updateContext: jest.fn(), + updateMetadata: jest.fn(), + updateFeatureFlags: jest.fn() + } + + const { client } = makeClient(NativeClient, { enabledErrorTypes: { nativeCrashes: false } }) + client.leaveBreadcrumb('no sync') + client.setUser('1234', 'user@example.com', 'Ben') + client.setContext('no sync') + client.addMetadata('no sync', { id: '14', count: 340 }) + client.addFeatureFlag('no', 'sync') + + expect(NativeClient.leaveBreadcrumb).not.toHaveBeenCalled() + expect(NativeClient.updateUser).not.toHaveBeenCalled() + expect(NativeClient.updateContext).not.toHaveBeenCalled() + expect(NativeClient.updateMetadata).not.toHaveBeenCalled() + expect(NativeClient.updateFeatureFlags).not.toHaveBeenCalled() + }) + + it('does not sync data to the NativeClient if autoDetectErrors is disabled', () => { + const NativeClient = { + leaveBreadcrumb: jest.fn(), + updateUser: jest.fn(), + updateContext: jest.fn(), + updateMetadata: jest.fn(), + updateFeatureFlags: jest.fn() + } + + const { client } = makeClient(NativeClient, { autoDetectErrors: false }) + client.leaveBreadcrumb('no sync') + client.setUser('1234', 'user@example.com', 'Ben') + client.setContext('no sync') + client.addMetadata('no sync', { id: '14', count: 340 }) + client.addFeatureFlag('no', 'sync') + + expect(NativeClient.leaveBreadcrumb).not.toHaveBeenCalled() + expect(NativeClient.updateUser).not.toHaveBeenCalled() + expect(NativeClient.updateContext).not.toHaveBeenCalled() + expect(NativeClient.updateMetadata).not.toHaveBeenCalled() + expect(NativeClient.updateFeatureFlags).not.toHaveBeenCalled() + }) }) From 81080c9966e384778d741868256e9cf252c333f8 Mon Sep 17 00:00:00 2001 From: Yousif Ahmed Date: Mon, 20 Nov 2023 17:04:37 +0000 Subject: [PATCH 7/7] update CHANGELOG --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a85f350583..f7d99e6ef5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,13 @@ ### Changed -- (react-native) Update bugsnag-android from v5.28.4 to [v5.31.3](https://github.com/bugsnag/bugsnag-cocoa/blob/master/CHANGELOG.md#5313-2023-11-06) +- (react-native) Update bugsnag-android from v5.28.4 to [v5.31.3](https://github.com/bugsnag/bugsnag-android/blob/master/CHANGELOG.md#5313-2023-11-06) - (react-native) Update bugsnag-cocoa from v6.26.2 to [v6.27.3](https://github.com/bugsnag/bugsnag-cocoa/blob/master/CHANGELOG.md#6273-2023-11-15) +### Fixed + +- (electron) Do not sync to NativeClient when `autoDetectErrors` or `nativeCrashes` are disabled [#2040](https://github.com/bugsnag/bugsnag-js/pull/2040) + ## 7.22.1 (2023-10-31) ### Fixed