mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-09-26 02:43:44 +00:00
efitenandClaude Opus 5
5c016a210a
Closes #1997. ## What was wrong `/api/analytics/distance` answers `202 {status:"building", retry_after_seconds:5}` with no `summary` until the lazy index (#1011) has been built. 1. `renderDistanceTab` read `data.summary.totalHops` straight away. The TypeError is caught by the tab's own try/catch, so it never reaches `window.onerror`: it is painted into the tab as `Failed to load distance analytics: Cannot read properties of undefined (reading 'totalHops')`. 2. `api()` caches any `res.ok` body, and `res.ok` is true for 202, so the placeholder was stored for the `analyticsRF` TTL. Even a correct retry read the cached "building" body back. That is the half that made the broken state outlast the index build. ## What changed - `public/app.js:173` skips the cache write when `res.status === 202`. A 200 still caches, unchanged. - `public/analytics.js` renders a building notice and retries itself, honouring `retry_after_seconds` clamped to [1s, 30s]. The timer is cleared on tab switch and in `destroy()`, and a new render supersedes a pending retry, so two renders cannot write into the same tab. The notice uses `.text-center`/`.text-muted` rather than the `.spinner` class used at `analytics.js:1683`, because `.spinner` has no CSS anywhere in the repo and renders nothing. ## Tests `tests/unit/test-issue-1997-distance-building.js` (7 assertions, wired into `test-all.sh`) pins the two pure decisions the renderer makes and `api()`'s refusal to cache a 202 while still caching a 200. Red-run on the unfixed sources: 6 of 7 fail, and the "a 200 is still cached" control stays green. `tests/e2e/test-issue-1997-distance-building-e2e.js` (classified in `scripts/non-unit-tests.json`, invoked from `deploy.yml` with `CHROMIUM_REQUIRE=1`) serves both responses by route interception, so it does not depend on whether the server under test has an index built. It asserts: the building state appears, the tab does not paint the error text, a retry arrives with no interaction, the retry replaces the placeholder once the server answers 200, and no retry fires after leaving the tab. Check (2) deliberately asserts on the rendered text and not on `pageerror`: the TypeError is caught, so a `pageerror` assertion would pass on the broken build too. ## Verification No local cgo toolchain here since #1992, so I could not build a server to run the E2E against. Instead I ran its five steps in Playwright against a live instance with this branch's `public/app.js` and `public/analytics.js` injected in place of the deployed ones (both files are byte-identical between that instance and upstream master, so the injection is faithful): | check | deployed build | this branch | |---|---|---| | (1) building state shown | fail | pass | | (2) not rendered as data | fail | pass | | (3) retried on its own | fail (1 request) | pass (2 requests) | | (4) real payload after retry | fail | pass | | (5) no retry after leaving the tab | pass | pass | (5) passes on the broken build too: it schedules no retry at all, so it is a control and only means anything together with (3). The committed E2E suite has not been run as committed. CI is its first real run. ## Not done - The server still recomputes on every 202 poll rather than signalling readiness. - No other analytics tab was audited for the same assume-a-summary pattern. - One pre-existing unit suite (`test-preflight-xss-gate.js`) fails on this Windows machine with a cp1252 `UnicodeEncodeError` from its Python helper, on a clean tree as well as with this change. Unrelated, and green on Linux CI. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>