From e3ef40607c9d045a39007165d4d0a3fa355dbacb Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 30 Aug 2023 16:05:26 +0200 Subject: [PATCH 01/14] feat: Add option to track app focus as sessions --- .../integrations/browser-window-session.ts | 73 +++++++++++++++++++ src/main/integrations/index.ts | 1 + src/main/integrations/main-process-session.ts | 46 +----------- src/main/sessions.ts | 41 ++++++++++- .../sessions/good-window/package.json | 8 ++ .../test-apps/sessions/good-window/recipe.yml | 4 + .../sessions/good-window/session-first.json | 17 +++++ .../sessions/good-window/session-fourth.json | 17 +++++ .../sessions/good-window/session-second.json | 17 +++++ .../sessions/good-window/session-third.json | 17 +++++ .../sessions/good-window/src/index.html | 15 ++++ .../sessions/good-window/src/main.js | 35 +++++++++ 12 files changed, 246 insertions(+), 45 deletions(-) create mode 100644 src/main/integrations/browser-window-session.ts create mode 100644 test/e2e/test-apps/sessions/good-window/package.json create mode 100644 test/e2e/test-apps/sessions/good-window/recipe.yml create mode 100644 test/e2e/test-apps/sessions/good-window/session-first.json create mode 100644 test/e2e/test-apps/sessions/good-window/session-fourth.json create mode 100644 test/e2e/test-apps/sessions/good-window/session-second.json create mode 100644 test/e2e/test-apps/sessions/good-window/session-third.json create mode 100644 test/e2e/test-apps/sessions/good-window/src/index.html create mode 100644 test/e2e/test-apps/sessions/good-window/src/main.js diff --git a/src/main/integrations/browser-window-session.ts b/src/main/integrations/browser-window-session.ts new file mode 100644 index 00000000..85a5bec9 --- /dev/null +++ b/src/main/integrations/browser-window-session.ts @@ -0,0 +1,73 @@ +import { Integration } from '@sentry/types'; +import { app, BrowserWindow } from 'electron'; + +import { endSession, endSessionOnExit, startSession } from '../sessions'; + +interface Options { + /** + * Number of seconds to wait before ending a session after the app loses focus. + * + * Default: 10 seconds + */ + noFocusSecondsTimeout?: number; +} + +type SessionState = 'active' | 'inactive' | { timer: NodeJS.Timeout }; + +/** Tracks sessions as BrowserWindows focused. */ +export class BrowserWindowSession implements Integration { + /** @inheritDoc */ + public static id: string = 'BrowserWindowSession'; + + /** @inheritDoc */ + public readonly name: string; + + private _state: SessionState; + + public constructor(private readonly _options: Options = {}) { + this.name = BrowserWindowSession.id; + this._state = 'inactive'; + } + + /** @inheritDoc */ + public setupOnce(): void { + app.on('browser-window-blur', () => this._windowStateChanged()); + app.on('browser-window-focus', () => this._windowStateChanged()); + + this._windowStateChanged(); + + endSessionOnExit(); + } + + /** */ + private _windowStateChanged(): void { + const appHasFocus = !!BrowserWindow.getFocusedWindow(); + + if (appHasFocus) { + if (this._state === 'inactive') { + void startSession(true); + } else if (typeof this._state !== 'string') { + // Clear the timeout since the app has become active again + clearTimeout(this._state.timer); + } + + this._state = 'active'; + } else { + if (this._state === 'active') { + const timeout = (this._options.noFocusSecondsTimeout ?? 10) * 1_000; + + const timer = setTimeout(() => { + // if we're still waiting for the timeout, end the session + if (typeof this._state !== 'string') { + this._state = 'inactive'; + void endSession(); + } + }, timeout) + // unref so this timer doesn't block app exit + .unref(); + + this._state = { timer }; + } + } + } +} diff --git a/src/main/integrations/index.ts b/src/main/integrations/index.ts index 313bb41f..e723f064 100644 --- a/src/main/integrations/index.ts +++ b/src/main/integrations/index.ts @@ -5,6 +5,7 @@ export { SentryMinidump } from './sentry-minidump'; export { ElectronMinidump } from './electron-minidump'; export { PreloadInjection } from './preload-injection'; export { MainProcessSession } from './main-process-session'; +export { BrowserWindowSession } from './browser-window-session'; export { AdditionalContext } from './additional-context'; export { Net } from './net-breadcrumbs'; export { ChildProcess } from './child-process'; diff --git a/src/main/integrations/main-process-session.ts b/src/main/integrations/main-process-session.ts index 983900d3..3d161681 100644 --- a/src/main/integrations/main-process-session.ts +++ b/src/main/integrations/main-process-session.ts @@ -1,8 +1,6 @@ import { Integration } from '@sentry/types'; -import { logger } from '@sentry/utils'; -import { app } from 'electron'; -import { endSession, startSession } from '../sessions'; +import { endSessionOnExit, startSession } from '../sessions'; interface Options { /** @@ -29,46 +27,6 @@ export class MainProcessSession implements Integration { public setupOnce(): void { void startSession(!!this._options.sendOnCreate); - // We track sessions via the 'will-quit' event which is the last event emitted before close. - // - // We need to be the last 'will-quit' listener so as not to interfere with any user defined listeners which may - // call `event.preventDefault()`. - this._ensureExitHandlerLast(); - - // 'before-quit' is always called before 'will-quit' so we listen there and ensure our 'will-quit' handler is still - // the last listener - app.on('before-quit', () => { - this._ensureExitHandlerLast(); - }); - } - - /** - * Hooks 'will-quit' and ensures the handler is always last - */ - private _ensureExitHandlerLast(): void { - app.removeListener('will-quit', this._exitHandler); - app.on('will-quit', this._exitHandler); + endSessionOnExit(); } - - /** Handles the exit */ - private _exitHandler: (event: Electron.Event) => Promise = async (event: Electron.Event) => { - if (event.defaultPrevented) { - return; - } - - logger.log('[MainProcessSession] Exit Handler'); - - // Stop the exit so we have time to send the session - event.preventDefault(); - - try { - // End the session - await endSession(); - } catch (e) { - // Ignore and log any errors which would prevent app exit - logger.warn('[MainProcessSession] Error ending session:', e); - } - - app.exit(); - }; } diff --git a/src/main/sessions.ts b/src/main/sessions.ts index f970b2db..080a8522 100644 --- a/src/main/sessions.ts +++ b/src/main/sessions.ts @@ -2,6 +2,7 @@ import { getCurrentHub, makeSession, updateSession } from '@sentry/core'; import { flush, NodeClient } from '@sentry/node'; import { SerializedSession, Session, SessionContext, SessionStatus } from '@sentry/types'; import { logger } from '@sentry/utils'; +import { app } from 'electron'; import { sentryCachePath } from './fs'; import { Store } from './store'; @@ -11,7 +12,7 @@ const PERSIST_INTERVAL_MS = 60_000; /** Stores the app session in case of termination due to main process crash or app killed */ const sessionStore = new Store(sentryCachePath, 'session', undefined); -/** Previous session that did not exit cleanly */ +/** Previous session if it did not exit cleanly */ let previousSession: Promise | undefined> | undefined = sessionStore.get(); let persistTimer: NodeJS.Timer | undefined; @@ -145,3 +146,41 @@ export function sessionCrashed(): void { hub.captureSession(); } + +/** + * End the current session on app exit + */ +export function endSessionOnExit(): void { + // 'before-quit' is always called before 'will-quit' so we listen there and ensure our 'will-quit' handler is still + // the last listener + app.on('before-quit', () => { + // We track the end of sessions via the 'will-quit' event which is the last event emitted before close. + // + // We need to be the last 'will-quit' listener so as not to interfere with any user defined listeners which may + // call `event.preventDefault()` to abort the exit. + app.removeListener('will-quit', exitHandler); + app.on('will-quit', exitHandler); + }); +} + +/** Handles the exit */ +const exitHandler: (event: Electron.Event) => Promise = async (event: Electron.Event) => { + if (event.defaultPrevented) { + return; + } + + logger.log('[Session] Exit Handler'); + + // Stop the exit so we have time to send the session + event.preventDefault(); + + try { + // End the session + await endSession(); + } catch (e) { + // Ignore and log any errors which would prevent app exit + logger.warn('[Session] Error ending session:', e); + } + + app.exit(); +}; diff --git a/test/e2e/test-apps/sessions/good-window/package.json b/test/e2e/test-apps/sessions/good-window/package.json new file mode 100644 index 00000000..2217bf58 --- /dev/null +++ b/test/e2e/test-apps/sessions/good-window/package.json @@ -0,0 +1,8 @@ +{ + "name": "good-session", + "version": "1.0.0", + "main": "src/main.js", + "dependencies": { + "@sentry/electron": "3.0.0" + } +} diff --git a/test/e2e/test-apps/sessions/good-window/recipe.yml b/test/e2e/test-apps/sessions/good-window/recipe.yml new file mode 100644 index 00000000..475422be --- /dev/null +++ b/test/e2e/test-apps/sessions/good-window/recipe.yml @@ -0,0 +1,4 @@ +description: Good Session - Window +category: Sessions +command: yarn +runTwice: true diff --git a/test/e2e/test-apps/sessions/good-window/session-first.json b/test/e2e/test-apps/sessions/good-window/session-first.json new file mode 100644 index 00000000..7362ab21 --- /dev/null +++ b/test/e2e/test-apps/sessions/good-window/session-first.json @@ -0,0 +1,17 @@ +{ + "appId": "277345", + "sentryKey": "37f8a2ee37c0409d8970bc7559c7c7e4", + "method": "envelope", + "data": { + "sid": "{{id}}", + "init": true, + "started": 0, + "timestamp": 0, + "status": "ok", + "errors": 0, + "duration": 0, + "attrs": { + "release": "good-session@1.0.0" + } + } +} diff --git a/test/e2e/test-apps/sessions/good-window/session-fourth.json b/test/e2e/test-apps/sessions/good-window/session-fourth.json new file mode 100644 index 00000000..e55639a8 --- /dev/null +++ b/test/e2e/test-apps/sessions/good-window/session-fourth.json @@ -0,0 +1,17 @@ +{ + "appId": "277345", + "sentryKey": "37f8a2ee37c0409d8970bc7559c7c7e4", + "method": "envelope", + "data": { + "sid": "{{id}}", + "init": false, + "started": 0, + "timestamp": 0, + "status": "exited", + "errors": 0, + "duration": 0, + "attrs": { + "release": "good-session@1.0.0" + } + } +} diff --git a/test/e2e/test-apps/sessions/good-window/session-second.json b/test/e2e/test-apps/sessions/good-window/session-second.json new file mode 100644 index 00000000..e55639a8 --- /dev/null +++ b/test/e2e/test-apps/sessions/good-window/session-second.json @@ -0,0 +1,17 @@ +{ + "appId": "277345", + "sentryKey": "37f8a2ee37c0409d8970bc7559c7c7e4", + "method": "envelope", + "data": { + "sid": "{{id}}", + "init": false, + "started": 0, + "timestamp": 0, + "status": "exited", + "errors": 0, + "duration": 0, + "attrs": { + "release": "good-session@1.0.0" + } + } +} diff --git a/test/e2e/test-apps/sessions/good-window/session-third.json b/test/e2e/test-apps/sessions/good-window/session-third.json new file mode 100644 index 00000000..7362ab21 --- /dev/null +++ b/test/e2e/test-apps/sessions/good-window/session-third.json @@ -0,0 +1,17 @@ +{ + "appId": "277345", + "sentryKey": "37f8a2ee37c0409d8970bc7559c7c7e4", + "method": "envelope", + "data": { + "sid": "{{id}}", + "init": true, + "started": 0, + "timestamp": 0, + "status": "ok", + "errors": 0, + "duration": 0, + "attrs": { + "release": "good-session@1.0.0" + } + } +} diff --git a/test/e2e/test-apps/sessions/good-window/src/index.html b/test/e2e/test-apps/sessions/good-window/src/index.html new file mode 100644 index 00000000..b43f77af --- /dev/null +++ b/test/e2e/test-apps/sessions/good-window/src/index.html @@ -0,0 +1,15 @@ + + + + + + + + + diff --git a/test/e2e/test-apps/sessions/good-window/src/main.js b/test/e2e/test-apps/sessions/good-window/src/main.js new file mode 100644 index 00000000..c36fed43 --- /dev/null +++ b/test/e2e/test-apps/sessions/good-window/src/main.js @@ -0,0 +1,35 @@ +const path = require('path'); + +const { app, BrowserWindow } = require('electron'); +const { init, Integrations } = require('@sentry/electron'); + +init({ + dsn: '__DSN__', + debug: true, + integrations: [new Integrations.BrowserWindowSession({ noFocusSecondsTimeout: 1 })], + onFatalError: () => {}, +}); + +app.on('ready', () => { + const mainWindow = new BrowserWindow({ + show: true, + webPreferences: { + nodeIntegration: true, + contextIsolation: false, + }, + }); + + mainWindow.loadFile(path.join(__dirname, 'index.html')); + + setTimeout(() => { + mainWindow.minimize(); + + setTimeout(() => { + mainWindow.restore(); + + setTimeout(() => { + app.exit(); + }, 2000); + }, 2000); + }, 2000); +}); From b9aca8d595491f8e601aef88daed59b794060330 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 30 Aug 2023 16:35:27 +0200 Subject: [PATCH 02/14] Filter integrations so only one session integration is included --- src/main/sdk.ts | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/main/sdk.ts b/src/main/sdk.ts index 80e84d07..94b645d9 100644 --- a/src/main/sdk.ts +++ b/src/main/sdk.ts @@ -128,21 +128,32 @@ export function init(userOptions: ElectronMainOptions): void { nodeInit(options); } -/** Sets the default integrations and ensures that multiple minidump integrations are not enabled */ +/** A list of integrations which cause default integrations to be removed */ +const INTEGRATION_OVERRIDES = [ + { override: 'ElectronMinidump', remove: 'SentryMinidump' }, + { override: 'BrowserWindowSession', remove: 'MainProcessSession' }, +]; + +/** Sets the default integrations and ensures that multiple minidump or session integrations are not enabled */ function setDefaultIntegrations(defaults: Integration[], options: ElectronMainOptions): void { if (options.defaultIntegrations === undefined) { - // If ElectronMinidump has been included, automatically remove SentryMinidump - if (Array.isArray(options.integrations) && options.integrations.some((i) => i.name === 'ElectronMinidump')) { - options.defaultIntegrations = defaults.filter((integration) => integration.name !== 'SentryMinidump'); + const removeDefaultsMatching = (user: Integration[], defaults: Integration[]): Integration[] => { + const toRemove = INTEGRATION_OVERRIDES.filter(({ override }) => user.some((i) => i.name === override)).map( + ({ remove }) => remove, + ); + + return defaults.filter((i) => !toRemove.includes(i.name)); + }; + + if (Array.isArray(options.integrations)) { + options.defaultIntegrations = removeDefaultsMatching(options.integrations, defaults); return; } else if (typeof options.integrations === 'function') { const originalFn = options.integrations; options.integrations = (integrations) => { - const userIntegrations = originalFn(integrations); - return userIntegrations.some((i) => i.name === 'ElectronMinidump') - ? userIntegrations.filter((integration) => integration.name !== 'SentryMinidump') - : userIntegrations; + const resultIntegrations = originalFn(integrations); + return removeDefaultsMatching(resultIntegrations, resultIntegrations); }; } From 968eea655529f31c0e742e1678d179d7175c3568 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 30 Aug 2023 16:41:11 +0200 Subject: [PATCH 03/14] Use 30s second timout --- src/main/integrations/browser-window-session.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/integrations/browser-window-session.ts b/src/main/integrations/browser-window-session.ts index 85a5bec9..a94b49a0 100644 --- a/src/main/integrations/browser-window-session.ts +++ b/src/main/integrations/browser-window-session.ts @@ -54,7 +54,7 @@ export class BrowserWindowSession implements Integration { this._state = 'active'; } else { if (this._state === 'active') { - const timeout = (this._options.noFocusSecondsTimeout ?? 10) * 1_000; + const timeout = (this._options.noFocusSecondsTimeout ?? 30) * 1_000; const timer = setTimeout(() => { // if we're still waiting for the timeout, end the session From 34b97a42f79901ea232d6c36bb19f389689ce134 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 30 Aug 2023 16:42:02 +0200 Subject: [PATCH 04/14] change option name --- src/main/integrations/browser-window-session.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/integrations/browser-window-session.ts b/src/main/integrations/browser-window-session.ts index a94b49a0..67bef127 100644 --- a/src/main/integrations/browser-window-session.ts +++ b/src/main/integrations/browser-window-session.ts @@ -9,7 +9,7 @@ interface Options { * * Default: 10 seconds */ - noFocusSecondsTimeout?: number; + backgroundTimeoutSeconds?: number; } type SessionState = 'active' | 'inactive' | { timer: NodeJS.Timeout }; @@ -54,7 +54,7 @@ export class BrowserWindowSession implements Integration { this._state = 'active'; } else { if (this._state === 'active') { - const timeout = (this._options.noFocusSecondsTimeout ?? 30) * 1_000; + const timeout = (this._options.backgroundTimeoutSeconds ?? 30) * 1_000; const timer = setTimeout(() => { // if we're still waiting for the timeout, end the session From 8d55be2bd906d8bf9d513369bdb7b1a1803e9046 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 30 Aug 2023 16:44:18 +0200 Subject: [PATCH 05/14] Correct the test param --- test/e2e/test-apps/sessions/good-window/src/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/test-apps/sessions/good-window/src/main.js b/test/e2e/test-apps/sessions/good-window/src/main.js index c36fed43..03f60cd4 100644 --- a/test/e2e/test-apps/sessions/good-window/src/main.js +++ b/test/e2e/test-apps/sessions/good-window/src/main.js @@ -6,7 +6,7 @@ const { init, Integrations } = require('@sentry/electron'); init({ dsn: '__DSN__', debug: true, - integrations: [new Integrations.BrowserWindowSession({ noFocusSecondsTimeout: 1 })], + integrations: [new Integrations.BrowserWindowSession({ backgroundTimeoutSeconds: 1 })], onFatalError: () => {}, }); From 49a6aa94e1f739b8828f4e67b8f6dd133fd53ff4 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 30 Aug 2023 16:57:23 +0200 Subject: [PATCH 06/14] Rename new test package --- test/e2e/test-apps/sessions/good-window/package.json | 2 +- test/e2e/test-apps/sessions/good-window/session-first.json | 2 +- test/e2e/test-apps/sessions/good-window/session-fourth.json | 2 +- test/e2e/test-apps/sessions/good-window/session-second.json | 2 +- test/e2e/test-apps/sessions/good-window/session-third.json | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/e2e/test-apps/sessions/good-window/package.json b/test/e2e/test-apps/sessions/good-window/package.json index 2217bf58..bdabd15f 100644 --- a/test/e2e/test-apps/sessions/good-window/package.json +++ b/test/e2e/test-apps/sessions/good-window/package.json @@ -1,5 +1,5 @@ { - "name": "good-session", + "name": "good-session-window", "version": "1.0.0", "main": "src/main.js", "dependencies": { diff --git a/test/e2e/test-apps/sessions/good-window/session-first.json b/test/e2e/test-apps/sessions/good-window/session-first.json index 7362ab21..9ae61330 100644 --- a/test/e2e/test-apps/sessions/good-window/session-first.json +++ b/test/e2e/test-apps/sessions/good-window/session-first.json @@ -11,7 +11,7 @@ "errors": 0, "duration": 0, "attrs": { - "release": "good-session@1.0.0" + "release": "good-session-window@1.0.0" } } } diff --git a/test/e2e/test-apps/sessions/good-window/session-fourth.json b/test/e2e/test-apps/sessions/good-window/session-fourth.json index e55639a8..aaec8549 100644 --- a/test/e2e/test-apps/sessions/good-window/session-fourth.json +++ b/test/e2e/test-apps/sessions/good-window/session-fourth.json @@ -11,7 +11,7 @@ "errors": 0, "duration": 0, "attrs": { - "release": "good-session@1.0.0" + "release": "good-session-window@1.0.0" } } } diff --git a/test/e2e/test-apps/sessions/good-window/session-second.json b/test/e2e/test-apps/sessions/good-window/session-second.json index e55639a8..aaec8549 100644 --- a/test/e2e/test-apps/sessions/good-window/session-second.json +++ b/test/e2e/test-apps/sessions/good-window/session-second.json @@ -11,7 +11,7 @@ "errors": 0, "duration": 0, "attrs": { - "release": "good-session@1.0.0" + "release": "good-session-window@1.0.0" } } } diff --git a/test/e2e/test-apps/sessions/good-window/session-third.json b/test/e2e/test-apps/sessions/good-window/session-third.json index 7362ab21..9ae61330 100644 --- a/test/e2e/test-apps/sessions/good-window/session-third.json +++ b/test/e2e/test-apps/sessions/good-window/session-third.json @@ -11,7 +11,7 @@ "errors": 0, "duration": 0, "attrs": { - "release": "good-session@1.0.0" + "release": "good-session-window@1.0.0" } } } From 930e85de4e584dcc0e18fcd9ebda66f29e580e3d Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 30 Aug 2023 17:24:46 +0200 Subject: [PATCH 07/14] Fix test --- test/e2e/test-apps/sessions/good-window/recipe.yml | 1 - test/e2e/test-apps/sessions/good-window/src/main.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/test/e2e/test-apps/sessions/good-window/recipe.yml b/test/e2e/test-apps/sessions/good-window/recipe.yml index 475422be..c355b759 100644 --- a/test/e2e/test-apps/sessions/good-window/recipe.yml +++ b/test/e2e/test-apps/sessions/good-window/recipe.yml @@ -1,4 +1,3 @@ description: Good Session - Window category: Sessions command: yarn -runTwice: true diff --git a/test/e2e/test-apps/sessions/good-window/src/main.js b/test/e2e/test-apps/sessions/good-window/src/main.js index 03f60cd4..11f81568 100644 --- a/test/e2e/test-apps/sessions/good-window/src/main.js +++ b/test/e2e/test-apps/sessions/good-window/src/main.js @@ -28,7 +28,7 @@ app.on('ready', () => { mainWindow.restore(); setTimeout(() => { - app.exit(); + app.quit(); }, 2000); }, 2000); }, 2000); From ff6146c9506b647054a656b7da454f57336697e5 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 30 Aug 2023 18:06:42 +0200 Subject: [PATCH 08/14] Update test versions --- test/e2e/versions.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/versions.json b/test/e2e/versions.json index 9eccc72b..424c7c55 100644 --- a/test/e2e/versions.json +++ b/test/e2e/versions.json @@ -1 +1 @@ -["2.0.18","3.1.13","4.2.12","5.0.13","6.1.12","7.3.3","8.5.5","9.4.4","10.4.7","11.5.0","12.2.3","13.6.9","14.2.9","15.5.7","16.2.8","17.4.11","18.3.15","19.1.9","20.3.12","21.4.4","22.3.20","23.3.12","24.7.1","25.5.0","26.0.0"] +["2.0.18","3.1.13","4.2.12","5.0.13","6.1.12","7.3.3","8.5.5","9.4.4","10.4.7","11.5.0","12.2.3","13.6.9","14.2.9","15.5.7","16.2.8","17.4.11","18.3.15","19.1.9","20.3.12","21.4.4","22.3.22","23.3.13","24.8.1","25.7.0","26.1.0"] From e54140cb2224cecffa87f6cbc8db256dbc9ab552 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 30 Aug 2023 21:55:36 +0200 Subject: [PATCH 09/14] Try blur rather than minimize --- test/e2e/test-apps/sessions/good-window/src/main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/test-apps/sessions/good-window/src/main.js b/test/e2e/test-apps/sessions/good-window/src/main.js index 11f81568..f3926056 100644 --- a/test/e2e/test-apps/sessions/good-window/src/main.js +++ b/test/e2e/test-apps/sessions/good-window/src/main.js @@ -22,10 +22,10 @@ app.on('ready', () => { mainWindow.loadFile(path.join(__dirname, 'index.html')); setTimeout(() => { - mainWindow.minimize(); + mainWindow.blur(); setTimeout(() => { - mainWindow.restore(); + mainWindow.focus(); setTimeout(() => { app.quit(); From 42791f85c3cfd9916db6c6559f7ae35a012673a7 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 30 Aug 2023 23:59:30 +0200 Subject: [PATCH 10/14] Improve status detection and limit to Electron >= v12 --- src/main/electron-normalize.ts | 2 ++ .../integrations/browser-window-session.ts | 34 ++++++++++++++----- .../test-apps/sessions/good-window/recipe.yml | 1 + 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/main/electron-normalize.ts b/src/main/electron-normalize.ts index cbdf4d39..e4f698c7 100644 --- a/src/main/electron-normalize.ts +++ b/src/main/electron-normalize.ts @@ -7,6 +7,8 @@ import { Optional } from '../common/types'; const parsed = parseSemver(process.versions.electron); const version = { major: parsed.major || 0, minor: parsed.minor || 0, patch: parsed.patch || 0 }; +export const ELECTRON_MAJOR_VERSION = version.major; + /** Returns if the app is packaged. Copied from Electron to support < v3 */ export const isPackaged = (() => { const execFile = basename(process.execPath).toLowerCase(); diff --git a/src/main/integrations/browser-window-session.ts b/src/main/integrations/browser-window-session.ts index 67bef127..179d59d8 100644 --- a/src/main/integrations/browser-window-session.ts +++ b/src/main/integrations/browser-window-session.ts @@ -1,6 +1,7 @@ import { Integration } from '@sentry/types'; import { app, BrowserWindow } from 'electron'; +import { ELECTRON_MAJOR_VERSION } from '../electron-normalize'; import { endSession, endSessionOnExit, startSession } from '../sessions'; interface Options { @@ -14,7 +15,11 @@ interface Options { type SessionState = 'active' | 'inactive' | { timer: NodeJS.Timeout }; -/** Tracks sessions as BrowserWindows focused. */ +/** + * Tracks sessions as BrowserWindows focused. + * + * Supports Electron >= v12 + */ export class BrowserWindowSession implements Integration { /** @inheritDoc */ public static id: string = 'BrowserWindowSession'; @@ -25,25 +30,38 @@ export class BrowserWindowSession implements Integration { private _state: SessionState; public constructor(private readonly _options: Options = {}) { + if (ELECTRON_MAJOR_VERSION < 12) { + throw new Error('BrowserWindowSession requires Electron >= v12'); + } + this.name = BrowserWindowSession.id; this._state = 'inactive'; } /** @inheritDoc */ public setupOnce(): void { - app.on('browser-window-blur', () => this._windowStateChanged()); - app.on('browser-window-focus', () => this._windowStateChanged()); + app.on('browser-window-created', (_event, window) => { + window.on('focus', this._windowStateChanged); + window.on('blur', this._windowStateChanged); + window.on('show', this._windowStateChanged); + window.on('hide', this._windowStateChanged); - this._windowStateChanged(); + window.once('closed', () => { + window.removeListener('focus', this._windowStateChanged); + window.removeListener('blur', this._windowStateChanged); + window.removeListener('show', this._windowStateChanged); + window.removeListener('hide', this._windowStateChanged); + }); + }); endSessionOnExit(); } /** */ - private _windowStateChanged(): void { - const appHasFocus = !!BrowserWindow.getFocusedWindow(); + private _windowStateChanged = (): void => { + const aWindowIsActive = BrowserWindow.getAllWindows().some((window) => window.isVisible() && window.isFocused()); - if (appHasFocus) { + if (aWindowIsActive) { if (this._state === 'inactive') { void startSession(true); } else if (typeof this._state !== 'string') { @@ -69,5 +87,5 @@ export class BrowserWindowSession implements Integration { this._state = { timer }; } } - } + }; } diff --git a/test/e2e/test-apps/sessions/good-window/recipe.yml b/test/e2e/test-apps/sessions/good-window/recipe.yml index c355b759..e3e3a34f 100644 --- a/test/e2e/test-apps/sessions/good-window/recipe.yml +++ b/test/e2e/test-apps/sessions/good-window/recipe.yml @@ -1,3 +1,4 @@ description: Good Session - Window category: Sessions command: yarn +condition: version.major >= 12 From 1804928826b11de35b561c2f65799322ad406630 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 31 Aug 2023 11:20:34 +0200 Subject: [PATCH 11/14] Remove empty jsdoc --- src/main/integrations/browser-window-session.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/integrations/browser-window-session.ts b/src/main/integrations/browser-window-session.ts index 179d59d8..027bfbd0 100644 --- a/src/main/integrations/browser-window-session.ts +++ b/src/main/integrations/browser-window-session.ts @@ -57,7 +57,6 @@ export class BrowserWindowSession implements Integration { endSessionOnExit(); } - /** */ private _windowStateChanged = (): void => { const aWindowIsActive = BrowserWindow.getAllWindows().some((window) => window.isVisible() && window.isFocused()); From 52a5ccec1830c64f0fc62046f076b65cd9045dc0 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 31 Aug 2023 12:54:04 +0200 Subject: [PATCH 12/14] use hide/show in CI test for reliability --- test/e2e/test-apps/sessions/good-window/src/main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/test-apps/sessions/good-window/src/main.js b/test/e2e/test-apps/sessions/good-window/src/main.js index f3926056..6e6e5e65 100644 --- a/test/e2e/test-apps/sessions/good-window/src/main.js +++ b/test/e2e/test-apps/sessions/good-window/src/main.js @@ -22,10 +22,10 @@ app.on('ready', () => { mainWindow.loadFile(path.join(__dirname, 'index.html')); setTimeout(() => { - mainWindow.blur(); + mainWindow.hide(); setTimeout(() => { - mainWindow.focus(); + mainWindow.show(); setTimeout(() => { app.quit(); From 31acf110bd23e7656c62a5bbd901f67abcdd7877 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 31 Aug 2023 15:57:53 +0200 Subject: [PATCH 13/14] add some more comments --- src/main/integrations/browser-window-session.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/integrations/browser-window-session.ts b/src/main/integrations/browser-window-session.ts index 027bfbd0..9f6687ad 100644 --- a/src/main/integrations/browser-window-session.ts +++ b/src/main/integrations/browser-window-session.ts @@ -13,6 +13,7 @@ interface Options { backgroundTimeoutSeconds?: number; } +// The state can be, active, inactive, or waiting for a timeout type SessionState = 'active' | 'inactive' | { timer: NodeJS.Timeout }; /** @@ -46,6 +47,7 @@ export class BrowserWindowSession implements Integration { window.on('show', this._windowStateChanged); window.on('hide', this._windowStateChanged); + // when the window is closed we need to remove the listeners window.once('closed', () => { window.removeListener('focus', this._windowStateChanged); window.removeListener('blur', this._windowStateChanged); @@ -54,14 +56,18 @@ export class BrowserWindowSession implements Integration { }); }); + // if the app exits while the session is active, end the session endSessionOnExit(); } private _windowStateChanged = (): void => { + // We need to test all windows for visibility AND focus const aWindowIsActive = BrowserWindow.getAllWindows().some((window) => window.isVisible() && window.isFocused()); if (aWindowIsActive) { + // We are now active if (this._state === 'inactive') { + // If we were inactive, start a new session void startSession(true); } else if (typeof this._state !== 'string') { // Clear the timeout since the app has become active again @@ -71,10 +77,11 @@ export class BrowserWindowSession implements Integration { this._state = 'active'; } else { if (this._state === 'active') { + // We have become inactive, start the timeout const timeout = (this._options.backgroundTimeoutSeconds ?? 30) * 1_000; const timer = setTimeout(() => { - // if we're still waiting for the timeout, end the session + // if the state says we're still waiting for the timeout, end the session if (typeof this._state !== 'string') { this._state = 'inactive'; void endSession(); From ef7c826335300731c1f18d2055f11df5850f9843 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 10 Sep 2023 23:00:50 +0200 Subject: [PATCH 14/14] PR review changes + additional test --- .../integrations/browser-window-session.ts | 21 ++-- .../test-apps/sessions/window-bad/event.json | 99 +++++++++++++++++++ .../sessions/window-bad/package.json | 8 ++ .../test-apps/sessions/window-bad/recipe.yml | 4 + .../sessions/window-bad/session-first.json | 17 ++++ .../sessions/window-bad/session-second.json | 17 ++++ .../sessions/window-bad/session-third.json | 17 ++++ .../sessions/window-bad/src/index.html | 19 ++++ .../test-apps/sessions/window-bad/src/main.js | 27 +++++ .../{good-window => window-good}/package.json | 0 .../{good-window => window-good}/recipe.yml | 0 .../session-first.json | 0 .../session-fourth.json | 0 .../session-second.json | 0 .../session-third.json | 0 .../src/index.html | 0 .../{good-window => window-good}/src/main.js | 0 17 files changed, 218 insertions(+), 11 deletions(-) create mode 100644 test/e2e/test-apps/sessions/window-bad/event.json create mode 100644 test/e2e/test-apps/sessions/window-bad/package.json create mode 100644 test/e2e/test-apps/sessions/window-bad/recipe.yml create mode 100644 test/e2e/test-apps/sessions/window-bad/session-first.json create mode 100644 test/e2e/test-apps/sessions/window-bad/session-second.json create mode 100644 test/e2e/test-apps/sessions/window-bad/session-third.json create mode 100644 test/e2e/test-apps/sessions/window-bad/src/index.html create mode 100644 test/e2e/test-apps/sessions/window-bad/src/main.js rename test/e2e/test-apps/sessions/{good-window => window-good}/package.json (100%) rename test/e2e/test-apps/sessions/{good-window => window-good}/recipe.yml (100%) rename test/e2e/test-apps/sessions/{good-window => window-good}/session-first.json (100%) rename test/e2e/test-apps/sessions/{good-window => window-good}/session-fourth.json (100%) rename test/e2e/test-apps/sessions/{good-window => window-good}/session-second.json (100%) rename test/e2e/test-apps/sessions/{good-window => window-good}/session-third.json (100%) rename test/e2e/test-apps/sessions/{good-window => window-good}/src/index.html (100%) rename test/e2e/test-apps/sessions/{good-window => window-good}/src/main.js (100%) diff --git a/src/main/integrations/browser-window-session.ts b/src/main/integrations/browser-window-session.ts index 9f6687ad..142cdf45 100644 --- a/src/main/integrations/browser-window-session.ts +++ b/src/main/integrations/browser-window-session.ts @@ -14,7 +14,7 @@ interface Options { } // The state can be, active, inactive, or waiting for a timeout -type SessionState = 'active' | 'inactive' | { timer: NodeJS.Timeout }; +type SessionState = { name: 'active' } | { name: 'inactive' } | { name: 'timeout'; timer: NodeJS.Timeout }; /** * Tracks sessions as BrowserWindows focused. @@ -36,7 +36,7 @@ export class BrowserWindowSession implements Integration { } this.name = BrowserWindowSession.id; - this._state = 'inactive'; + this._state = { name: 'inactive' }; } /** @inheritDoc */ @@ -61,36 +61,35 @@ export class BrowserWindowSession implements Integration { } private _windowStateChanged = (): void => { - // We need to test all windows for visibility AND focus - const aWindowIsActive = BrowserWindow.getAllWindows().some((window) => window.isVisible() && window.isFocused()); + const aWindowIsActive = !!BrowserWindow.getFocusedWindow(); if (aWindowIsActive) { // We are now active - if (this._state === 'inactive') { + if (this._state.name === 'inactive') { // If we were inactive, start a new session void startSession(true); - } else if (typeof this._state !== 'string') { + } else if (this._state.name === 'timeout') { // Clear the timeout since the app has become active again clearTimeout(this._state.timer); } - this._state = 'active'; + this._state = { name: 'active' }; } else { - if (this._state === 'active') { + if (this._state.name === 'active') { // We have become inactive, start the timeout const timeout = (this._options.backgroundTimeoutSeconds ?? 30) * 1_000; const timer = setTimeout(() => { // if the state says we're still waiting for the timeout, end the session - if (typeof this._state !== 'string') { - this._state = 'inactive'; + if (this._state.name === 'timeout') { + this._state = { name: 'inactive' }; void endSession(); } }, timeout) // unref so this timer doesn't block app exit .unref(); - this._state = { timer }; + this._state = { name: 'timeout', timer }; } } }; diff --git a/test/e2e/test-apps/sessions/window-bad/event.json b/test/e2e/test-apps/sessions/window-bad/event.json new file mode 100644 index 00000000..1d07c6b5 --- /dev/null +++ b/test/e2e/test-apps/sessions/window-bad/event.json @@ -0,0 +1,99 @@ +{ + "method": "envelope", + "sentryKey": "37f8a2ee37c0409d8970bc7559c7c7e4", + "appId": "277345", + "data": { + "sdk": { + "name": "sentry.javascript.electron", + "packages": [ + { + "name": "npm:@sentry/electron", + "version": "{{version}}" + } + ], + "version": "{{version}}" + }, + "contexts": { + "app": { + "app_name": "bad-session-window", + "app_version": "1.0.0", + "app_start_time": "{{time}}" + }, + "browser": { + "name": "Chrome" + }, + "chrome": { + "name": "Chrome", + "type": "runtime", + "version": "{{version}}" + }, + "device": { + "arch": "{{arch}}", + "family": "Desktop", + "memory_size": 0, + "free_memory": 0, + "processor_count": 0, + "processor_frequency": 0, + "cpu_description": "{{cpu}}", + "screen_resolution":"{{screen}}", + "screen_density": 1, + "language": "{{language}}" + }, + "node": { + "name": "Node", + "type": "runtime", + "version": "{{version}}" + }, + "os": { + "name": "{{platform}}", + "version": "{{version}}" + }, + "runtime": { + "name": "Electron", + "version": "{{version}}" + } + }, + "release": "bad-session-window@1.0.0", + "environment": "development", + "user": { + "ip_address": "{{auto}}" + }, + "exception": { + "values": [ + { + "type": "Error", + "value": "Some renderer error", + "stacktrace": { + "frames": [ + { + "colno": 0, + "filename": "app:///src/index.html", + "function": "{{function}}", + "in_app": true, + "lineno": 0 + } + ] + }, + "mechanism": { + "handled": true, + "type": "instrument" + } + } + ] + }, + "level": "error", + "event_id": "{{id}}", + "platform": "javascript", + "timestamp": 0, + "breadcrumbs": [], + "request": { + "url": "app:///src/index.html" + }, + "tags": { + "event.environment": "javascript", + "event.origin": "electron", + "event.process": "renderer", + "event_type": "javascript" + } + } +} diff --git a/test/e2e/test-apps/sessions/window-bad/package.json b/test/e2e/test-apps/sessions/window-bad/package.json new file mode 100644 index 00000000..8dce09ae --- /dev/null +++ b/test/e2e/test-apps/sessions/window-bad/package.json @@ -0,0 +1,8 @@ +{ + "name": "bad-session-window", + "version": "1.0.0", + "main": "src/main.js", + "dependencies": { + "@sentry/electron": "3.0.0" + } +} diff --git a/test/e2e/test-apps/sessions/window-bad/recipe.yml b/test/e2e/test-apps/sessions/window-bad/recipe.yml new file mode 100644 index 00000000..990c6571 --- /dev/null +++ b/test/e2e/test-apps/sessions/window-bad/recipe.yml @@ -0,0 +1,4 @@ +description: Bad Session - Window +category: Sessions +command: yarn +condition: version.major >= 12 diff --git a/test/e2e/test-apps/sessions/window-bad/session-first.json b/test/e2e/test-apps/sessions/window-bad/session-first.json new file mode 100644 index 00000000..83434cfa --- /dev/null +++ b/test/e2e/test-apps/sessions/window-bad/session-first.json @@ -0,0 +1,17 @@ +{ + "appId": "277345", + "sentryKey": "37f8a2ee37c0409d8970bc7559c7c7e4", + "method": "envelope", + "data": { + "sid": "{{id}}", + "init": true, + "started": 0, + "timestamp": 0, + "status": "ok", + "errors": 0, + "duration": 0, + "attrs": { + "release": "bad-session-window@1.0.0" + } + } +} diff --git a/test/e2e/test-apps/sessions/window-bad/session-second.json b/test/e2e/test-apps/sessions/window-bad/session-second.json new file mode 100644 index 00000000..ca4ea31d --- /dev/null +++ b/test/e2e/test-apps/sessions/window-bad/session-second.json @@ -0,0 +1,17 @@ +{ + "appId": "277345", + "sentryKey": "37f8a2ee37c0409d8970bc7559c7c7e4", + "method": "envelope", + "data": { + "sid": "{{id}}", + "init": false, + "started": 0, + "timestamp": 0, + "status": "ok", + "errors": 1, + "duration": 0, + "attrs": { + "release": "bad-session-window@1.0.0" + } + } +} diff --git a/test/e2e/test-apps/sessions/window-bad/session-third.json b/test/e2e/test-apps/sessions/window-bad/session-third.json new file mode 100644 index 00000000..11b6d849 --- /dev/null +++ b/test/e2e/test-apps/sessions/window-bad/session-third.json @@ -0,0 +1,17 @@ +{ + "appId": "277345", + "sentryKey": "37f8a2ee37c0409d8970bc7559c7c7e4", + "method": "envelope", + "data": { + "sid": "{{id}}", + "init": false, + "started": 0, + "timestamp": 0, + "status": "exited", + "errors": 1, + "duration": 0, + "attrs": { + "release": "bad-session-window@1.0.0" + } + } +} diff --git a/test/e2e/test-apps/sessions/window-bad/src/index.html b/test/e2e/test-apps/sessions/window-bad/src/index.html new file mode 100644 index 00000000..c30b5ca8 --- /dev/null +++ b/test/e2e/test-apps/sessions/window-bad/src/index.html @@ -0,0 +1,19 @@ + + + + + + + + + diff --git a/test/e2e/test-apps/sessions/window-bad/src/main.js b/test/e2e/test-apps/sessions/window-bad/src/main.js new file mode 100644 index 00000000..c542d786 --- /dev/null +++ b/test/e2e/test-apps/sessions/window-bad/src/main.js @@ -0,0 +1,27 @@ +const path = require('path'); + +const { app, BrowserWindow } = require('electron'); +const { init, Integrations } = require('@sentry/electron'); + +init({ + dsn: '__DSN__', + debug: true, + integrations: [new Integrations.BrowserWindowSession({ backgroundTimeoutSeconds: 1 })], + onFatalError: () => {}, +}); + +app.on('ready', () => { + const mainWindow = new BrowserWindow({ + show: true, + webPreferences: { + nodeIntegration: true, + contextIsolation: false, + }, + }); + + mainWindow.loadFile(path.join(__dirname, 'index.html')); + + setTimeout(() => { + app.quit(); + }, 2000); +}); diff --git a/test/e2e/test-apps/sessions/good-window/package.json b/test/e2e/test-apps/sessions/window-good/package.json similarity index 100% rename from test/e2e/test-apps/sessions/good-window/package.json rename to test/e2e/test-apps/sessions/window-good/package.json diff --git a/test/e2e/test-apps/sessions/good-window/recipe.yml b/test/e2e/test-apps/sessions/window-good/recipe.yml similarity index 100% rename from test/e2e/test-apps/sessions/good-window/recipe.yml rename to test/e2e/test-apps/sessions/window-good/recipe.yml diff --git a/test/e2e/test-apps/sessions/good-window/session-first.json b/test/e2e/test-apps/sessions/window-good/session-first.json similarity index 100% rename from test/e2e/test-apps/sessions/good-window/session-first.json rename to test/e2e/test-apps/sessions/window-good/session-first.json diff --git a/test/e2e/test-apps/sessions/good-window/session-fourth.json b/test/e2e/test-apps/sessions/window-good/session-fourth.json similarity index 100% rename from test/e2e/test-apps/sessions/good-window/session-fourth.json rename to test/e2e/test-apps/sessions/window-good/session-fourth.json diff --git a/test/e2e/test-apps/sessions/good-window/session-second.json b/test/e2e/test-apps/sessions/window-good/session-second.json similarity index 100% rename from test/e2e/test-apps/sessions/good-window/session-second.json rename to test/e2e/test-apps/sessions/window-good/session-second.json diff --git a/test/e2e/test-apps/sessions/good-window/session-third.json b/test/e2e/test-apps/sessions/window-good/session-third.json similarity index 100% rename from test/e2e/test-apps/sessions/good-window/session-third.json rename to test/e2e/test-apps/sessions/window-good/session-third.json diff --git a/test/e2e/test-apps/sessions/good-window/src/index.html b/test/e2e/test-apps/sessions/window-good/src/index.html similarity index 100% rename from test/e2e/test-apps/sessions/good-window/src/index.html rename to test/e2e/test-apps/sessions/window-good/src/index.html diff --git a/test/e2e/test-apps/sessions/good-window/src/main.js b/test/e2e/test-apps/sessions/window-good/src/main.js similarity index 100% rename from test/e2e/test-apps/sessions/good-window/src/main.js rename to test/e2e/test-apps/sessions/window-good/src/main.js