From cf425addff753f5d53213da88aaa11cb51d11948 Mon Sep 17 00:00:00 2001 From: rocketraccoon Date: Tue, 12 May 2026 03:56:09 +0700 Subject: [PATCH 1/2] feat: handling process exit --- src/browser-pool/basic-pool.ts | 5 +- src/browser/browser.ts | 5 ++ src/browser/new-browser.ts | 6 +- src/runner/index.ts | 2 + src/runner/test-runner/regular-test-runner.js | 4 +- src/signal-handler.js | 30 -------- src/signal-handler.ts | 46 ++++++++++++ src/testplane.ts | 1 + src/utils/workers-registry.js | 2 + .../{new-browser.js => new-browser.ts} | 74 +++++++++++-------- .../{signal-handler.js => signal-handler.ts} | 28 +++---- test/src/testplane.js | 2 +- 12 files changed, 124 insertions(+), 81 deletions(-) delete mode 100644 src/signal-handler.js create mode 100644 src/signal-handler.ts rename test/src/browser/{new-browser.js => new-browser.ts} (89%) rename test/src/{signal-handler.js => signal-handler.ts} (65%) diff --git a/src/browser-pool/basic-pool.ts b/src/browser-pool/basic-pool.ts index edbfc21ed..ec0c3f549 100644 --- a/src/browser-pool/basic-pool.ts +++ b/src/browser-pool/basic-pool.ts @@ -63,14 +63,17 @@ export class BasicPool implements Pool { this.log(`stop browser ${browser.fullId}`); + let error; + try { await this._emit(MasterEvents.SESSION_END, browser); // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (err: any) { + error = err; console.warn((err && err.stack) || err); } - await browser.quit(); + await browser.quit(error); } private _emit(event: string, browser: Browser): Promise { diff --git a/src/browser/browser.ts b/src/browser/browser.ts index fa1616fd5..20e40feea 100644 --- a/src/browser/browser.ts +++ b/src/browser/browser.ts @@ -54,6 +54,7 @@ export class Browser { protected _customCommands: Set; protected _wdPool?: WebdriverPool; protected _wdProcess: WdProcess | null; + protected _exitError?: Error; /** This Promise is awaited after test is finished. Can be used for cleanup. Right now is used to wait for time travel snapshots to finish collecting */ protected _snapshotsPromiseRef: history.PromiseRef; @@ -214,4 +215,8 @@ export class Browser { get emitter(): AsyncEmitter { return this._emitter; } + + get exitError(): Error | undefined { + return this._exitError; + } } diff --git a/src/browser/new-browser.ts b/src/browser/new-browser.ts index 0249b8718..f3a81a5e1 100644 --- a/src/browser/new-browser.ts +++ b/src/browser/new-browser.ts @@ -64,7 +64,7 @@ export class NewBrowser extends Browser { constructor(config: Config, opts: BrowserOpts) { super(config, opts); - signalHandler.on("exit", () => this.quit()); + signalHandler.on("exit", (err?: Error) => this.quit(err)); } async init(): Promise { @@ -81,7 +81,9 @@ export class NewBrowser extends Browser { return Promise.resolve(); } - async quit(): Promise { + async quit(err?: Error): Promise { + this._exitError = err; + try { this.setHttpTimeout(this._config.sessionQuitTimeout); await this._session!.deleteSession(); diff --git a/src/runner/index.ts b/src/runner/index.ts index 207982047..2ee1e0332 100644 --- a/src/runner/index.ts +++ b/src/runner/index.ts @@ -92,6 +92,8 @@ export class MainRunner extends RunnableEmitter { this.workersRegistry.init(); this.workers = this.registerWorkers(require.resolve("../worker"), ["runTest", "cancel"] as const) as Workers; this.browserPool = pool.create(this.config, this); + + eventsUtils.passthroughEvent(this, this.workersRegistry, MasterEvents.EXIT); } _isRunning(): boolean { diff --git a/src/runner/test-runner/regular-test-runner.js b/src/runner/test-runner/regular-test-runner.js index dee04fc3b..4bbca5cfa 100644 --- a/src/runner/test-runner/regular-test-runner.js +++ b/src/runner/test-runner/regular-test-runner.js @@ -38,9 +38,9 @@ module.exports = class RegularTestRunner extends RunnableEmitter { this._emit(MasterEvents.TEST_PASS); } catch (error) { - this._test.err = error; + this._test.err = this._browser?.exitError || error; - this._applyTestResults(error); + this._applyTestResults(this._test.err); this._emit(MasterEvents.TEST_FAIL); } diff --git a/src/signal-handler.js b/src/signal-handler.js deleted file mode 100644 index 462eff9a0..000000000 --- a/src/signal-handler.js +++ /dev/null @@ -1,30 +0,0 @@ -"use strict"; - -const { AsyncEmitter } = require("./events/async-emitter"); -const { log } = require("./utils/logger"); -const signalHandler = new AsyncEmitter(); - -signalHandler.setMaxListeners(0); - -module.exports = signalHandler; - -process.on("SIGHUP", notifyAndExit(1)); -process.on("SIGINT", notifyAndExit(2)); -process.on("SIGTERM", notifyAndExit(15)); - -let callCount = 0; - -function notifyAndExit(signalNo) { - const exitCode = 128 + signalNo; - - return function () { - if (callCount++ > 0) { - log("Force quit."); - process.exit(exitCode); - } - - signalHandler.emitAndWait("exit").then(function () { - process.exit(exitCode); - }); - }; -} diff --git a/src/signal-handler.ts b/src/signal-handler.ts new file mode 100644 index 000000000..308870f49 --- /dev/null +++ b/src/signal-handler.ts @@ -0,0 +1,46 @@ +import { AsyncEmitter } from "./events"; +import { log } from "./utils/logger"; +import { MasterEvents } from "./events"; + +const signalHandler = new AsyncEmitter(); + +signalHandler.setMaxListeners(0); + +let callCount = 0; + +let lastCallTime = 0; + +const throttleTime = 10; + +function notifyAndExit(signalNo: number): (signal: NodeJS.Signals) => void { + const exitCode = 128 + signalNo; + + return function (signal: NodeJS.Signals) { + const time = Date.now(); + + if (time - lastCallTime < throttleTime) { + lastCallTime = time; + + return; + } + + lastCallTime = time; + + if (callCount++ > 0) { + log("Force quit."); + process.exit(exitCode); + } + + const err = new Error(`The process was terminated by a signal: ${signal}`); + + signalHandler.emitAndWait(MasterEvents.EXIT, err).then(() => { + process.exit(exitCode); + }); + }; +} + +process.on("SIGHUP", notifyAndExit(1)); +process.on("SIGINT", notifyAndExit(2)); +process.on("SIGTERM", notifyAndExit(15)); + +export default signalHandler; diff --git a/src/testplane.ts b/src/testplane.ts index 583ad8b86..ca30d2717 100644 --- a/src/testplane.ts +++ b/src/testplane.ts @@ -182,6 +182,7 @@ export class Testplane extends BaseTestplane { eventsUtils.passthroughEvent(this.runner, this, _.values(MasterSyncEvents)); eventsUtils.passthroughEventAsync(this.runner, this, _.values(MasterAsyncEvents)); eventsUtils.passthroughEventAsync(signalHandler, this, MasterEvents.EXIT); + eventsUtils.passthroughEventAsync(signalHandler, this.runner, MasterEvents.EXIT); await this._startServersIfNeeded(); await this._emitInitEventOnce(); diff --git a/src/utils/workers-registry.js b/src/utils/workers-registry.js index 80c7fd167..9f3998384 100644 --- a/src/utils/workers-registry.js +++ b/src/utils/workers-registry.js @@ -156,6 +156,8 @@ module.exports = class WorkersRegistry extends EventEmitter { logger.error(`testplane:worker:${child.pid} terminated unexpectedly with ${errMsg}`); }); + this.once(MasterEvents.EXIT, () => child.kill()); + child.on("message", (data = {}) => { switch (data.event) { case WORKER_INIT: diff --git a/test/src/browser/new-browser.js b/test/src/browser/new-browser.ts similarity index 89% rename from test/src/browser/new-browser.js rename to test/src/browser/new-browser.ts index 4c6f72d9c..6ffc169d9 100644 --- a/test/src/browser/new-browser.js +++ b/test/src/browser/new-browser.ts @@ -1,20 +1,27 @@ -"use strict"; - -const crypto = require("crypto"); -const proxyquire = require("proxyquire"); -const signalHandler = require("src/signal-handler"); -const history = require("src/browser/history"); -const { WEBDRIVER_PROTOCOL, DEVTOOLS_PROTOCOL } = require("src/constants/config"); -const { X_REQUEST_ID_DELIMITER } = require("src/constants/browser"); -const RuntimeConfig = require("src/config/runtime-config"); -const { mkNewBrowser_, mkSessionStub_, mkWdPool_ } = require("./utils"); +import sinon, { SinonStub } from "sinon"; +import crypto from "crypto"; +import proxyquire from "proxyquire"; +import signalHandler from "src/signal-handler"; +import { runGroup } from "src/browser/history"; +import { WEBDRIVER_PROTOCOL, DEVTOOLS_PROTOCOL } from "src/constants/config"; +import { X_REQUEST_ID_DELIMITER } from "src/constants/browser"; +import RuntimeConfig from "src/config/runtime-config"; +import { mkNewBrowser_, mkSessionStub_, mkWdPool_ } from "./utils"; +import { Config } from "src/config"; +import { RequestOptions } from "node:https"; +import { DesiredCapabilities, SelenoidOptions } from "@testplane/wdio-types/build/Capabilities"; describe("NewBrowser", () => { const sandbox = sinon.createSandbox(); - let session; - let NewBrowser, webdriverioRemoteStub, runGroupStub, initCommandHistoryStub, installBrowserStub, warnStub; - - const mkBrowser_ = (configOpts, opts) => { + let session: any; + let NewBrowser: any; + let webdriverioRemoteStub: SinonStub; + let runGroupStub: SinonStub; + let initCommandHistoryStub: SinonStub; + let installBrowserStub: SinonStub; + let warnStub: SinonStub; + + const mkBrowser_ = (configOpts?: Partial, opts?: any): any => { return mkNewBrowser_(configOpts, opts, NewBrowser); }; @@ -23,7 +30,7 @@ describe("NewBrowser", () => { installBrowserStub = sandbox.stub().resolves("/browser/path"); warnStub = sandbox.stub(); webdriverioRemoteStub = sandbox.stub().resolves(session); - runGroupStub = sandbox.stub().callsFake(history.runGroup); + runGroupStub = sandbox.stub().callsFake(runGroup); initCommandHistoryStub = sandbox.stub(); NewBrowser = proxyquire("src/browser/new-browser", { @@ -74,7 +81,7 @@ describe("NewBrowser", () => { }); it("should use devtools protocol if testplane runs in devtools mode", async () => { - RuntimeConfig.getInstance.returns({ devtools: true }); + (RuntimeConfig.getInstance as SinonStub).returns({ devtools: true }); await mkBrowser_().init(); @@ -281,7 +288,7 @@ describe("NewBrowser", () => { describe("transformRequest option", () => { beforeEach(() => { - sandbox.stub(crypto, "randomUUID").returns("00000"); + sandbox.stub(crypto, "randomUUID").returns("0-0-0-0-0"); }); it("should call user handler from config", async () => { @@ -297,9 +304,9 @@ describe("NewBrowser", () => { }); it('should not add "X-Request-ID" header if it is already add by user', async () => { - const request = { headers: {} }; - const transformRequestStub = req => { - req.headers["X-Request-ID"] = "100500"; + const request: RequestOptions = { headers: {} }; + const transformRequestStub = (req: RequestOptions): RequestOptions => { + (req.headers as Record)["X-Request-ID"] = "100500"; return req; }; @@ -308,20 +315,23 @@ describe("NewBrowser", () => { const { transformRequest } = webdriverioRemoteStub.lastCall.args[0]; transformRequest(request); - assert.equal(request.headers["X-Request-ID"], "100500"); + assert.equal((request.headers as Record)["X-Request-ID"], "100500"); }); it('should add "X-Request-ID" header', async () => { - crypto.randomUUID.returns("67890"); + (crypto.randomUUID as SinonStub).returns("67890"); const state = { testXReqId: "12345" }; - const request = { headers: {} }; + const request: RequestOptions = { headers: {} }; await mkBrowser_({}, { state }).init(); const { transformRequest } = webdriverioRemoteStub.lastCall.args[0]; transformRequest(request); - assert.equal(request.headers["X-Request-ID"], `12345${X_REQUEST_ID_DELIMITER}67890`); + assert.equal( + (request.headers as Record)["X-Request-ID"], + `12345${X_REQUEST_ID_DELIMITER}67890`, + ); }); }); @@ -340,7 +350,7 @@ describe("NewBrowser", () => { }); describe("set page load timeout if it is specified in a config", () => { - let browser; + let browser: any; beforeEach(() => { browser = mkBrowser_({ pageLoadTimeout: 100500 }); @@ -380,7 +390,7 @@ describe("NewBrowser", () => { describe("should use local grid url", () => { it("if gridUrl is 'local'", async () => { installBrowserStub.withArgs("chrome", "115.0").resolves("/browser/path/chrome/115.0"); - RuntimeConfig.getInstance.returns({ local: false }); + (RuntimeConfig.getInstance as SinonStub).returns({ local: false }); const wdPool = mkWdPool_({ gridUrl: "http://localhost:12345/" }); const browser = mkBrowser_( { @@ -413,7 +423,7 @@ describe("NewBrowser", () => { it("if local cli arg is set", async () => { installBrowserStub.withArgs("chrome", "115.0").resolves("/browser/path/chrome/115.0"); - RuntimeConfig.getInstance.returns({ local: true }); + (RuntimeConfig.getInstance as SinonStub).returns({ local: true }); const wdPool = mkWdPool_({ gridUrl: "http://localhost:12345/" }); const browser = mkBrowser_( { @@ -446,7 +456,7 @@ describe("NewBrowser", () => { it("should remove unknown capabilities", async () => { installBrowserStub.withArgs("chrome", "115.0").resolves("/browser/path/chrome/115.0"); - RuntimeConfig.getInstance.returns({ local: true }); + (RuntimeConfig.getInstance as SinonStub).returns({ local: true }); const wdPool = mkWdPool_({ gridUrl: "http://localhost:23456/" }); const browser = mkBrowser_( { @@ -455,10 +465,10 @@ describe("NewBrowser", () => { desiredCapabilities: { browserName: "chrome", browserVersion: "115.0", - "selenoid:options": { baz: "qux" }, + "selenoid:options": { baz: "qux" } as SelenoidOptions, "moz:firefoxOptions": {}, perfLoggingPrefs: { foo: "bar" }, - }, + } as DesiredCapabilities, }, { wdPool }, ); @@ -516,7 +526,7 @@ describe("NewBrowser", () => { }); it("should free webdriver session", async () => { - RuntimeConfig.getInstance.returns({ local: false }); + (RuntimeConfig.getInstance as SinonStub).returns({ local: false }); const wdProcess = { gridUrl: "http://localhost:12345", free: sandbox.stub(), kill: sandbox.stub() }; const wdPool = { getWebdriver: sandbox.stub().resolves(wdProcess) }; const browser = mkBrowser_( @@ -540,7 +550,7 @@ describe("NewBrowser", () => { }); it("should kill webdriver session if cant quit normally", async () => { - RuntimeConfig.getInstance.returns({ local: false }); + (RuntimeConfig.getInstance as SinonStub).returns({ local: false }); session.deleteSession.rejects(new Error("failed end")); const wdProcess = { gridUrl: "http://localhost:12345", free: sandbox.stub(), kill: sandbox.stub() }; const wdPool = { getWebdriver: sandbox.stub().resolves(wdProcess) }; diff --git a/test/src/signal-handler.js b/test/src/signal-handler.ts similarity index 65% rename from test/src/signal-handler.js rename to test/src/signal-handler.ts index 631a68688..389a324ed 100644 --- a/test/src/signal-handler.js +++ b/test/src/signal-handler.ts @@ -1,25 +1,27 @@ -"use strict"; - -const clearRequire = require("clear-require"); -const proxyquire = require("proxyquire"); -const { promiseDelay } = require("../../src/utils/promise"); +import sinon, { SinonSpyCall, SinonStub } from "sinon"; +import clearRequire from "clear-require"; +import proxyquire from "proxyquire"; +import { promiseDelay } from "../../src/utils/promise"; +import { AsyncEmitter } from "src/events"; describe("src/signal-handler", () => { const sandbox = sinon.createSandbox(); - let signalHandler; + let signalHandler: AsyncEmitter; + let processOnStub: SinonStub; + let processExitStub: SinonStub; - const getCallBySignal = sig => { - return process.on.getCalls().find(call => call.args[0] === sig); + const getCallBySignal = (sig: string): SinonSpyCall => { + return processOnStub.getCalls().find((call: SinonSpyCall) => call.args[0] === sig) as SinonSpyCall; }; - const sendSignal = sig => { + const sendSignal = (sig: string): void => { getCallBySignal(sig).args[1](); }; beforeEach(() => { - sandbox.stub(process, "on"); - sandbox.stub(process, "exit"); + processOnStub = sandbox.stub(process, "on") as SinonStub; + processExitStub = sandbox.stub(process, "exit") as SinonStub; clearRequire("src/signal-handler"); signalHandler = proxyquire("src/signal-handler", { @@ -38,7 +40,7 @@ describe("src/signal-handler", () => { ].forEach(({ signal, exitCode }) => { describe(signal, () => { it(`should subscribe to ${signal} event`, () => { - assert.calledWith(process.on, signal); + assert.calledWith(processOnStub, signal); }); it("should emit and wait for exit", () => { @@ -49,7 +51,7 @@ describe("src/signal-handler", () => { sendSignal(signal); return promiseDelay(20).then(() => { - assert.callOrder(handler, afterHandler, process.exit); + assert.callOrder(handler, afterHandler, processExitStub); }); }); diff --git a/test/src/testplane.js b/test/src/testplane.js index 140091e57..6be90ee73 100644 --- a/test/src/testplane.js +++ b/test/src/testplane.js @@ -604,7 +604,7 @@ describe("testplane", () => { signalHandler.emitAndWait("exit"); - assert.calledOnce(onExit); + assert.calledTwice(onExit); }); }); From 4251d5af697d97c388430e1f9ef044faca299060 Mon Sep 17 00:00:00 2001 From: rocketraccoon Date: Thu, 21 May 2026 02:59:40 +0700 Subject: [PATCH 2/2] fix: emit runner end event --- src/signal-handler.ts | 4 +++- src/testplane.ts | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/signal-handler.ts b/src/signal-handler.ts index 308870f49..b1a54aef0 100644 --- a/src/signal-handler.ts +++ b/src/signal-handler.ts @@ -34,7 +34,9 @@ function notifyAndExit(signalNo: number): (signal: NodeJS.Signals) => void { const err = new Error(`The process was terminated by a signal: ${signal}`); signalHandler.emitAndWait(MasterEvents.EXIT, err).then(() => { - process.exit(exitCode); + signalHandler.emitAndWait(MasterEvents.RUNNER_END, err).then(() => { + process.exit(exitCode); + }); }); }; } diff --git a/src/testplane.ts b/src/testplane.ts index ca30d2717..e56c8aff7 100644 --- a/src/testplane.ts +++ b/src/testplane.ts @@ -182,6 +182,7 @@ export class Testplane extends BaseTestplane { eventsUtils.passthroughEvent(this.runner, this, _.values(MasterSyncEvents)); eventsUtils.passthroughEventAsync(this.runner, this, _.values(MasterAsyncEvents)); eventsUtils.passthroughEventAsync(signalHandler, this, MasterEvents.EXIT); + eventsUtils.passthroughEventAsync(signalHandler, this, MasterEvents.RUNNER_END); eventsUtils.passthroughEventAsync(signalHandler, this.runner, MasterEvents.EXIT); await this._startServersIfNeeded();