From 4d3b4b74b115af5725521731e4d5eeda34e667b0 Mon Sep 17 00:00:00 2001 From: Wojciech Mech Date: Fri, 25 Sep 2026 17:58:44 +0200 Subject: [PATCH 1/8] TD-2600: separate browser liveness from test progress in the server watchdog The server used a single 55 second budget for both "the browser has stopped responding" and "the browser has not finished a test", so a remote browser that was merely slow was killed as if its driver had died. - Controller now tracks liveness (session.alive) separately from progress (session.updated). Silence from the browser is still caught after 55 seconds; a browser that is responding but slow gets the new --progressTimeout, which defaults to 5 minutes. - The runner sends keep-alive pings in auto mode as well as manual. Results are batched and a page reload posts nothing at all, so without them the server has no evidence of liveness other than test results. - /tests/alive now uses the throttled maybeSendKeepAlive, so the extra pings do not issue a webdriver command every few seconds. --- CHANGELOG.md | 3 +++ modules/runner/src/main/ts/runner/Runner.ts | 20 ++++++++--------- .../src/main/ts/bedrock/cli/ClOptions.ts | 9 ++++++++ .../server/src/main/ts/bedrock/cli/Clis.ts | 1 + .../src/main/ts/bedrock/core/Settings.ts | 1 + .../server/src/main/ts/bedrock/server/Apis.ts | 8 ++++--- .../src/main/ts/bedrock/server/Controller.ts | 22 +++++++++++++++---- .../src/main/ts/bedrock/server/Serve.ts | 4 +++- modules/server/src/test/ts/ClisTest.ts | 9 ++++++++ 9 files changed, 59 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cedfa92d..df96ef35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Added a `--pageHost` CLI option to `bedrock-auto`. This can be useful to set `host.docker.internal` when the browser is in a docker container. #TINYMCE-14879 +- Added a `--progressTimeout` CLI option, defaulting to 5 minutes, controlling how long a browser may go without completing a test while still responding to the server. #TD-2600 ### Fixed - The grunt task passed config values to bedrock without type coercion, so string values for `Number` or `Boolean` options (e.g. `--webdriverPort=14444` or `--useSelenium=false` via `grunt.option`) were rejected or misread. #TINYMCE-14879 - Extended test start pings to every page load, allowing updates during a chunk or fail-retry reload. This should reduce how often the server's no-updates watchdog is tripped. #TINYMCE-14879 - Safari 26 failed to load the test page through the LambdaTest tunnel on `localhost`. The LT tunnel URL now uses LambdaTest's `localhost.lambdatest.com` DNS alias for all browsers. #TINYMCE-14879 +- The server used a single 55 second budget for both "the browser has stopped responding" and "the browser has not finished a test", so a slow remote browser was killed as if the driver had died. Liveness is now tracked separately from progress: silence from the browser is still caught after 55 seconds, while a browser that is responding but slow gets `--progressTimeout`. #TD-2600 +- The runner only sent keep-alive pings in manual mode, so in auto mode the server had nothing but batched test results to judge the browser by. Auto mode now sends them too, and `/tests/alive` uses the throttled driver keep-alive so the extra pings do not issue a webdriver command each time. #TD-2600 ## 17.0.1 - 2026-08-28 diff --git a/modules/runner/src/main/ts/runner/Runner.ts b/modules/runner/src/main/ts/runner/Runner.ts index a8260180..260e740d 100644 --- a/modules/runner/src/main/ts/runner/Runner.ts +++ b/modules/runner/src/main/ts/runner/Runner.ts @@ -78,16 +78,16 @@ export const Runner = (rootSuite: Suite, params: UrlParams, callbacks: Callbacks ui.render(params.offset, numTests, actions.restartTests, retryTest, loadNextTest); return Promise.all([callbacks.sendInit(params.session), callbacks.loadHarness()]).then(([_, harness]) => { - // we don't need a keep-alive timer in auto mode, - if (harness.mode === 'manual') { - // delay this ajax call until after the reporter status elements are in the page - const keepAliveTimer = setInterval(() => { - callbacks.sendKeepAlive(params.session).catch(() => { - // if the server shuts down stop trying to send messages - clearInterval(keepAliveTimer); - }); - }, KEEP_ALIVE_INTERVAL); - } + // Auto mode needs this as much as manual mode does. Results are batched, and a page + // reload posts nothing at all, so without a keep-alive the server cannot tell a browser + // that has died from one that is merely slow, and has to assume the worst. + // delay this ajax call until after the reporter status elements are in the page + const keepAliveTimer = setInterval(() => { + callbacks.sendKeepAlive(params.session).catch(() => { + // if the server shuts down stop trying to send messages + clearInterval(keepAliveTimer); + }); + }, KEEP_ALIVE_INTERVAL); return harness; }); diff --git a/modules/server/src/main/ts/bedrock/cli/ClOptions.ts b/modules/server/src/main/ts/bedrock/cli/ClOptions.ts index 0220e2e2..f20abdbe 100644 --- a/modules/server/src/main/ts/bedrock/cli/ClOptions.ts +++ b/modules/server/src/main/ts/bedrock/cli/ClOptions.ts @@ -179,6 +179,15 @@ export const overallTimeout: ClOption = { uncommon: true }; +export const progressTimeout: ClOption = { + name: 'progressTimeout', + type: Number, + description: 'How long in milliseconds a browser may go without completing a test, while still responding to the server, before bedrock gives up. Silence from the browser itself is always caught after 55 seconds regardless of this setting.', + validate: Extraction.any, + defaultValue: 5 * 60 * 1000, + uncommon: true +}; + export const singleTimeout: ClOption = { name: 'singleTimeout', type: Number, diff --git a/modules/server/src/main/ts/bedrock/cli/Clis.ts b/modules/server/src/main/ts/bedrock/cli/Clis.ts index 9a763dd2..a519354e 100644 --- a/modules/server/src/main/ts/bedrock/cli/Clis.ts +++ b/modules/server/src/main/ts/bedrock/cli/Clis.ts @@ -16,6 +16,7 @@ const commonOptions = (directories: Directories) => { ClOptions.projectdir(directories.current), ClOptions.basedir(directories.bin), ClOptions.overallTimeout, + ClOptions.progressTimeout, ClOptions.singleTimeout, ClOptions.customRoutes, ClOptions.chunk, diff --git a/modules/server/src/main/ts/bedrock/core/Settings.ts b/modules/server/src/main/ts/bedrock/core/Settings.ts index 6185b596..97f7a505 100644 --- a/modules/server/src/main/ts/bedrock/core/Settings.ts +++ b/modules/server/src/main/ts/bedrock/core/Settings.ts @@ -6,6 +6,7 @@ export interface BedrockSettings { readonly chunk: number; readonly loglevel: 'simple' | 'advanced'; readonly overallTimeout: number; + readonly progressTimeout: number; readonly projectdir: string; readonly singleTimeout: number; readonly stopOnFailure: boolean; diff --git a/modules/server/src/main/ts/bedrock/server/Apis.ts b/modules/server/src/main/ts/bedrock/server/Apis.ts index 02ab25d9..bb66caec 100644 --- a/modules/server/src/main/ts/bedrock/server/Apis.ts +++ b/modules/server/src/main/ts/bedrock/server/Apis.ts @@ -45,7 +45,7 @@ const pollRate = 200; const maxInvalidAttempts = 300; // TODO: Do not use files here. -export const create = (master: DriverMaster | null, pMaybeDriver: Promise>, projectdir: string, basedir: string, stickyFirstSession: boolean, overallTimeout: number, testfiles: string[], loglevel: 'simple' | 'advanced', resetMousePosition: boolean): Apis => { +export const create = (master: DriverMaster | null, pMaybeDriver: Promise>, projectdir: string, basedir: string, stickyFirstSession: boolean, overallTimeout: number, progressTimeout: number, testfiles: string[], loglevel: 'simple' | 'advanced', resetMousePosition: boolean): Apis => { let pageHasLoaded = false; let needsMousePositionReset = true; let mousePositionResetSupported = true; @@ -83,7 +83,7 @@ export const create = (master: DriverMaster | null, pMaybeDriver: Promise { @@ -160,7 +160,9 @@ export const create = (master: DriverMaster | null, pMaybeDriver: Promise { c.recordAlive(data.session); - return keepAliveAction(); + // the runner pings every few seconds, so this must be the throttled variant or every + // lane would issue a webdriver command on every ping + return maybeSendKeepAlive(); }), Routes.effect('POST', '/tests/init', () => resetMousePositionAction(true)), Routes.effect('POST', '/tests/start', (data: StartData) => { diff --git a/modules/server/src/main/ts/bedrock/server/Controller.ts b/modules/server/src/main/ts/bedrock/server/Controller.ts index 93faa2e9..f82d2344 100644 --- a/modules/server/src/main/ts/bedrock/server/Controller.ts +++ b/modules/server/src/main/ts/bedrock/server/Controller.ts @@ -51,7 +51,14 @@ export interface Controller { readonly awaitDone: () => Promise; } -export const create = (stickyFirstSession: boolean, overallTimeout: number, testfiles: string[], loglevel: 'simple' | 'advanced'): Controller => { +/* + * How long the browser may go without contacting the server at all before we treat the + * driver as dead. Every route bumps `session.alive`, and the runner also sends a payload-free + * keep-alive, so silence for this long means nothing is getting through. + */ +const LIVENESS_TIMEOUT = 55 * 1000; + +export const create = (stickyFirstSession: boolean, overallTimeout: number, progressTimeout: number, testfiles: string[], loglevel: 'simple' | 'advanced'): Controller => { const hud = Hud.create(testfiles, loglevel); const sessions: Record = {}; let stickyId: string | null = null; @@ -218,9 +225,16 @@ export const create = (stickyFirstSession: boolean, overallTimeout: number, test } clearInterval(poller); } else { - if ((now - session.updated) > (55 * 1000)) { - // sticky sessions have longer before they time out, but we still can't sit around if the driver has failed - const message = `No updates from the browser in ${formatTime(now - session.updated)}, assuming driver has failed.`; + if ((now - session.alive) > LIVENESS_TIMEOUT) { + // nothing at all is reaching us: the driver really has failed + const message = `No updates from the browser in ${formatTime(now - session.alive)}, assuming driver has failed.`; + reject({message, results, start, now}); + clearInterval(poller); + timeoutError = true; + } else if ((now - session.updated) > progressTimeout) { + // the browser is still talking to us, it just isn't finishing tests + const message = `No test progress in ${formatTime(now - session.updated)}, but the browser is still responding. ` + + `Assuming the run is stuck. Limit is set to ${formatTime(progressTimeout)}.`; reject({message, results, start, now}); clearInterval(poller); timeoutError = true; diff --git a/modules/server/src/main/ts/bedrock/server/Serve.ts b/modules/server/src/main/ts/bedrock/server/Serve.ts index 408685a9..dacde61d 100644 --- a/modules/server/src/main/ts/bedrock/server/Serve.ts +++ b/modules/server/src/main/ts/bedrock/server/Serve.ts @@ -20,6 +20,7 @@ export interface ServeSettings { readonly loglevel: 'simple' | 'advanced'; readonly master: DriverMaster | null; readonly overallTimeout: number; + readonly progressTimeout: number; readonly projectdir: string; readonly runner: Promise; readonly skipResetMousePosition: boolean; @@ -65,10 +66,11 @@ export const startCustom = async (settings: ServeSettings, createServer: (port: const master = pref('master'); const stickyFirstSession = settings.stickyFirstSession; const overallTimeout = pref('overallTimeout'); + const progressTimeout = pref('progressTimeout'); const resetMousePosition = !pref('skipResetMousePosition'); const runner = pref('runner'); - const api = Apis.create(master, maybeDriver, projectdir, basedir, stickyFirstSession, overallTimeout, testfiles, settings.loglevel, resetMousePosition); + const api = Apis.create(master, maybeDriver, projectdir, basedir, stickyFirstSession, overallTimeout, progressTimeout, testfiles, settings.loglevel, resetMousePosition); // it is important to not use `await` here so the port opens as fast as possible const serverRoutes = runner.then(async r => ({ diff --git a/modules/server/src/test/ts/ClisTest.ts b/modules/server/src/test/ts/ClisTest.ts index 7a5b0887..d1d56795 100644 --- a/modules/server/src/test/ts/ClisTest.ts +++ b/modules/server/src/test/ts/ClisTest.ts @@ -49,6 +49,7 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, + progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -92,6 +93,7 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, + progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -136,6 +138,7 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, + progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -179,6 +182,7 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, + progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -225,6 +229,7 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, + progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -273,6 +278,7 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, + progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -338,6 +344,7 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, + progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -381,6 +388,7 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, + progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -422,6 +430,7 @@ describe('Clis.forManual', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, + progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, From 1e76517c8660bc1bb8bd8174be17526938d6d252 Mon Sep 17 00:00:00 2001 From: Wojciech Mech Date: Tue, 13 Oct 2026 00:07:20 +0200 Subject: [PATCH 2/8] TD-2668: count client pings instead of timing progress The progress half of the watchdog used `--progressTimeout`, a configurable wall-clock budget. That number cannot be set correctly, because the failure it exists to catch is a connection slowing down, and a constant does not slow down with it. Five minutes was too long for a stuck run and too short for a bad day on the grid, and two reviewers said so independently. Progress is now judged by counting the posts the runner makes. Every post is a ping, only a post carrying results is progress, so a link that halves in speed halves the ping rate and widens the tolerance by the same factor. `session.updated` and `session.done` move inside the `results.length > 0` guard. Without that the heartbeat would feed the very clock it exists to let us read, and the progress watchdog would be silently dead. `--progressTimeout` is removed along with its settings and CLI plumbing, so this change adds no user-facing option. --- .../src/main/ts/bedrock/cli/ClOptions.ts | 9 ----- .../server/src/main/ts/bedrock/cli/Clis.ts | 1 - .../src/main/ts/bedrock/core/Settings.ts | 1 - .../server/src/main/ts/bedrock/server/Apis.ts | 8 ++-- .../src/main/ts/bedrock/server/Controller.ts | 38 ++++++++++++++----- .../src/main/ts/bedrock/server/Serve.ts | 4 +- modules/server/src/test/ts/ClisTest.ts | 9 ----- 7 files changed, 34 insertions(+), 36 deletions(-) diff --git a/modules/server/src/main/ts/bedrock/cli/ClOptions.ts b/modules/server/src/main/ts/bedrock/cli/ClOptions.ts index f20abdbe..0220e2e2 100644 --- a/modules/server/src/main/ts/bedrock/cli/ClOptions.ts +++ b/modules/server/src/main/ts/bedrock/cli/ClOptions.ts @@ -179,15 +179,6 @@ export const overallTimeout: ClOption = { uncommon: true }; -export const progressTimeout: ClOption = { - name: 'progressTimeout', - type: Number, - description: 'How long in milliseconds a browser may go without completing a test, while still responding to the server, before bedrock gives up. Silence from the browser itself is always caught after 55 seconds regardless of this setting.', - validate: Extraction.any, - defaultValue: 5 * 60 * 1000, - uncommon: true -}; - export const singleTimeout: ClOption = { name: 'singleTimeout', type: Number, diff --git a/modules/server/src/main/ts/bedrock/cli/Clis.ts b/modules/server/src/main/ts/bedrock/cli/Clis.ts index a519354e..9a763dd2 100644 --- a/modules/server/src/main/ts/bedrock/cli/Clis.ts +++ b/modules/server/src/main/ts/bedrock/cli/Clis.ts @@ -16,7 +16,6 @@ const commonOptions = (directories: Directories) => { ClOptions.projectdir(directories.current), ClOptions.basedir(directories.bin), ClOptions.overallTimeout, - ClOptions.progressTimeout, ClOptions.singleTimeout, ClOptions.customRoutes, ClOptions.chunk, diff --git a/modules/server/src/main/ts/bedrock/core/Settings.ts b/modules/server/src/main/ts/bedrock/core/Settings.ts index 97f7a505..6185b596 100644 --- a/modules/server/src/main/ts/bedrock/core/Settings.ts +++ b/modules/server/src/main/ts/bedrock/core/Settings.ts @@ -6,7 +6,6 @@ export interface BedrockSettings { readonly chunk: number; readonly loglevel: 'simple' | 'advanced'; readonly overallTimeout: number; - readonly progressTimeout: number; readonly projectdir: string; readonly singleTimeout: number; readonly stopOnFailure: boolean; diff --git a/modules/server/src/main/ts/bedrock/server/Apis.ts b/modules/server/src/main/ts/bedrock/server/Apis.ts index bb66caec..49ceae7d 100644 --- a/modules/server/src/main/ts/bedrock/server/Apis.ts +++ b/modules/server/src/main/ts/bedrock/server/Apis.ts @@ -45,7 +45,7 @@ const pollRate = 200; const maxInvalidAttempts = 300; // TODO: Do not use files here. -export const create = (master: DriverMaster | null, pMaybeDriver: Promise>, projectdir: string, basedir: string, stickyFirstSession: boolean, overallTimeout: number, progressTimeout: number, testfiles: string[], loglevel: 'simple' | 'advanced', resetMousePosition: boolean): Apis => { +export const create = (master: DriverMaster | null, pMaybeDriver: Promise>, projectdir: string, basedir: string, stickyFirstSession: boolean, overallTimeout: number, testfiles: string[], loglevel: 'simple' | 'advanced', resetMousePosition: boolean): Apis => { let pageHasLoaded = false; let needsMousePositionReset = true; let mousePositionResetSupported = true; @@ -83,7 +83,7 @@ export const create = (master: DriverMaster | null, pMaybeDriver: Promise { @@ -160,8 +160,8 @@ export const create = (master: DriverMaster | null, pMaybeDriver: Promise { c.recordAlive(data.session); - // the runner pings every few seconds, so this must be the throttled variant or every - // lane would issue a webdriver command on every ping + // manual mode pings this every few seconds, so it must be the throttled variant or a + // run would issue a webdriver command on every ping return maybeSendKeepAlive(); }), Routes.effect('POST', '/tests/init', () => resetMousePositionAction(true)), diff --git a/modules/server/src/main/ts/bedrock/server/Controller.ts b/modules/server/src/main/ts/bedrock/server/Controller.ts index f82d2344..57f6d197 100644 --- a/modules/server/src/main/ts/bedrock/server/Controller.ts +++ b/modules/server/src/main/ts/bedrock/server/Controller.ts @@ -35,6 +35,8 @@ interface TestSession { readonly lookup: Record>; alive: number; updated: number; + pings: number; + pingsAtLastProgress: number; previous: PreviousTest | null; done: boolean; error?: string; @@ -52,13 +54,21 @@ export interface Controller { } /* - * How long the browser may go without contacting the server at all before we treat the - * driver as dead. Every route bumps `session.alive`, and the runner also sends a payload-free - * keep-alive, so silence for this long means nothing is getting through. + * How long the browser may go without contacting the server at all before we treat the driver + * as dead. Every route bumps `session.alive`, and the runner posts on a fixed heartbeat whether + * or not a test has finished, so silence for this long means nothing is getting through. */ const LIVENESS_TIMEOUT = 55 * 1000; -export const create = (stickyFirstSession: boolean, overallTimeout: number, progressTimeout: number, testfiles: string[], loglevel: 'simple' | 'advanced'): Controller => { +/* + * How many client pings may arrive without a single test finishing before we treat the run as + * stuck. The runner posts on a fixed interval whether or not a test has completed, so this is + * deliberately a count and not a duration: a link that halves in speed stretches the tolerance + * by the same factor instead of tripping it, which a wall-clock budget cannot do. + */ +const STALE_PINGS = 6; + +export const create = (stickyFirstSession: boolean, overallTimeout: number, testfiles: string[], loglevel: 'simple' | 'advanced'): Controller => { const hud = Hud.create(testfiles, loglevel); const sessions: Record = {}; let stickyId: string | null = null; @@ -91,6 +101,8 @@ export const create = (stickyFirstSession: boolean, overallTimeout: number, prog id: sessionId, alive: now, updated: now, + pings: 0, + pingsAtLastProgress: 0, results: [], lookup: {}, previous: null, @@ -129,6 +141,7 @@ export const create = (stickyFirstSession: boolean, overallTimeout: number, prog const session = getSession(id); const now = Date.now(); session.updated = now; + session.pingsAtLastProgress = session.pings; session.totalTests = totalTests; session.currentTest = currentCount; session.done = false; @@ -161,9 +174,14 @@ export const create = (stickyFirstSession: boolean, overallTimeout: number, prog const recordTestResults = (id: string, results: TestResult[]) => { const now = Date.now(); const session = getSession(id); - session.updated = now; - session.done = false; + // Every post counts as a ping, including the empty ones the runner sends on its heartbeat. + // Only a post that actually carries results counts as progress, otherwise the heartbeat + // would keep feeding the very clock it exists to let us read. + session.pings++; if (results.length > 0) { + session.updated = now; + session.done = false; + session.pingsAtLastProgress = session.pings; const {name, file} = results.slice(-1)[0]; session.previous = { name, @@ -186,6 +204,7 @@ export const create = (stickyFirstSession: boolean, overallTimeout: number, prog session.currentTest = session.totalTests; } session.updated = Date.now(); + session.pingsAtLastProgress = session.pings; updateHud(session); }; @@ -231,10 +250,11 @@ export const create = (stickyFirstSession: boolean, overallTimeout: number, prog reject({message, results, start, now}); clearInterval(poller); timeoutError = true; - } else if ((now - session.updated) > progressTimeout) { + } else if ((session.pings - session.pingsAtLastProgress) >= STALE_PINGS) { // the browser is still talking to us, it just isn't finishing tests - const message = `No test progress in ${formatTime(now - session.updated)}, but the browser is still responding. ` + - `Assuming the run is stuck. Limit is set to ${formatTime(progressTimeout)}.`; + const message = `No test progress for ${session.pings - session.pingsAtLastProgress} client pings ` + + `(${formatTime(now - session.updated)}), but the browser is still responding. ` + + `Assuming the run is stuck. Limit is set to ${STALE_PINGS} pings.`; reject({message, results, start, now}); clearInterval(poller); timeoutError = true; diff --git a/modules/server/src/main/ts/bedrock/server/Serve.ts b/modules/server/src/main/ts/bedrock/server/Serve.ts index dacde61d..408685a9 100644 --- a/modules/server/src/main/ts/bedrock/server/Serve.ts +++ b/modules/server/src/main/ts/bedrock/server/Serve.ts @@ -20,7 +20,6 @@ export interface ServeSettings { readonly loglevel: 'simple' | 'advanced'; readonly master: DriverMaster | null; readonly overallTimeout: number; - readonly progressTimeout: number; readonly projectdir: string; readonly runner: Promise; readonly skipResetMousePosition: boolean; @@ -66,11 +65,10 @@ export const startCustom = async (settings: ServeSettings, createServer: (port: const master = pref('master'); const stickyFirstSession = settings.stickyFirstSession; const overallTimeout = pref('overallTimeout'); - const progressTimeout = pref('progressTimeout'); const resetMousePosition = !pref('skipResetMousePosition'); const runner = pref('runner'); - const api = Apis.create(master, maybeDriver, projectdir, basedir, stickyFirstSession, overallTimeout, progressTimeout, testfiles, settings.loglevel, resetMousePosition); + const api = Apis.create(master, maybeDriver, projectdir, basedir, stickyFirstSession, overallTimeout, testfiles, settings.loglevel, resetMousePosition); // it is important to not use `await` here so the port opens as fast as possible const serverRoutes = runner.then(async r => ({ diff --git a/modules/server/src/test/ts/ClisTest.ts b/modules/server/src/test/ts/ClisTest.ts index d1d56795..7a5b0887 100644 --- a/modules/server/src/test/ts/ClisTest.ts +++ b/modules/server/src/test/ts/ClisTest.ts @@ -49,7 +49,6 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, - progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -93,7 +92,6 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, - progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -138,7 +136,6 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, - progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -182,7 +179,6 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, - progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -229,7 +225,6 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, - progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -278,7 +273,6 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, - progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -344,7 +338,6 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, - progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -388,7 +381,6 @@ describe('Clis.forAuto', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, - progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, @@ -430,7 +422,6 @@ describe('Clis.forManual', () => { singleTimeout: 30000, stopOnFailure: false, overallTimeout: 600000, - progressTimeout: 300000, loglevel: 'advanced', version: false, chunk: 2000, From ad33812c909b0352412a5189f1263a50245e29ca Mon Sep 17 00:00:00 2001 From: Wojciech Mech Date: Tue, 13 Oct 2026 00:07:33 +0200 Subject: [PATCH 3/8] TD-2668: post on a heartbeat instead of on test completion The runner only posted when a test finished, and results were batched for up to 30 seconds, so a browser working through one slow test looked exactly like a browser that had died. The server had nothing else to judge it by. It now posts on a fixed interval whether or not anything has finished, and an empty batch still goes out: that empty post is what tells a slow browser from a gone one. The timer reschedules only once the previous post settles, so a slow connection stretches the gap rather than stacking requests on top of one that has not come back. The previous `setInterval` had no such protection, and because the request carries no timeout it could never abort either. This replaces the auto-mode keep-alive rather than adding to it. That channel goes back to manual mode only, so there is one channel on the wire instead of two, at roughly three requests a minute. --- .../runner/src/main/ts/reporter/Reporter.ts | 44 ++++++++++++++++--- modules/runner/src/main/ts/runner/Runner.ts | 24 +++++----- .../src/test/ts/reporter/ReporterTest.ts | 15 +++++++ .../src/test/ts/runner/RunnerTestUtils.ts | 1 + 4 files changed, 68 insertions(+), 16 deletions(-) diff --git a/modules/runner/src/main/ts/reporter/Reporter.ts b/modules/runner/src/main/ts/reporter/Reporter.ts index 219020c0..bb854f4b 100644 --- a/modules/runner/src/main/ts/reporter/Reporter.ts +++ b/modules/runner/src/main/ts/reporter/Reporter.ts @@ -17,6 +17,7 @@ export interface TestReporter { export interface Reporter { readonly summary: () => { offset: number; passed: number; failed: number; skipped: number }; readonly test: (file: string, name: string, totalNumTests: number) => TestReporter; + readonly startHeartbeat: (intervalMs?: number) => void; readonly strayFailure: (file: string, name: string, e: LoggedError) => void; readonly waitForResults: () => Promise; readonly retry: () => Promise; @@ -49,9 +50,16 @@ const mapError = (e: LoggedError) => mapStackTrace(e.stack).then((mappedStack) = return e; }); +/* + * How often the runner posts to the server whether or not a test has finished. The server counts + * these to decide whether a run has stalled, so the value is a cadence and not a budget: slowing + * the connection slows the pings and widens the server's tolerance by the same factor. + */ +const HEARTBEAT_INTERVAL = 20 * 1000; + export const Reporter = (params: UrlParams, callbacks: Callbacks, ui: ReporterUi, mouse: MouseWatch): Reporter => { const initial = Date.now(); - let timeOfLastReport = initial; + let heartbeatTimer: ReturnType | null = null; let currentCount = params.offset || 0; let passCount = 0; let skipCount = 0; @@ -66,8 +74,6 @@ export const Reporter = (params: UrlParams, callbacks: Callbacks, ui: ReporterUi const requestsInFlight: Promise[] = []; const takeResults = (): TestReport[] => { - // Assume the results will be posted once they are returned - timeOfLastReport = Date.now(); // splice deletes the values specified and returns them return testResults.splice(0, testResults.length); }; @@ -80,9 +86,33 @@ export const Reporter = (params: UrlParams, callbacks: Callbacks, ui: ReporterUi const reportResult = (result: TestReport): void => { testResults.push(result); - if (Date.now() - timeOfLastReport > 30 * 1000) { - // ping the server with results every 30 seconds or so, as a form of keep-alive - reportResults(); + }; + + // Posts even when the batch is empty. An empty post is how the server tells a browser that is + // slow from one that is gone, so this must not be skipped when nothing has finished. + const reportProgress = (): Promise => { + const sent = callbacks.sendTestResults(params.session, takeResults()); + requestsInFlight.push(sent); + return sent; + }; + + // Rescheduled only once the previous post settles, so a slow connection stretches the gap + // instead of stacking requests on top of one that has not come back. + const scheduleHeartbeat = (intervalMs: number = HEARTBEAT_INTERVAL): void => { + heartbeatTimer = setTimeout(() => { + const again = (): void => { + if (!finished) { + scheduleHeartbeat(intervalMs); + } + }; + reportProgress().then(again, again); + }, intervalMs); + }; + + const stopHeartbeat = (): void => { + if (heartbeatTimer !== null) { + clearTimeout(heartbeatTimer); + heartbeatTimer = null; } }; @@ -266,6 +296,7 @@ export const Reporter = (params: UrlParams, callbacks: Callbacks, ui: ReporterUi const done = (error?: LoggedError): void => { finished = true; + stopHeartbeat(); const setAsDone = (): void => { const totalTime = elapsed(initial); ui.done(totalTime); @@ -282,6 +313,7 @@ export const Reporter = (params: UrlParams, callbacks: Callbacks, ui: ReporterUi return { summary, test, + startHeartbeat: scheduleHeartbeat, strayFailure, retry, waitForResults, diff --git a/modules/runner/src/main/ts/runner/Runner.ts b/modules/runner/src/main/ts/runner/Runner.ts index 260e740d..aced0262 100644 --- a/modules/runner/src/main/ts/runner/Runner.ts +++ b/modules/runner/src/main/ts/runner/Runner.ts @@ -78,16 +78,20 @@ export const Runner = (rootSuite: Suite, params: UrlParams, callbacks: Callbacks ui.render(params.offset, numTests, actions.restartTests, retryTest, loadNextTest); return Promise.all([callbacks.sendInit(params.session), callbacks.loadHarness()]).then(([_, harness]) => { - // Auto mode needs this as much as manual mode does. Results are batched, and a page - // reload posts nothing at all, so without a keep-alive the server cannot tell a browser - // that has died from one that is merely slow, and has to assume the worst. - // delay this ajax call until after the reporter status elements are in the page - const keepAliveTimer = setInterval(() => { - callbacks.sendKeepAlive(params.session).catch(() => { - // if the server shuts down stop trying to send messages - clearInterval(keepAliveTimer); - }); - }, KEEP_ALIVE_INTERVAL); + // The server counts these to tell a slow browser from a dead one, so it runs in both + // modes and regardless of whether any test is finishing. + reporter.startHeartbeat(); + + // we don't need a keep-alive timer in auto mode, + if (harness.mode === 'manual') { + // delay this ajax call until after the reporter status elements are in the page + const keepAliveTimer = setInterval(() => { + callbacks.sendKeepAlive(params.session).catch(() => { + // if the server shuts down stop trying to send messages + clearInterval(keepAliveTimer); + }); + }, KEEP_ALIVE_INTERVAL); + } return harness; }); diff --git a/modules/runner/src/test/ts/reporter/ReporterTest.ts b/modules/runner/src/test/ts/reporter/ReporterTest.ts index a9986761..aa3650cb 100644 --- a/modules/runner/src/test/ts/reporter/ReporterTest.ts +++ b/modules/runner/src/test/ts/reporter/ReporterTest.ts @@ -52,6 +52,7 @@ describe('Reporter.test', () => { let doneCalled: boolean, doneError: string | undefined, offset: number; let mouseMoved: boolean, startFails: boolean; let startGate: Promise | null; + let resultsPosts: number; const mouse: MouseWatch = { hasMoved: () => mouseMoved, clear: () => { @@ -71,6 +72,7 @@ describe('Reporter.test', () => { return startGate !== null ? startGate : Promise.resolve(); }, sendTestResults: (session, results) => { + resultsPosts++; results.forEach(r => endTestData.push({session, ...r})); return Promise.resolve(); }, @@ -86,6 +88,7 @@ describe('Reporter.test', () => { mouseMoved = false; startFails = false; startGate = null; + resultsPosts = 0; reporter = Reporter({ ...params, offset }, callbacks, ui, mouse); startTestData = []; endTestData = []; @@ -95,6 +98,18 @@ describe('Reporter.test', () => { beforeEach(() => reset()); + it('TD-2668: the heartbeat posts even when no test has finished', () => { + reset(0); + // The server counts these posts to tell a slow browser from a dead one, so an empty batch + // still has to go out. Without that there is nothing to count while one slow test runs. + reporter.startHeartbeat(10); + return new Promise((resolve) => setTimeout(resolve, 60)).then(() => { + assert.isAtLeast(resultsPosts, 2, 'Checking the heartbeat posted repeatedly with nothing to send'); + assert.lengthOf(endTestData, 0, 'Checking the posts carried no results'); + reporter.done(); + }); + }); + it('should report the session id, number tests, file and name on start', () => { return fc.assert(fc.asyncProperty(fc.string(), fc.string(), fc.integer({ max: offset }), (fileName, testName, testCount) => { reset(0); diff --git a/modules/runner/src/test/ts/runner/RunnerTestUtils.ts b/modules/runner/src/test/ts/runner/RunnerTestUtils.ts index ced1ed9f..b80f7667 100644 --- a/modules/runner/src/test/ts/runner/RunnerTestUtils.ts +++ b/modules/runner/src/test/ts/runner/RunnerTestUtils.ts @@ -53,6 +53,7 @@ export const MockReporter = (): MockReporter => { return { test, + startHeartbeat: noop, strayFailure: (_file, name, e) => { failures.push(e); failedNames.push(name); From 545aeba72210600b7bc46320bf292139c33d419e Mon Sep 17 00:00:00 2001 From: Wojciech Mech Date: Tue, 13 Oct 2026 00:07:33 +0200 Subject: [PATCH 4/8] TD-2668: record browser activity on the bundle re-fetch `/tests/init` called no controller method, so the window where a chunk or retry reload re-fetches the test bundle counted as silence against the liveness clock. No JS runs during it, which is exactly when the browser can least afford to look idle. The runner already posts the session id to that route, so recording activity there costs one line. --- CHANGELOG.md | 6 +++--- modules/server/src/main/ts/bedrock/server/Apis.ts | 7 ++++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df96ef35..9e14e647 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,14 +8,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Added a `--pageHost` CLI option to `bedrock-auto`. This can be useful to set `host.docker.internal` when the browser is in a docker container. #TINYMCE-14879 -- Added a `--progressTimeout` CLI option, defaulting to 5 minutes, controlling how long a browser may go without completing a test while still responding to the server. #TD-2600 ### Fixed - The grunt task passed config values to bedrock without type coercion, so string values for `Number` or `Boolean` options (e.g. `--webdriverPort=14444` or `--useSelenium=false` via `grunt.option`) were rejected or misread. #TINYMCE-14879 - Extended test start pings to every page load, allowing updates during a chunk or fail-retry reload. This should reduce how often the server's no-updates watchdog is tripped. #TINYMCE-14879 - Safari 26 failed to load the test page through the LambdaTest tunnel on `localhost`. The LT tunnel URL now uses LambdaTest's `localhost.lambdatest.com` DNS alias for all browsers. #TINYMCE-14879 -- The server used a single 55 second budget for both "the browser has stopped responding" and "the browser has not finished a test", so a slow remote browser was killed as if the driver had died. Liveness is now tracked separately from progress: silence from the browser is still caught after 55 seconds, while a browser that is responding but slow gets `--progressTimeout`. #TD-2600 -- The runner only sent keep-alive pings in manual mode, so in auto mode the server had nothing but batched test results to judge the browser by. Auto mode now sends them too, and `/tests/alive` uses the throttled driver keep-alive so the extra pings do not issue a webdriver command each time. #TD-2600 +- The server used a single 55 second budget for both "the browser has stopped responding" and "the browser has not finished a test", so a slow remote browser was killed as if the driver had died. Liveness is now tracked separately from progress: silence from the browser is still caught after 55 seconds, while lack of progress is measured in client pings rather than elapsed time, so a slower connection widens the tolerance by the same factor. #TD-2600 +- The runner only posted to the server when a test finished, so a browser running one slow test looked the same as a browser that had died. It now posts on a fixed heartbeat regardless of test activity, rescheduled after each post completes so a slow connection cannot stack requests. #TD-2600 +- `/tests/init` did not record browser activity, so the bundle re-fetch after a chunk or retry reload counted as silence. #TD-2600 ## 17.0.1 - 2026-08-28 diff --git a/modules/server/src/main/ts/bedrock/server/Apis.ts b/modules/server/src/main/ts/bedrock/server/Apis.ts index 49ceae7d..f5dcbec5 100644 --- a/modules/server/src/main/ts/bedrock/server/Apis.ts +++ b/modules/server/src/main/ts/bedrock/server/Apis.ts @@ -164,7 +164,12 @@ export const create = (master: DriverMaster | null, pMaybeDriver: Promise resetMousePositionAction(true)), + // The bundle re-fetch after a chunk or retry reload runs no JS, so without this the whole + // reload window looks like silence to the liveness clock. + Routes.effect('POST', '/tests/init', (data: { session: string }) => { + c.recordAlive(data.session); + return resetMousePositionAction(true); + }), Routes.effect('POST', '/tests/start', (data: StartData) => { c.recordTestResults(data.session, data.results); c.recordTestStart(data.session, data.name, data.file, data.number, data.totalTests); From 07435b5cef296e3b01b93adbd95ccc914f051a68 Mon Sep 17 00:00:00 2001 From: Wojciech Mech Date: Mon, 19 Oct 2026 07:40:20 +0200 Subject: [PATCH 5/8] TD-2668: start the heartbeat only in auto mode --- modules/runner/src/main/ts/runner/Runner.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/modules/runner/src/main/ts/runner/Runner.ts b/modules/runner/src/main/ts/runner/Runner.ts index aced0262..0e7ef327 100644 --- a/modules/runner/src/main/ts/runner/Runner.ts +++ b/modules/runner/src/main/ts/runner/Runner.ts @@ -78,11 +78,6 @@ export const Runner = (rootSuite: Suite, params: UrlParams, callbacks: Callbacks ui.render(params.offset, numTests, actions.restartTests, retryTest, loadNextTest); return Promise.all([callbacks.sendInit(params.session), callbacks.loadHarness()]).then(([_, harness]) => { - // The server counts these to tell a slow browser from a dead one, so it runs in both - // modes and regardless of whether any test is finishing. - reporter.startHeartbeat(); - - // we don't need a keep-alive timer in auto mode, if (harness.mode === 'manual') { // delay this ajax call until after the reporter status elements are in the page const keepAliveTimer = setInterval(() => { @@ -91,6 +86,9 @@ export const Runner = (rootSuite: Suite, params: UrlParams, callbacks: Callbacks clearInterval(keepAliveTimer); }); }, KEEP_ALIVE_INTERVAL); + } else { + // only auto mode has the watchdog that counts these pings; manual keeps its keep-alive + reporter.startHeartbeat(); } return harness; From 55c61b1fa5ccabcf779bb27ee327d2997ef16980 Mon Sep 17 00:00:00 2001 From: Wojciech Mech Date: Mon, 19 Oct 2026 07:40:21 +0200 Subject: [PATCH 6/8] TD-2668: drop a comment the review called unnecessary --- modules/server/src/main/ts/bedrock/server/Apis.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/modules/server/src/main/ts/bedrock/server/Apis.ts b/modules/server/src/main/ts/bedrock/server/Apis.ts index f5dcbec5..765924e6 100644 --- a/modules/server/src/main/ts/bedrock/server/Apis.ts +++ b/modules/server/src/main/ts/bedrock/server/Apis.ts @@ -164,8 +164,6 @@ export const create = (master: DriverMaster | null, pMaybeDriver: Promise { c.recordAlive(data.session); return resetMousePositionAction(true); From 61e5ba61b187c986e9ea3c1e6df6e1b78b21d252 Mon Sep 17 00:00:00 2001 From: Wojciech Mech Date: Mon, 19 Oct 2026 07:40:21 +0200 Subject: [PATCH 7/8] TD-2668: lower the stale-ping budget to four pings --- modules/server/src/main/ts/bedrock/server/Controller.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/server/src/main/ts/bedrock/server/Controller.ts b/modules/server/src/main/ts/bedrock/server/Controller.ts index 57f6d197..5182c277 100644 --- a/modules/server/src/main/ts/bedrock/server/Controller.ts +++ b/modules/server/src/main/ts/bedrock/server/Controller.ts @@ -64,9 +64,11 @@ const LIVENESS_TIMEOUT = 55 * 1000; * How many client pings may arrive without a single test finishing before we treat the run as * stuck. The runner posts on a fixed interval whether or not a test has completed, so this is * deliberately a count and not a duration: a link that halves in speed stretches the tolerance - * by the same factor instead of tripping it, which a wall-clock budget cannot do. + * by the same factor instead of tripping it, which a wall-clock budget cannot do. The count times + * the heartbeat interval must stay above the longest single-test timeout in use, or one + * slow-but-legal test would be mistaken for a stuck run. */ -const STALE_PINGS = 6; +const STALE_PINGS = 4; export const create = (stickyFirstSession: boolean, overallTimeout: number, testfiles: string[], loglevel: 'simple' | 'advanced'): Controller => { const hud = Hud.create(testfiles, loglevel); From cf5467b00a3db9292eaadf6149a13f64a8790084 Mon Sep 17 00:00:00 2001 From: Wojciech Mech Date: Mon, 19 Oct 2026 07:40:21 +0200 Subject: [PATCH 8/8] TD-2668: fold the watchdog reject branches into one helper --- .../src/main/ts/bedrock/server/Controller.ts | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/modules/server/src/main/ts/bedrock/server/Controller.ts b/modules/server/src/main/ts/bedrock/server/Controller.ts index 5182c277..6e74f30c 100644 --- a/modules/server/src/main/ts/bedrock/server/Controller.ts +++ b/modules/server/src/main/ts/bedrock/server/Controller.ts @@ -231,6 +231,11 @@ export const create = (stickyFirstSession: boolean, overallTimeout: number, test return Promise.reject({message, results, start, now}); } return new Promise((resolve, reject) => { + const failRun = (message: string, results: TestResult[], now: number) => { + reject({message, results, start, now}); + clearInterval(poller); + timeoutError = true; + }; const poller = setInterval(() => { const now = Date.now(); const allElapsed = now - start; @@ -248,18 +253,12 @@ export const create = (stickyFirstSession: boolean, overallTimeout: number, test } else { if ((now - session.alive) > LIVENESS_TIMEOUT) { // nothing at all is reaching us: the driver really has failed - const message = `No updates from the browser in ${formatTime(now - session.alive)}, assuming driver has failed.`; - reject({message, results, start, now}); - clearInterval(poller); - timeoutError = true; + failRun(`No updates from the browser in ${formatTime(now - session.alive)}, assuming driver has failed.`, results, now); } else if ((session.pings - session.pingsAtLastProgress) >= STALE_PINGS) { // the browser is still talking to us, it just isn't finishing tests - const message = `No test progress for ${session.pings - session.pingsAtLastProgress} client pings ` + + failRun(`No test progress for ${session.pings - session.pingsAtLastProgress} client pings ` + `(${formatTime(now - session.updated)}), but the browser is still responding. ` + - `Assuming the run is stuck. Limit is set to ${STALE_PINGS} pings.`; - reject({message, results, start, now}); - clearInterval(poller); - timeoutError = true; + `Assuming the run is stuck. Limit is set to ${STALE_PINGS} pings.`, results, now); } else if (allElapsed > overallTimeout) { // combined tests took too long let lastTest; @@ -276,18 +275,12 @@ export const create = (stickyFirstSession: boolean, overallTimeout: number, test const estimatedTotal = Math.ceil(((allElapsed / session.results.length) * testfiles.length) / 1000) * 1000; const message = 'Tests timed out: ' + formatTime(allElapsed) + '. Limit is set to ' + formatTime(overallTimeout) + '. Estimated required time ' + formatTime(estimatedTotal) + '.\n' + lastTest + '\nTop 10 longest running tests:\n' + longest; - reject({message, results, start, now}); - clearInterval(poller); - timeoutError = true; + failRun(message, results, now); } } } else if (allElapsed > overallTimeout) { // combined tests took too long - const message = 'Tests took too long to start'; - const results: TestResult[] = []; - reject({message, results, start, now}); - clearInterval(poller); - timeoutError = true; + failRun('Tests took too long to start', [], now); } }, 1000); });