From 4dcd6f697ec237251727aa4bed6750140e0a5469 Mon Sep 17 00:00:00 2001 From: SandObserver <260779319+SandObserver@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:10:48 -0400 Subject: [PATCH] Bound the batched polls separately and show a failed badge as stale /api/badges and /api/health answer only once every item has settled, so one dead service added its full 6s timeout to every tile's refresh. Give the batched fan-outs their own 2.5s deadline and keep PING_MS for a single connection test a user waits on. The badges route already reports a per-item error, which the dashboard discarded, painting a service that never answered as no badge at all. Keep the last known value and mark it out of date instead. --- CHANGELOG.md | 6 ++ api/src/routes/badges.js | 4 +- api/src/routes/health.js | 4 +- api/src/timeouts.js | 4 ++ api/test/badge-batch-timeout.test.js | 86 ++++++++++++++++++++++++++++ docs/frontend.md | 4 +- ui/js/badge-logic.js | 22 ++++++- ui/js/dashboard.js | 11 +++- ui/test/badge-logic.test.mjs | 46 ++++++++++++++- 9 files changed, 177 insertions(+), 10 deletions(-) create mode 100644 api/test/badge-batch-timeout.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cac7ff8..0916f31b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- One unreachable service no longer delays every other tile's refresh by six + seconds. + +- A badge whose service did not answer keeps its last value and is marked out of + date, instead of showing nothing. + - Icons from the community set are fetched once through Stackyard and kept in memory, instead of being loaded from a public CDN on every page load. diff --git a/api/src/routes/badges.js b/api/src/routes/badges.js index 4a0b0ce5..6d686e41 100644 --- a/api/src/routes/badges.js +++ b/api/src/routes/badges.js @@ -3,7 +3,7 @@ const { loadConfig } = require('../config'); const { fetchChecked, fetchUnchecked, pingChecked, statusDesc } = require('../proxy'); const { rateLimit } = require('../auth'); const LIMITS = require('../poll-limits'); -const { PING_MS, FETCH_MS } = require('../timeouts'); +const { PING_MS, FETCH_MS, BATCH_MS } = require('../timeouts'); const { IS_DEMO } = require('../demo'); const demoData = require('../demo-data'); const { collectNumbers, computeBadgeValue } = require('../badge-extract'); @@ -59,7 +59,7 @@ on('GET', '/api/badges', async (req, res) => { const url = Object.keys(params).length ? baseUrl + (baseUrl.includes('?') ? '&' : '?') + new URLSearchParams(params) : baseUrl; - const r = await fetchUnchecked(url, { headers, timeout: PING_MS, skipTls: item.skipTlsVerify === true }); + const r = await fetchUnchecked(url, { headers, timeout: BATCH_MS, skipTls: item.skipTlsVerify === true }); const badge = item.monitoring?.activity?.enabled ? { extract: item.monitoring.activity.extract, diff --git a/api/src/routes/health.js b/api/src/routes/health.js index 999732b1..78355ba7 100644 --- a/api/src/routes/health.js +++ b/api/src/routes/health.js @@ -13,7 +13,7 @@ const { isDockerServiceName, isPrivateAddress, } = require('../proxy'); -const { PING_MS } = require('../timeouts'); +const { PING_MS, BATCH_MS } = require('../timeouts'); const { IS_DEMO } = require('../demo'); const demoData = require('../demo-data'); const log = require('../log'); @@ -154,7 +154,7 @@ on('GET', '/api/health', async (req, res) => { detail.status = c?.status || ''; } if (ping) { - const r = await pingUnchecked(ping, PING_MS, item.skipTlsVerify === true); + const r = await pingUnchecked(ping, BATCH_MS, item.skipTlsVerify === true); if (!r.ok) unhealthy = true; detail.pingStatus = r.status; detail.pingError = r.error; diff --git a/api/src/timeouts.js b/api/src/timeouts.js index 3dbe3bee..76533fcc 100644 --- a/api/src/timeouts.js +++ b/api/src/timeouts.js @@ -1,4 +1,8 @@ module.exports = { PING_MS: 6000, FETCH_MS: 8000, + /* The batched dashboard polls answer only when every item has settled, so one + dead service delays every tile by this much. Shorter than PING_MS, which + bounds a single connection test a user is waiting on. */ + BATCH_MS: 2500, }; diff --git a/api/test/badge-batch-timeout.test.js b/api/test/badge-batch-timeout.test.js new file mode 100644 index 00000000..707ad517 --- /dev/null +++ b/api/test/badge-batch-timeout.test.js @@ -0,0 +1,86 @@ +/* /api/badges answers only when every item has settled, so the batch is as slow + as its slowest item. It used the same 6s deadline as a connection test a user + runs by hand and waits for, which let one dead service hold up every other + tile's refresh on every poll. */ + +const path = require('node:path'); + +process.env.ALLOW_PRIVATE_IPS = 'true'; +const { tmpDir } = require('../test-support/tmp'); +process.env.CONFIG_PATH = path.join(tmpDir('badge-batch'), 'apps.json'); + +const { test, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +const http = require('node:http'); + +require('../src/routes'); +const { dispatch } = require('../src/router'); +const { saveConfig } = require('../src/config'); +const { PING_MS, BATCH_MS } = require('../src/timeouts'); + +let server, base, fast, fastBase, slow, slowBase; +const open = new Set(); + +const listen = s => new Promise(r => s.listen(0, '127.0.0.1', () => r(`http://127.0.0.1:${s.address().port}`))); +const close = s => + new Promise(r => { + s.closeAllConnections?.(); + s.close(r); + }); + +before(async () => { + fast = http.createServer((_, res) => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ pending: 5 })); + }); + fastBase = await listen(fast); + /* Accepts the connection and never answers, which is what a hung service + looks like. A refused port would return immediately instead. */ + slow = http.createServer(req => open.add(req)); + slowBase = await listen(slow); + server = http.createServer(dispatch); + base = await listen(server); +}); +after(async () => { + await close(server); + await close(fast); + await close(slow); +}); + +function get(pathname) { + const u = new URL(base + pathname); + return new Promise((resolve, reject) => { + http + .request({ hostname: u.hostname, port: u.port, path: u.pathname, method: 'GET' }, res => { + let b = ''; + res.on('data', c => { + b += c; + }); + res.on('end', () => resolve(JSON.parse(b))); + }) + .on('error', reject) + .end(); + }); +} + +test('the batch deadline is shorter than a hand-run connection test', () => { + assert.ok(BATCH_MS < PING_MS, `${BATCH_MS} should be under ${PING_MS}`); +}); + +test('one hung service does not add its full timeout to the rest of the batch', async () => { + saveConfig({ + items: [ + { id: 'good', type: 'app', name: 'Good', badge: { enabled: true, url: `${fastBase}/x`, extract: 'pending' } }, + { id: 'hung', type: 'app', name: 'Hung', badge: { enabled: true, url: `${slowBase}/x`, extract: 'pending' } }, + ], + settings: {}, + }); + + const started = Date.now(); + const body = await get('/api/badges'); + const elapsed = Date.now() - started; + + assert.equal(body.good.value, 5, 'the reachable item still reports its value'); + assert.ok(body.hung.error, 'the hung item reports a failure'); + assert.ok(elapsed < PING_MS, `the batch took ${elapsed}ms, which is the old deadline or worse`); +}); diff --git a/docs/frontend.md b/docs/frontend.md index 3bf8cb93..2ccab4e8 100644 --- a/docs/frontend.md +++ b/docs/frontend.md @@ -30,7 +30,9 @@ See [widgets.md](./widgets.md). `dashboard.js` polls `/api/badges` and `/api/health` and paints tiles through an id-to-elements registry. Appearance is one pure function, `computeBadgeVisual` in `badge-logic.js`. -Each of these is a single batch request. The server fetches every configured badge or health target concurrently, each bounded by `PING_MS`, and returns one combined object once all of them settle. A slow or unreachable upstream delays the refresh of the other tiles by up to `PING_MS`. +Each of these is a single batch request. The server fetches every configured badge or health target concurrently, each bounded by `BATCH_MS`, and returns one combined object once all of them settle. A slow or unreachable upstream delays the refresh of the other tiles by up to `BATCH_MS`. `PING_MS` is longer and bounds a single connection test instead. + +An item that failed is reported with an error beside its value. The tile keeps its last known value and is marked out of date, so a service that did not answer never reads as zero. ## Cache busting diff --git a/ui/js/badge-logic.js b/ui/js/badge-logic.js index c892a7c3..f550fcba 100644 --- a/ui/js/badge-logic.js +++ b/ui/js/badge-logic.js @@ -69,13 +69,27 @@ export function healthReason(detail, translate) { return parts.join(' \u2022 '); } +/** What one item's entry in a /api/badges response means for the tile. + + The route reports a per-item failure alongside the others' values, so an item + that did not answer must not be read as the number zero. + + @param {any} entry @returns {{ value: number|null, failed: boolean }} */ +export function readBadgeUpdate(entry) { + if (!entry || typeof entry !== 'object') return { value: null, failed: true }; + if (entry.kind || entry.error) return { value: null, failed: true }; + const n = Number(entry.value); + if (!Number.isFinite(n)) return { value: null, failed: true }; + return { value: n, failed: false }; +} + /** The badge an item should show, as class, text and background colour. @param {{ health?: boolean, activity?: number, custom?: { unit?: string, color?: string }, staticBdg?: { enabled?: boolean, label?: string, color?: string }, hasHC?: boolean, hideHealthy?: boolean, - badgesStale?: boolean, healthStale?: boolean, + badgesStale?: boolean, healthStale?: boolean, activityStale?: boolean, healthDetail?: Record, translate?: (key: string, vars?: Record) => string, }} opts */ @@ -88,6 +102,7 @@ export function computeBadgeVisual({ hideHealthy, badgesStale, healthStale, + activityStale, healthDetail, translate, }) { @@ -126,7 +141,10 @@ export function computeBadgeVisual({ else if (staticBdg.enabled && staticBdg.label) aria = staticBdg.label; else if (cls.includes('green')) aria = tr('status.healthy'); - if ((activity > 0 && badgesStale) || ((health || cls.includes('green')) && healthStale)) { + if ( + (activity > 0 && (badgesStale || activityStale)) || + ((health || cls.includes('green')) && (healthStale || activityStale)) + ) { cls += ' stale'; aria = (aria ? aria + ' ' : '') + tr('status.stale'); } diff --git a/ui/js/dashboard.js b/ui/js/dashboard.js index 37535884..57e03c60 100644 --- a/ui/js/dashboard.js +++ b/ui/js/dashboard.js @@ -26,7 +26,7 @@ import { initI18n, t, currentLang } from '/js/i18n.js?v=133a7aac'; import { pwStrength, passwordMismatch } from '/js/password-strength.js?v=dab9978e'; import { sanitizeItemLinks } from '/js/link-url.js?v=19038560'; import { initUI, mkFolder, openFolderDesktop, openFolderMobile, buildMobile } from '/js/ui.js?v=1515288b'; -import { badgeSignature, computeBadgeVisual } from '/js/badge-logic.js?v=d278c683'; +import { badgeSignature, computeBadgeVisual, readBadgeUpdate } from '/js/badge-logic.js?v=d278c683'; import { configChanged, landingAfterSetup, @@ -118,6 +118,7 @@ function bupd(id) { const { cls, txt, bg, aria, color, title } = computeBadgeVisual({ health: s.health, activity: s.activity, + activityStale: !!s.activityStale, custom, staticBdg, hasHC, @@ -412,7 +413,13 @@ function refreshBadges() { async function pollBadges() { try { const d = await (await fetch('/api/badges', { cache: 'no-store' })).json(); - for (const [id, v] of Object.entries(d)) bset(id, 'activity', v.value || 0); + for (const [id, v] of Object.entries(d)) { + const { value, failed } = readBadgeUpdate(v); + /* A failed item keeps its last value, marked stale. Overwriting it with + zero reads as "nothing pending", which is not what happened. */ + bset(id, 'activityStale', failed); + if (!failed) bset(id, 'activity', value); + } _badgeFails = 0; if (badgesStale) { badgesStale = false; diff --git a/ui/test/badge-logic.test.mjs b/ui/test/badge-logic.test.mjs index 72ef7e8a..7c51ebb4 100644 --- a/ui/test/badge-logic.test.mjs +++ b/ui/test/badge-logic.test.mjs @@ -1,6 +1,14 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { computeBadgeVisual, needsDark, resolveColor, NAMED, healthReason, badgeSignature } from '../js/badge-logic.js'; +import { + computeBadgeVisual, + needsDark, + resolveColor, + NAMED, + healthReason, + badgeSignature, + readBadgeUpdate, +} from '../js/badge-logic.js'; test('unhealthy takes priority over everything else', () => { const v = computeBadgeVisual({ @@ -211,3 +219,39 @@ test('a stale badge signs differently from the same badge fresh', () => { const stale = computeBadgeVisual({ activity: 2, badgesStale: true }); assert.notEqual(badgeSignature(fresh), badgeSignature(stale)); }); + +/* ── one item failing inside a working response ─────────────────────────────── + The route reports each item separately: a failure arrives as an error field + beside a value of zero. Reading the zero paints "nothing pending" on a tile + whose service never answered. */ + +test('a reported value is read as a value', () => { + assert.deepEqual(readBadgeUpdate({ value: 4 }), { value: 4, failed: false }); + assert.deepEqual(readBadgeUpdate({ value: 0 }), { value: 0, failed: false }); +}); + +test('an item that failed is not read as zero', () => { + assert.deepEqual(readBadgeUpdate({ value: 0, error: 'Timed out', kind: 'timeout' }), { value: null, failed: true }); + assert.deepEqual(readBadgeUpdate({ value: 0, error: 'Connection refused.' }), { value: null, failed: true }); +}); + +test('an entry that is missing or the wrong shape counts as failed', () => { + for (const v of [null, undefined, 5, 'x', {}, { value: 'many' }]) { + assert.deepEqual(readBadgeUpdate(v), { value: null, failed: true }, JSON.stringify(v) ?? 'undefined'); + } +}); + +test('an item that failed marks its badge as out of date', () => { + const fresh = computeBadgeVisual({ activity: 3 }); + const stale = computeBadgeVisual({ activity: 3, activityStale: true }); + assert.ok(stale.cls.includes('stale')); + assert.match(stale.aria, /out of date/); + assert.equal(stale.txt, fresh.txt, 'the last known value is still shown'); + assert.notEqual(badgeSignature(fresh), badgeSignature(stale)); +}); + +test('a healthy dot is marked out of date too when its item failed', () => { + const v = computeBadgeVisual({ hasHC: true, hideHealthy: false, activityStale: true }); + assert.ok(v.cls.includes('green')); + assert.ok(v.cls.includes('stale')); +});