${s.totalHops.toLocaleString()}
Total Hops Analyzed
@@ -3112,11 +3155,13 @@
}
}
-function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData = {}; _channelData = null; if (_ngState && _ngState.animId) { cancelAnimationFrame(_ngState.animId); } _ngState = null; if (_themeRefreshHandler) { window.removeEventListener('theme-refresh', _themeRefreshHandler); _themeRefreshHandler = null; } }
+function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _stopDistanceRetry(); _analyticsData = {}; _channelData = null; if (_ngState && _ngState.animId) { cancelAnimationFrame(_ngState.animId); } _ngState = null; if (_themeRefreshHandler) { window.removeEventListener('theme-refresh', _themeRefreshHandler); _themeRefreshHandler = null; } }
// Expose for testing
if (typeof window !== 'undefined') {
- window._analyticsDecorateChannels = decorateAnalyticsChannels;
+ window._distanceIsBuilding = _distanceIsBuilding;
+ window._distanceRetryDelayMs = _distanceRetryDelayMs;
+ window._analyticsDecorateChannels = decorateAnalyticsChannels;
window._analyticsSortChannels = sortChannels;
window._analyticsLoadChannelSort = loadChannelSort;
window._analyticsSaveChannelSort = saveChannelSort;
diff --git a/public/app.js b/public/app.js
index 5969dbed..9d9485bb 100644
--- a/public/app.js
+++ b/public/app.js
@@ -165,7 +165,12 @@ async function api(path, { ttl = 0, bust = false } = {}) {
_apiPerf.log.push({ path, ms: Math.round(ms), time: Date.now() });
if (_apiPerf.log.length > 200) _apiPerf.log.shift();
if (ms > 500) console.warn(`[SLOW API] ${path} took ${Math.round(ms)}ms`);
- if (ttl > 0) _apiCache.set(path, { data, expires: Date.now() + ttl });
+ // #1997: never cache a "not ready yet" body. The lazy distance
+ // index answers 202 with {status:"building"} until it is built, and
+ // res.ok is true for 202, so caching it would serve that placeholder
+ // back to every retry for the whole TTL — the page would stay in its
+ // building state for minutes after the index was ready.
+ if (ttl > 0 && res.status !== 202) _apiCache.set(path, { data, expires: Date.now() + ttl });
return data;
}
} finally {
diff --git a/scripts/non-unit-tests.json b/scripts/non-unit-tests.json
index d89eb9cb..2c457e56 100644
--- a/scripts/non-unit-tests.json
+++ b/scripts/non-unit-tests.json
@@ -80,6 +80,7 @@
"tests/e2e/test-issue-1758-ng-filter-rerenders-e2e.js",
"tests/e2e/test-issue-1799-label-vocab-e2e.js",
"tests/e2e/test-issue-1833-legend-toggle-clickable-e2e.js",
+ "tests/e2e/test-issue-1997-distance-building-e2e.js",
"tests/e2e/test-live-dedup.js",
"tests/e2e/test-live-fullscreen-1572-e2e.js",
"tests/e2e/test-live-layout-1178-1179-e2e.js",
diff --git a/test-all.sh b/test-all.sh
index 02165abf..f9622ad3 100755
--- a/test-all.sh
+++ b/test-all.sh
@@ -133,6 +133,7 @@ node tests/unit/test-issue-1868-control-decode.js
node tests/unit/test-issue-1890-og-url.js
node tests/unit/test-issue-1956-release-routing.js
node tests/unit/test-issue-1979-scope-adverts-by-role.js
+node tests/unit/test-issue-1997-distance-building.js
node tests/unit/test-issue-2001-map-scope-state.js
node tests/unit/test-issue-2012-clear-filters-selection.js
node tests/unit/test-live-anims.js
diff --git a/tests/e2e/test-issue-1997-distance-building-e2e.js b/tests/e2e/test-issue-1997-distance-building-e2e.js
new file mode 100644
index 00000000..6feb6e16
--- /dev/null
+++ b/tests/e2e/test-issue-1997-distance-building-e2e.js
@@ -0,0 +1,156 @@
+#!/usr/bin/env node
+/* Issue #1997 — Analytics → Distance against the lazy index's 202.
+ *
+ * /api/analytics/distance answers 202 {status:"building", retry_after_seconds}
+ * with no summary until the distance index (#1011) has been built. The tab
+ * read data.summary.totalHops straight away and threw
+ * "Cannot read properties of undefined (reading 'totalHops')", and api()
+ * cached the placeholder (res.ok is true for 202), so every retry read the
+ * cached "building" body back and the page never recovered.
+ *
+ * The unit suite (tests/unit/test-issue-1997-distance-building.js) pins the
+ * two pure decisions and api()'s refusal to cache a 202. This one pins what
+ * the unit suite cannot: that a real browser, on a real render, shows the
+ * building state instead of an exception, retries on its own, replaces it
+ * with the data once the server is ready, and stops retrying when the user
+ * leaves the tab.
+ *
+ * Both responses are served by a route interception, so the suite does not
+ * depend on whether the server under test has an index built.
+ *
+ * CHROMIUM_REQUIRE=1 makes Chromium-launch failure a HARD FAIL.
+ */
+'use strict';
+
+const { chromium } = require('playwright');
+
+const BASE = process.env.BASE_URL || 'http://localhost:13581';
+
+let passes = 0, failures = 0;
+function pass(msg) { console.log(` ✓ ${msg}`); passes++; }
+function fail(msg) { console.error(` ✗ ${msg}`); failures++; }
+
+const READY = {
+ summary: { totalHops: 4242, totalPaths: 77, avgDist: 12.5, maxDist: 88.25 },
+ catStats: { 'R↔R': { count: 10, avg: 12, median: 11, min: 1, max: 40 } },
+ distHistogram: { bins: [{ x: 1.5, count: 3 }, { x: 2.5, count: 5 }] },
+ distOverTime: [],
+ topHops: [],
+ topPaths: [],
+};
+
+async function main() {
+ const requireChromium = process.env.CHROMIUM_REQUIRE === '1';
+ let browser;
+ try {
+ browser = await chromium.launch({ headless: true });
+ } catch (err) {
+ if (requireChromium) {
+ console.error(`HARD FAIL — Chromium unavailable: ${err.message}`);
+ process.exit(1);
+ }
+ console.warn(`SKIP — Chromium unavailable: ${err.message}`);
+ process.exit(0);
+ }
+
+ const ctx = await browser.newContext({ viewport: { width: 1280, height: 900 } });
+ const page = await ctx.newPage();
+
+ // The server answers "building" until this flips, exactly like the lazy
+ // index does once its first build finishes.
+ let ready = false;
+ let distanceCalls = 0;
+ await page.route('**/api/analytics/distance*', async (route) => {
+ distanceCalls++;
+ if (ready) {
+ await route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(READY) });
+ } else {
+ await route.fulfill({
+ status: 202,
+ contentType: 'application/json',
+ body: JSON.stringify({ status: 'building', retry_after_seconds: 1, detail: 'distance index is being computed' }),
+ });
+ }
+ });
+
+ // Any "reading 'totalHops' of undefined" would land here.
+ const pageErrors = [];
+ page.on('pageerror', (e) => pageErrors.push(e.message));
+
+ await page.goto(`${BASE}/#/analytics?tab=distance`, { waitUntil: 'domcontentloaded' });
+
+ // (1) The 202 renders the building state, not an exception and not a crash.
+ try {
+ await page.waitForSelector('#distanceBuilding', { timeout: 15000 });
+ pass('(1) the 202 renders the building state');
+ } catch {
+ const shown = await page.evaluate(() => {
+ const el = document.getElementById('analyticsContent');
+ return el ? (el.innerText || '').slice(0, 300) : '(no #analyticsContent)';
+ });
+ fail(`(1) the building state never appeared within 15s; the tab showed: ${JSON.stringify(shown)}`);
+ }
+
+ // (2) The reported symptom, asserted where it is actually visible.
+ // renderDistanceTab wraps its body in try/catch, so the TypeError never
+ // reaches window.onerror: it is caught and painted into the tab as
+ // "Failed to load distance analytics: Cannot read properties of undefined
+ // (reading 'totalHops')". Asserting on pageErrors here would pass on the
+ // broken build too, so assert on the text the user sees.
+ const buildingText = await page.evaluate(() => {
+ const el = document.getElementById('analyticsContent');
+ return el ? (el.innerText || '') : '';
+ });
+ if (!/Failed to load distance analytics|Cannot read properties of undefined/.test(buildingText)) {
+ pass('(2) the building body is not rendered as data');
+ } else {
+ fail(`(2) the building body was rendered as data: ${JSON.stringify(buildingText.slice(0, 160))}`);
+ }
+
+ // (3) The tab retries on its own: a second request arrives without any
+ // interaction. retry_after_seconds is 1, so this is a short wait.
+ const callsAfterFirstRender = distanceCalls;
+ // The counter lives in Node (the route handler), so poll it here.
+ const deadline = Date.now() + 10000;
+ while (distanceCalls <= callsAfterFirstRender && Date.now() < deadline) {
+ await page.waitForTimeout(200);
+ }
+ if (distanceCalls > callsAfterFirstRender) pass(`(3) the tab retried on its own (${distanceCalls} requests)`);
+ else fail('(3) no retry arrived within 10s, so a stuck build never recovers');
+
+ // (4) Once the server is ready, the retry replaces the placeholder with
+ // the real numbers. This is the half that api()'s 202 caching broke: a
+ // cached placeholder would keep the building state on screen.
+ ready = true;
+ try {
+ await page.waitForFunction(() => {
+ const el = document.getElementById('analyticsContent');
+ return !!el && /4,?242/.test(el.innerText || '');
+ }, null, { timeout: 15000 });
+ pass('(4) the retry replaces the placeholder with the real payload');
+ } catch {
+ const shown = await page.evaluate(() => {
+ const el = document.getElementById('analyticsContent');
+ return el ? (el.innerText || '').slice(0, 300) : '(no #analyticsContent)';
+ });
+ fail(`(4) the tab never showed the ready payload; it showed: ${JSON.stringify(shown)}`);
+ }
+
+ // (5) Leaving the tab stops the retry. Go back to building, let one retry
+ // be scheduled, navigate away, and assert the request count stops moving.
+ ready = false;
+ await page.evaluate(() => { window.location.hash = '#/analytics?tab=overview'; });
+ await page.waitForTimeout(500);
+ const before = distanceCalls;
+ await page.waitForTimeout(3000); // three retry intervals
+ if (distanceCalls === before) pass('(5) no retry fires after leaving the tab');
+ else fail(`(5) the retry kept running after leaving the tab: ${distanceCalls - before} extra request(s)`);
+
+ if (pageErrors.length) console.log(` (page errors seen: ${pageErrors.length}) ${pageErrors.slice(0, 3).join(' | ')}`);
+
+ await browser.close();
+ console.log(`\ntest-issue-1997-distance-building-e2e: ${passes} passed, ${failures} failed`);
+ process.exit(failures ? 1 : 0);
+}
+
+main().catch((e) => { console.error(e); process.exit(1); });
diff --git a/tests/unit/test-issue-1997-distance-building.js b/tests/unit/test-issue-1997-distance-building.js
new file mode 100644
index 00000000..1e8f6c7a
--- /dev/null
+++ b/tests/unit/test-issue-1997-distance-building.js
@@ -0,0 +1,180 @@
+/* test-issue-1997-distance-building.js
+ *
+ * Issue #1997: the Distance tab crashed on the lazy index's first response.
+ *
+ * The backend answers 202 with {status:"building", retry_after_seconds:5} and
+ * no summary until the index (#1011) has been built. Two things went wrong:
+ *
+ * 1. renderDistanceTab read data.summary.totalHops straight away, so the
+ * page showed "Cannot read properties of undefined (reading 'totalHops')".
+ * 2. api() caches any res.ok body, and 202 is ok, so the placeholder was
+ * cached for the analyticsRF TTL. Even a correct retry would then read
+ * the cached "building" body back and stay stuck long after the index
+ * was ready. That is the half that makes the first one permanent.
+ *
+ * This pins both: the two pure decisions the renderer makes, and api()'s
+ * refusal to cache a 202 while still caching a 200.
+ */
+'use strict';
+const fs = require('fs');
+const path = require('path');
+const vm = require('vm');
+const assert = require('assert');
+
+const REPO_ROOT = path.resolve(__dirname, '..', '..');
+
+let passed = 0, failed = 0;
+function test(name, fn) {
+ try { fn(); passed++; console.log(' ✓ ' + name); }
+ catch (e) { failed++; console.error(' ✗ ' + name + ': ' + e.message); }
+}
+async function testAsync(name, fn) {
+ try { await fn(); passed++; console.log(' ✓ ' + name); }
+ catch (e) { failed++; console.error(' ✗ ' + name + ': ' + e.message); }
+}
+
+// ── The renderer's two decisions, loaded from the real analytics.js ─────────
+function loadAnalyticsHelpers() {
+ const sandbox = {
+ console,
+ window: null,
+ document: {
+ documentElement: {},
+ createElement: () => ({ style: {}, addEventListener() {} }),
+ addEventListener() {}, removeEventListener() {},
+ querySelector: () => null, querySelectorAll: () => [], getElementById: () => null,
+ },
+ localStorage: { getItem: () => null, setItem() {}, removeItem() {} },
+ getComputedStyle: () => ({ getPropertyValue: () => '' }),
+ registerPage: () => {},
+ api: async () => ({}),
+ fetch: async () => ({ ok: true, json: async () => ({}) }),
+ CLIENT_TTL: {},
+ RegionFilter: { getRegionParam: () => '', regionQueryString: () => '' },
+ Storage: function () {},
+ timeAgo: () => '',
+ histogram: () => ({ svg: '' }),
+ setTimeout, clearTimeout, setInterval, clearInterval,
+ requestAnimationFrame: () => 0,
+ cancelAnimationFrame: () => {},
+ };
+ sandbox.window = sandbox;
+ vm.createContext(sandbox);
+ vm.runInContext(fs.readFileSync(path.join(REPO_ROOT, 'public/analytics.js'), 'utf8'), sandbox);
+ return sandbox;
+}
+
+console.log('\n=== #1997: the Distance tab and the lazy index\'s 202 ===');
+
+const a = loadAnalyticsHelpers();
+
+test('the helpers are exposed for testing', () => {
+ assert.strictEqual(typeof a._distanceIsBuilding, 'function', '_distanceIsBuilding');
+ assert.strictEqual(typeof a._distanceRetryDelayMs, 'function', '_distanceRetryDelayMs');
+});
+
+test('the 202 body is recognised as still building', () => {
+ const body = { status: 'building', retry_after_seconds: 5, detail: 'distance index is being computed' };
+ assert.strictEqual(a._distanceIsBuilding(body), true);
+});
+
+test('a real payload is not treated as building', () => {
+ assert.strictEqual(a._distanceIsBuilding({ summary: { totalHops: 12 }, catStats: {} }), false);
+ assert.strictEqual(a._distanceIsBuilding({}), false);
+ assert.strictEqual(a._distanceIsBuilding(null), false);
+ assert.strictEqual(a._distanceIsBuilding(undefined), false);
+});
+
+test('a body carrying both a status and a summary is data, not a placeholder', () => {
+ // Defensive: if the contract ever grows a status field on real payloads,
+ // the presence of a summary decides, so the tab renders instead of looping.
+ assert.strictEqual(a._distanceIsBuilding({ status: 'building', summary: { totalHops: 1 } }), false);
+});
+
+test('the retry honours the server interval, and is clamped at both ends', () => {
+ assert.strictEqual(a._distanceRetryDelayMs({ retry_after_seconds: 5 }), 5000, 'uses what the server asked for');
+ assert.strictEqual(a._distanceRetryDelayMs({}), 5000, 'defaults to the server default when absent');
+ assert.strictEqual(a._distanceRetryDelayMs({ retry_after_seconds: 0 }), 5000, 'zero would be a hot loop');
+ assert.strictEqual(a._distanceRetryDelayMs({ retry_after_seconds: -3 }), 5000, 'negative likewise');
+ assert.strictEqual(a._distanceRetryDelayMs({ retry_after_seconds: 'soon' }), 5000, 'non-numeric likewise');
+ assert.strictEqual(a._distanceRetryDelayMs({ retry_after_seconds: 0.2 }), 1000, 'clamped up to 1s');
+ assert.strictEqual(a._distanceRetryDelayMs({ retry_after_seconds: 600 }), 30000, 'clamped down to 30s');
+});
+
+// ── api() must not cache a 202 ──────────────────────────────────────────────
+function loadApi(responses) {
+ const calls = [];
+ const sandbox = {
+ console,
+ window: null,
+ document: {
+ documentElement: { style: {}, setAttribute() {}, getAttribute: () => null, classList: { add() {}, remove() {}, contains: () => false } },
+ createElement: () => ({ style: {}, classList: { add() {}, remove() {} }, appendChild() {}, addEventListener() {} }),
+ addEventListener() {}, removeEventListener() {},
+ querySelector: () => null, querySelectorAll: () => [], getElementById: () => null,
+ body: { appendChild() {}, classList: { add() {}, remove() {} } },
+ },
+ localStorage: { getItem: () => null, setItem() {}, removeItem() {} },
+ location: { hash: '', search: '', pathname: '/' },
+ history: { replaceState() {}, pushState() {} },
+ performance: { now: () => 0 },
+ navigator: { userAgent: 'node' },
+ matchMedia: () => ({ matches: false, addEventListener() {}, removeEventListener() {} }),
+ getComputedStyle: () => ({ getPropertyValue: () => '' }),
+ setTimeout: (fn) => { fn(); return 0; }, // no wall-clock waiting
+ clearTimeout() {}, setInterval: () => 0, clearInterval() {},
+ requestAnimationFrame: () => 0, cancelAnimationFrame() {},
+ WebSocket: function () { this.close = function () {}; this.addEventListener = function () {}; },
+ addEventListener() {}, removeEventListener() {}, dispatchEvent: () => true,
+ CustomEvent: function () {},
+ URLSearchParams,
+ // app.js issues its own fetches while it loads (version banner, config,
+ // …). Only the distance path draws from the queued responses, and only
+ // it is counted: otherwise a load-time fetch consumes the 202 and the
+ // assertions below measure the wrong request.
+ fetch: async (url) => {
+ const mine = String(url).indexOf('/analytics/distance') !== -1;
+ if (!mine) return { ok: true, status: 200, headers: { get: () => null }, json: async () => ({}) };
+ calls.push(url);
+ const r = responses.shift() || responses[responses.length - 1];
+ return {
+ ok: r.status >= 200 && r.status < 300,
+ status: r.status,
+ headers: { get: (h) => (h.toLowerCase() === 'retry-after' ? r.retryAfter || null : null) },
+ json: async () => r.body,
+ };
+ },
+ };
+ sandbox.window = sandbox;
+ vm.createContext(sandbox);
+ vm.runInContext(fs.readFileSync(path.join(REPO_ROOT, 'public/app.js'), 'utf8'), sandbox);
+ return { sandbox, calls };
+}
+
+(async () => {
+ await testAsync('a 202 body is returned but never cached, so the retry reaches the server', async () => {
+ const building = { status: 202, body: { status: 'building', retry_after_seconds: 5 } };
+ const ready = { status: 200, body: { summary: { totalHops: 42 }, catStats: {} } };
+ const { sandbox, calls } = loadApi([building, ready]);
+
+ const first = await sandbox.api('/analytics/distance', { ttl: 300000 });
+ assert.strictEqual(first.status, 'building', 'the placeholder is handed to the caller, not swallowed');
+
+ const second = await sandbox.api('/analytics/distance', { ttl: 300000 });
+ assert.ok(second.summary, 'the retry must reach the server and get the real payload');
+ assert.strictEqual(second.summary.totalHops, 42);
+ assert.strictEqual(calls.length, 2, 'the second call must not have been served from cache');
+ });
+
+ await testAsync('a 200 body is still cached, so the fix does not disable caching', async () => {
+ const ready = { status: 200, body: { summary: { totalHops: 7 }, catStats: {} } };
+ const { sandbox, calls } = loadApi([ready, ready]);
+
+ await sandbox.api('/analytics/distance', { ttl: 300000 });
+ await sandbox.api('/analytics/distance', { ttl: 300000 });
+ assert.strictEqual(calls.length, 1, 'the second call should have been served from cache');
+ });
+
+ console.log(`\nTotal: ${passed} passed, ${failed} failed`);
+ if (failed) process.exit(1);
+})();