diff --git a/apps/electron-backend-e2e/src/electron-process-lifecycle.ts b/apps/electron-backend-e2e/src/electron-process-lifecycle.ts index c48330ca8..11bb38a3e 100644 --- a/apps/electron-backend-e2e/src/electron-process-lifecycle.ts +++ b/apps/electron-backend-e2e/src/electron-process-lifecycle.ts @@ -8,10 +8,10 @@ type ElectronChildProcess = Pick< export interface ClosableElectronApplication { close(): Promise; - process(): ElectronChildProcess; } export interface ElectronExitConfirmationOptions { + readonly childProcess: ElectronChildProcess; readonly closeTimeoutMs: number; readonly exitTimeoutMs: number; } @@ -58,7 +58,10 @@ export async function closeElectronApplicationAndConfirmExit( ): Promise { assertTimeout(options.closeTimeoutMs); assertTimeout(options.exitTimeoutMs); - const child = application.process(); + // Playwright discards its process dispatcher when Electron exits. Keep + // using the Node handle captured immediately after launch, including when + // the application's last window already caused a normal process exit. + const child = options.childProcess; if (hasExited(child)) return; const exit = observeProcessExit(child); diff --git a/apps/electron-backend-e2e/src/electron-test-fixtures.ts b/apps/electron-backend-e2e/src/electron-test-fixtures.ts index 3d4a5e42c..be8ffc951 100644 --- a/apps/electron-backend-e2e/src/electron-test-fixtures.ts +++ b/apps/electron-backend-e2e/src/electron-test-fixtures.ts @@ -7,7 +7,7 @@ import { Page, test as base, } from '@playwright/test'; -import { spawn } from 'child_process'; +import { spawn, type ChildProcess } from 'child_process'; import { createServer, Server } from 'http'; import { accessSync, @@ -136,6 +136,7 @@ declare global { export type LaunchedElectronApp = { electronApp: ElectronApplication; + electronProcess: ChildProcess; mainWindow: Page; }; @@ -234,15 +235,17 @@ export async function launchElectronApp( args, env: buildElectronLaunchEnvironment(dataDir, options), }); + const electronProcess = electronApp.process(); return prepareElectronApplication({ application: electronApp, dispose: (application) => closeElectronApplicationAndConfirmExit(application, { + childProcess: electronProcess, closeTimeoutMs: electronAppCloseTimeoutMs, exitTimeoutMs: electronAppKillWaitMs, }), prepare: async (application) => { - attachElectronProcessDiagnostics(application); + attachElectronProcessDiagnostics(electronProcess); const mainWindow = await findMainWindow(application); await waitForAppReady(mainWindow); await startPortalDebugCapture(mainWindow); @@ -250,6 +253,7 @@ export async function launchElectronApp( await startRendererFrameCapture(mainWindow); return { electronApp: application, + electronProcess, mainWindow, }; }, @@ -458,26 +462,33 @@ export async function launchPackagedElectronApp( NODE_ENV: 'test', }, }); - attachElectronProcessDiagnostics(electronApp); - - const mainWindow = await findMainWindow(electronApp); - await waitForAppReady(mainWindow); - - return { - electronApp, - mainWindow, - }; + const electronProcess = electronApp.process(); + return prepareElectronApplication({ + application: electronApp, + dispose: (application) => + closeElectronApplicationAndConfirmExit(application, { + childProcess: electronProcess, + closeTimeoutMs: electronAppCloseTimeoutMs, + exitTimeoutMs: electronAppKillWaitMs, + }), + prepare: async (application) => { + attachElectronProcessDiagnostics(electronProcess); + const mainWindow = await findMainWindow(application); + await waitForAppReady(mainWindow); + return { + electronApp: application, + electronProcess, + mainWindow, + }; + }, + }); } -function attachElectronProcessDiagnostics( - electronApp: ElectronApplication -): void { +function attachElectronProcessDiagnostics(childProcess: ChildProcess): void { if (!process.env['CI']) { return; } - const childProcess = electronApp.process(); - childProcess.stdout?.on('data', (chunk: Buffer) => { console.log(`[electron stdout] ${chunk.toString().trimEnd()}`); }); @@ -571,31 +582,12 @@ export async function closeElectronAppAndConfirmExit( app: LaunchedElectronApp ): Promise { await closeElectronApplicationAndConfirmExit(app.electronApp, { + childProcess: app.electronProcess, closeTimeoutMs: electronAppCloseTimeoutMs, exitTimeoutMs: electronAppKillWaitMs, }); } -async function waitForPromiseWithTimeout( - promise: Promise, - timeoutMs: number -): Promise { - let timeoutId: NodeJS.Timeout | undefined; - - try { - return await Promise.race([ - promise.then(() => true), - new Promise((resolvePromise) => { - timeoutId = setTimeout(() => resolvePromise(false), timeoutMs); - }), - ]); - } finally { - if (timeoutId) { - clearTimeout(timeoutId); - } - } -} - function assertPackagedRendererBuildIsElectronSafe(): void { if (!existsSync(packagedRendererIndexPath)) { throw new Error( diff --git a/apps/electron-backend-e2e/src/performance/electron-process-lifecycle.spec.ts b/apps/electron-backend-e2e/src/performance/electron-process-lifecycle.spec.ts index ca939452b..d7e3024a1 100644 --- a/apps/electron-backend-e2e/src/performance/electron-process-lifecycle.spec.ts +++ b/apps/electron-backend-e2e/src/performance/electron-process-lifecycle.spec.ts @@ -39,17 +39,43 @@ class FakeChildProcess extends EventEmitter { } describe('Electron process lifecycle', () => { + it('confirms an already exited process after Playwright disposes its dispatcher', async () => { + const child = new FakeChildProcess(); + child.exitCode = 0; + const app = { + electronProcess: child, + electronApp: { + close: async () => { + assert.fail( + 'an exited application must not be closed again' + ); + }, + process: () => { + throw new TypeError( + "Cannot read properties of undefined (reading '_object')" + ); + }, + }, + } as unknown as LaunchedElectronApp; + + await closeElectronApp(app); + assert.equal(child.killCalls, 0); + }); + it('does not return from public cleanup when close and termination both fail', async () => { const child = new FakeChildProcess(); child.kill = () => { throw new Error('termination failed'); }; const app = { + electronProcess: child, electronApp: { close: async () => { throw new Error('CDP disconnected'); }, - process: () => child, + process: () => { + assert.fail('cleanup must use the retained child process'); + }, }, } as unknown as LaunchedElectronApp; @@ -76,6 +102,7 @@ describe('Electron process lifecycle', () => { application, dispose: (app) => closeElectronApplicationAndConfirmExit(app, { + childProcess: child, closeTimeoutMs: 10, exitTimeoutMs: 10, }), @@ -89,6 +116,37 @@ describe('Electron process lifecycle', () => { assert.equal(child.exitCode, 0); }); + it('preserves preparation failure when Electron has already exited', async () => { + const child = new FakeChildProcess(); + const launchFailure = new Error('renderer closed before readiness'); + const application = { + close: async () => { + assert.fail('an exited application must not be closed again'); + }, + process: () => { + assert.fail('the Playwright dispatcher is already disposed'); + }, + }; + + await assert.rejects( + prepareElectronApplication({ + application, + dispose: (app) => + closeElectronApplicationAndConfirmExit(app, { + childProcess: child, + closeTimeoutMs: 10, + exitTimeoutMs: 10, + }), + prepare: async () => { + child.exitCode = 0; + throw launchFailure; + }, + }), + (error: unknown) => error === launchFailure + ); + assert.equal(child.killCalls, 0); + }); + it('fails when process exit remains unconfirmed after forced teardown', async () => { const child = new FakeChildProcess(); child.shouldExitOnKill = false; @@ -99,6 +157,7 @@ describe('Electron process lifecycle', () => { await assert.rejects( closeElectronApplicationAndConfirmExit(application, { + childProcess: child, closeTimeoutMs: 1, exitTimeoutMs: 1, }), @@ -143,6 +202,7 @@ describe('Electron process lifecycle', () => { await Promise.race([ closeElectronApplicationAndConfirmExit(application, { + childProcess: child, closeTimeoutMs: 1_000, exitTimeoutMs: 10, }), @@ -164,6 +224,7 @@ describe('Electron process lifecycle', () => { }; await closeElectronApplicationAndConfirmExit(application, { + childProcess: child, closeTimeoutMs: 1, exitTimeoutMs: 10, }); @@ -177,9 +238,8 @@ describe('Electron process lifecycle', () => { await closeApplication( { close: () => new Promise(() => undefined), - process: () => child, }, - { closeTimeoutMs: 1, exitTimeoutMs: 1 }, + { childProcess: child, closeTimeoutMs: 1, exitTimeoutMs: 1 }, (_child, signal) => { signals.push(signal); if (signal === 'SIGTERM') throw new Error('termination failed'); diff --git a/docs/development/electron-debugging.md b/docs/development/electron-debugging.md index 6281ed6b6..200f3378e 100644 --- a/docs/development/electron-debugging.md +++ b/docs/development/electron-debugging.md @@ -78,6 +78,11 @@ target only its own launch PID, never all Electron or IPTVnator processes. Cleanup confirms process exit even if Playwright's close promise fails or never settles. If termination fails, it retries and then throws instead of allowing a relaunch against a potentially locked profile. +Capture the Node child-process handle immediately after launch and retain it +for cleanup and preparation failures. After the last window closes on Linux, +Electron can exit before cleanup begins; Playwright disposes its dispatcher, +so calling `electronApp.process()` at that point can throw even after a clean +exit. The retained handle still provides the actual exit code and signal. The Linux portable build uploads `packaged-frame-copy-smoke` reports and traces even when the smoke fails. Check the paused-frame screenshot and trace before diff --git a/tools/packaging/snap-workflow-policy.test-helpers.mjs b/tools/packaging/snap-workflow-policy.test-helpers.mjs index 3377a642f..1a922300b 100644 --- a/tools/packaging/snap-workflow-policy.test-helpers.mjs +++ b/tools/packaging/snap-workflow-policy.test-helpers.mjs @@ -13,6 +13,7 @@ const PUBLISH_ACTION_ALLOWLIST = Object.freeze([ PINNED_UPLOAD_ARTIFACT_ACTION, ]); const BUILD_ACTION_ALLOWLIST = Object.freeze([ + PINNED_UPLOAD_ARTIFACT_ACTION, 'actions/cache/restore@v6', 'actions/cache/save@v6', 'actions/cache@v6',