Fix: bound the batched polls separately and show a failed badge as stale

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.
 

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,

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;

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,
 };

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`);
+});

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
 

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<string, unknown>,
       translate?: (key: string, vars?: Record<string, unknown>) => 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');
   }

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;

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'));
+});