From d327d70d040080b6025d3b004365f8fa4007806b Mon Sep 17 00:00:00 2001 From: kn Date: Thu, 22 Oct 2026 10:30:41 +0200 Subject: [PATCH 1/2] Keep at most one fallback frontend poll in flight When the event stream is unavailable, the client polls /api/frontend every 600 ms. Each tick sent a request whether or not the previous one had returned, so a server slower than the interval got an ever growing number of overlapping requests: more load on a server that is already behind. In a browser against a server answering polls after 3 s, five polls were in flight at once and the queued ones ran into the 10 s request timeout, because they also took the browser's connections. The reply serials only dropped out-of-order results; the requests still went out. A tick is now skipped while a request sent by retrieveFrontendState is pending (frontendRequestsInFlight), so a poll tick never adds a second request and the effective interval stretches to the server's response time. The counter is released however the request ends; the default 10 s timeout of axiosGetRequest bounds how long a hung request holds polling back. Navigation refreshes are not held back: the new page's values are needed at once, and the existing serials keep an older poll reply from overwriting them. A poll still pending when the stream takes over is dropped as before but keeps the gate until it settles, also if the client falls back to polling again in the meantime. Each tick also calls updateModules, which is what refreshes module data such as parameter lists and logs while polling. A skipped tick skips that too, so with a slow /api/frontend those modules are refreshed once per reply rather than every 600 ms, even if their own endpoints are fast. A server that slow is likely slow for them as well. The other periodic client requests (parameter list, logs, configuration list, part list, execution list deltas) already allow one request at a time. The change is in Client/core, so it applies to both the Vue and the Svelte client. --- Client/core/common/AMCApplication.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Client/core/common/AMCApplication.js b/Client/core/common/AMCApplication.js index 618b1dac..745f4b36 100644 --- a/Client/core/common/AMCApplication.js +++ b/Client/core/common/AMCApplication.js @@ -123,6 +123,8 @@ export default class AMCApplication extends Common.AMCObject { frontendScope: "", frontendRequestSerial: 0, frontendAppliedSerial: 0, + // /api/frontend requests sent by retrieveFrontendState that have not settled yet. + frontendRequestsInFlight: 0, navigationRequestSerial: 0, frontendMetrics: this._createEmptyFrontendMetrics (), userUUID: Common.nullUUID (), @@ -966,7 +968,13 @@ export default class AMCApplication extends Common.AMCObject { if (this.API.frontendState && (this.API.frontendRevision > 0) && (this.API.frontendScope === requestScope)) requestParams.since = this.API.frontendRevision; + // Counted for the fallback polling (see _startFrontendPolling) and released however the request + // ends; axiosGetRequest's default timeout bounds how long a hung request can hold polling back. + this.API.frontendRequestsInFlight++; return this.axiosGetRequest("/frontend", { params: requestParams }) + .finally(() => { + this.API.frontendRequestsInFlight--; + }) .then(resultJSON => { this._recordFrontendMetric (requestStartMS, resultJSON); @@ -1665,7 +1673,15 @@ export default class AMCApplication extends Common.AMCObject { if (updates.pollTimer) return; + // A tick never adds a second request: it is skipped while any /api/frontend request sent by + // retrieveFrontendState is pending (an earlier poll, a navigation refresh, or a poll still running + // from before the stream took over), so a server slower than the interval does not get an ever + // growing number of overlapping polls. Navigation refreshes themselves are not held back; the + // reply serials keep their results in order. A skipped tick also skips updateModules, so while + // polling, module data is refreshed once per reply. updates.pollTimer = setInterval (() => { + if (this.API.frontendRequestsInFlight > 0) + return; this.retrieveFrontendState () .finally (() => { this.updateModules (); From cc99834c0d2d9e090e15b9ab34a466c23a7b8d6c Mon Sep 17 00:00:00 2001 From: kn Date: Thu, 22 Oct 2026 10:30:41 +0200 Subject: [PATCH 2/2] Add node:test tests for the fallback frontend poll gate Client/core is framework-agnostic ES module code, so it can be tested under plain node without a browser or the client's npm packages. Client/tests is a small harness for that: support/coreLoader.mjs imports core modules through loader hooks (support/coreLoaderHooks.mjs) that load core's .js files as ES modules and replace the third-party imports (axios, luxon, three) with stubs. There are no dependencies; run "npm test" in Client/tests, or "node --test 'Client/tests/*.test.mjs'" from the repository root. frontendPollGate.test.mjs covers the gate from the previous commit: its release on reply, error and timeout, a server slower than the interval, a navigation refresh during a pending poll, and switching to the stream and back. Loading AMCApplication.js imports the list modules, which need Client/core/common/AMCListDeltaSync.js, so the tests require that file. --- Client/tests/frontendPollGate.test.mjs | 213 +++++++++++++++++++++++ Client/tests/package.json | 9 + Client/tests/support/coreLoader.mjs | 11 ++ Client/tests/support/coreLoaderHooks.mjs | 37 ++++ 4 files changed, 270 insertions(+) create mode 100644 Client/tests/frontendPollGate.test.mjs create mode 100644 Client/tests/package.json create mode 100644 Client/tests/support/coreLoader.mjs create mode 100644 Client/tests/support/coreLoaderHooks.mjs diff --git a/Client/tests/frontendPollGate.test.mjs b/Client/tests/frontendPollGate.test.mjs new file mode 100644 index 00000000..9589cb09 --- /dev/null +++ b/Client/tests/frontendPollGate.test.mjs @@ -0,0 +1,213 @@ +// Run with: cd Client/tests && npm test (or from the repository root: node --test 'Client/tests/*.test.mjs') +// +// The fallback /api/frontend polling, used when the event stream is unavailable, never adds a second +// request: a tick is skipped while a request is pending, and the gate opens again however the request +// ends (reply, error, timeout). + +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { importCore } from "./support/coreLoader.mjs"; + +const AMCApplication = await importCore ("common/AMCApplication.js"); +const AMCApplicationPage = await importCore ("common/AMCPage.js"); + +const POLL_INTERVAL_MS = 600; +const MAIN_PAGE_UUID = "00000000-0000-4000-8000-000000000001"; +const OTHER_PAGE_UUID = "00000000-0000-4000-8000-000000000002"; + +function frontendJSON (revision) { + return { + protocol: "amcf-frontend", version: 2, revision: revision, + menuitems: [], toolbaritems: [], custompages: [], dialogs: [], + pages: [ + { name: "main", uuid: MAIN_PAGE_UUID, visible: true, modules: [] }, + { name: "other", uuid: OTHER_PAGE_UUID, visible: true, modules: [] } + ] + }; +} + +function flushPromises () { + return new Promise ((resolve) => setImmediate (resolve)); +} + +// A logged-in application on page "main" with the HTTP boundary stubbed. By default GET requests stay +// pending until the test settles them; with serverDelayMS they are answered after that (mocked) delay. +function createApp (t, serverDelayMS) { + t.mock.timers.enable ({ apis: ["setInterval", "setTimeout"] }); + + const app = new AMCApplication ("http://127.0.0.1:1/api", null); + app.API.authToken = "1".repeat (64); + + const server = { gets: [], inFlight: 0, maxInFlight: 0, revision: 1 }; + app.axiosGetRequest = (url, config) => new Promise ((resolve, reject) => { + const request = { url, config, resolve, reject }; + server.gets.push (request); + server.inFlight++; + server.maxInFlight = Math.max (server.maxInFlight, server.inFlight); + request.settle = (fn, value) => { server.inFlight--; fn (value); }; + if (serverDelayMS !== undefined) + setTimeout (() => request.settle (resolve, { data: frontendJSON (++server.revision) }), serverDelayMS); + }); + app.axiosPostRequest = () => Promise.resolve ({ data: {} }); + + for (const pageJSON of frontendJSON (1).pages) { + const page = new AMCApplicationPage (app, pageJSON); + app.AppContent.Pages.push (page); + app.AppContent.PageMap.set (page.name, page); + } + app.AppState.activePage = "main"; + app._setFrontendState (frontendJSON (1), "main|"); + + app.FrontendUpdates.enabled = true; + app._startFrontendPolling (); + t.after (() => app.stopFrontendUpdates ()); + + return { app, server }; +} + +function polls (server) { + return server.gets.filter (request => request.url === "/frontend" && request.config && request.config.params); +} + +// Advances the mocked timers in small steps, letting the request promises settle in between. +async function advance (t, ms) { + const stepMS = 100; + for (let elapsedMS = 0; elapsedMS < ms; elapsedMS += stepMS) { + t.mock.timers.tick (stepMS); + await flushPromises (); + } +} + +test ("a poll tick is skipped while the previous poll is pending", async (t) => { + const { app, server } = createApp (t); + + await advance (t, POLL_INTERVAL_MS); + assert.equal (polls (server).length, 1); + assert.equal (app.API.frontendRequestsInFlight, 1); + + await advance (t, POLL_INTERVAL_MS * 10); + assert.equal (polls (server).length, 1, "no further poll while the first one is pending"); +}); + +test ("the gate opens after a reply", async (t) => { + const { app, server } = createApp (t); + + await advance (t, POLL_INTERVAL_MS); + polls (server)[0].settle (polls (server)[0].resolve, { data: frontendJSON (2) }); + await flushPromises (); + assert.equal (app.API.frontendRequestsInFlight, 0); + assert.equal (app.API.frontendRevision, 2); + + await advance (t, POLL_INTERVAL_MS); + assert.equal (polls (server).length, 2); +}); + +test ("the gate opens after a failed request", async (t) => { + const { app, server } = createApp (t); + + await advance (t, POLL_INTERVAL_MS); + const error = new Error ("Request failed with status code 500"); + error.response = { status: 500, data: { message: "internal error" } }; + polls (server)[0].settle (polls (server)[0].reject, error); + await flushPromises (); + assert.equal (app.API.frontendRequestsInFlight, 0); + assert.equal (app.API.unsuccessfulFrontendCounter, 1); + + await advance (t, POLL_INTERVAL_MS); + assert.equal (polls (server).length, 2); +}); + +test ("the gate opens when a poll times out", async (t) => { + const { app, server } = createApp (t); + + await advance (t, POLL_INTERVAL_MS); + + // axios rejects a request that exceeds its timeout (axiosGetRequest's default) with ECONNABORTED. + const error = new Error ("timeout of 10000ms exceeded"); + error.code = "ECONNABORTED"; + polls (server)[0].settle (polls (server)[0].reject, error); + await flushPromises (); + assert.equal (app.API.frontendRequestsInFlight, 0); + + await advance (t, POLL_INTERVAL_MS); + assert.equal (polls (server).length, 2); +}); + +test ("requests never overlap against a server slower than the poll interval", async (t) => { + const { app, server } = createApp (t, 1500); + + await advance (t, 20000); + assert.equal (server.maxInFlight, 1); + // One request per 1500 ms reply plus up to one interval of waiting for the next tick. + assert.ok (polls (server).length >= 9 && polls (server).length <= 14, "polls: " + polls (server).length); + // Every reply was applied, in order. + assert.equal (app.API.frontendRevision, server.revision); + assert.equal (app.API.frontendAppliedSerial, app.API.frontendRequestSerial - server.inFlight); +}); + +test ("a navigation refresh is sent at once and holds back the polling until it settles", async (t) => { + const { app, server } = createApp (t); + + await advance (t, POLL_INTERVAL_MS); + const poll = polls (server)[0]; + + app.changePage ("other"); + assert.equal (polls (server).length, 2, "the new scope is requested without waiting for the pending poll"); + const navigation = polls (server)[1]; + assert.equal (navigation.config.params.pages, "other"); + + navigation.settle (navigation.resolve, { data: frontendJSON (3) }); + await flushPromises (); + await advance (t, POLL_INTERVAL_MS * 3); + assert.equal (polls (server).length, 2, "the old poll is still pending"); + + // The older poll's reply arrives last and must not replace the newer state. + poll.settle (poll.resolve, { data: frontendJSON (2) }); + await flushPromises (); + assert.equal (app.API.frontendRevision, 3); + assert.equal (app.API.frontendScope, "other|"); + assert.equal (app.API.frontendRequestsInFlight, 0); + + await advance (t, POLL_INTERVAL_MS); + assert.equal (polls (server).length, 3); + assert.equal (polls (server)[2].config.params.since, 3); +}); + +test ("a poll pending across a switch to the stream and back keeps the gate until it settles", async (t) => { + const { app, server } = createApp (t); + + await advance (t, POLL_INTERVAL_MS); + const poll = polls (server)[0]; + + // The stream takes over: polling stops, and its structure load is not a poll. + app._enterFrontendStreamMode (); + assert.equal (app.FrontendUpdates.mode, "stream"); + assert.equal (app.FrontendUpdates.pollTimer, null); + + // The stream fails again before the poll has settled. + app._startFrontendPolling (); + await advance (t, POLL_INTERVAL_MS * 3); + assert.equal (polls (server).length, 1, "still gated by the poll sent before the switch"); + + poll.settle (poll.reject, Object.assign (new Error ("timeout"), { code: "ECONNABORTED" })); + await flushPromises (); + assert.equal (app.API.frontendRequestsInFlight, 0); + + await advance (t, POLL_INTERVAL_MS); + assert.equal (polls (server).length, 2); +}); + +test ("a poll reply arriving in stream mode is dropped and releases the gate", async (t) => { + const { app, server } = createApp (t); + + await advance (t, POLL_INTERVAL_MS); + const poll = polls (server)[0]; + app._stopFrontendPolling (); + app.FrontendUpdates.mode = "stream"; + + poll.settle (poll.resolve, { data: frontendJSON (5) }); + await flushPromises (); + assert.equal (app.API.frontendRevision, 1, "a scoped poll reply must not replace the stream's state"); + assert.equal (app.API.frontendRequestsInFlight, 0); +}); diff --git a/Client/tests/package.json b/Client/tests/package.json new file mode 100644 index 00000000..371361ec --- /dev/null +++ b/Client/tests/package.json @@ -0,0 +1,9 @@ +{ + "name": "amc-client-tests", + "version": "0.1.0", + "private": true, + "description": "node:test tests for the framework-agnostic client code in Client/core. No dependencies: third-party imports are stubbed (support/coreLoaderHooks.mjs).", + "scripts": { + "test": "node --test" + } +} diff --git a/Client/tests/support/coreLoader.mjs b/Client/tests/support/coreLoader.mjs new file mode 100644 index 00000000..f18aa29a --- /dev/null +++ b/Client/tests/support/coreLoader.mjs @@ -0,0 +1,11 @@ +// Imports Client/core modules for tests. The loader hooks must be registered before any core module +// is linked, so tests import core code through importCore (a dynamic import) rather than statically. + +import { register } from "node:module"; + +register ("./coreLoaderHooks.mjs", import.meta.url); + +export async function importCore (pathInCore) { + const module = await import (new URL ("../../core/" + pathInCore, import.meta.url).href); + return module.default; +} diff --git a/Client/tests/support/coreLoaderHooks.mjs b/Client/tests/support/coreLoaderHooks.mjs new file mode 100644 index 00000000..982185b9 --- /dev/null +++ b/Client/tests/support/coreLoaderHooks.mjs @@ -0,0 +1,37 @@ +// Module loader hooks for running Client/core under node:test without installing the client's +// npm packages. Registered by coreLoader.mjs. +// +// - Client/core is plain ES module code in .js files without a package.json "type", so its files +// are loaded as ES modules explicitly (no reliance on Node's syntax detection). +// - The few third-party packages Client/core imports (axios, luxon, three) are replaced by stubs. +// The tests never reach the network or render anything; requests are stubbed on the application. + +import { readFile } from "node:fs/promises"; + +const STUB_PREFIX = "amc-test-stub:"; + +const STUBS = { + "axios": "export default function axios () { throw new Error ('network access is stubbed out in tests'); }", + "luxon": "export const DateTime = {};", + "three": "export default {};", + "three/examples/jsm/loaders/SVGLoader": "export class SVGLoader {}", + "three/addons/controls/OrbitControls.js": "export class OrbitControls {}", +}; + +const CORE_URL_PART = "/Client/core/"; + +export async function resolve (specifier, context, nextResolve) { + if (Object.prototype.hasOwnProperty.call (STUBS, specifier)) + return { url: STUB_PREFIX + specifier, shortCircuit: true }; + return nextResolve (specifier, context); +} + +export async function load (url, context, nextLoad) { + if (url.startsWith (STUB_PREFIX)) + return { format: "module", source: STUBS[url.substring (STUB_PREFIX.length)], shortCircuit: true }; + + if (url.startsWith ("file:") && url.includes (CORE_URL_PART) && url.endsWith (".js")) + return { format: "module", source: await readFile (new URL (url), "utf8"), shortCircuit: true }; + + return nextLoad (url, context); +}