From 4d33e484ff45081d26c2f0603a15bd36045adc69 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 14 Oct 2026 09:08:33 +0000 Subject: [PATCH] Interval change keeps unreported samples for the reporter (closes #124) The interval menu cleared every host's history, and the reporter builds each report from that history, so the samples recorded since the last delivered report were never sent. The menu now resets each host with HostState.reset, which moves the history's entries to clearedHistory, keeping the last 100 it has cleared, and buildReport reads both. A host still pending after the reset is sent with the status of its last check. The per-host marks are unchanged, so a sample a delivered report had is not sent again, and the page's history still starts over. Reporter is exported for the new unit tests. Model: opus-5-5 --- README.md | 8 ++-- TODO.md | 12 ++++++ test/unit/main.test.js | 93 ++++++++++++++++++++++++++++++++++++++++++ web/src/main.js | 50 +++++++++++++++++------ 4 files changed, 148 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 47a9495..f9063f8 100644 --- a/README.md +++ b/README.md @@ -149,9 +149,11 @@ same-origin path `/api/v1/reports`: a random per-browser `clientId` kept in `localStorage`, `geo` sent as null, and each host's unreported, non-paused samples (timestamp, latency, error). A per-host high-water mark makes every report a delta, so only new samples are sent; the mark advances only on a -delivered report, and while paused nothing is sent. Delivery failure is quiet — -one debug-log line per outage, retried at the next interval, never blocking -probing. The report-building step is a pure function of host state. +delivered report, and while paused nothing is sent. Changing the interval clears +each host's history on the page; the samples in it not yet reported still go in +the next report. Delivery failure is quiet — one debug-log line per outage, +retried at the next interval, never blocking probing. The report-building step +is a pure function of host state. ### Backend diff --git a/TODO.md b/TODO.md index 76023ff..f666490 100644 --- a/TODO.md +++ b/TODO.md @@ -21,6 +21,18 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of # Completed Steps +- 2026-10-14: changing the interval no longer drops the samples not yet reported + to the backend ([#124](https://git.eeqj.de/sneak/netwatch/issues/124)). The + interval menu resets each host with the new `HostState.reset`, which moves the + history's entries to `clearedHistory`, keeping the last 100 of all it has + cleared, and `buildReport` reads both, so the next report has the samples + recorded before the change that no report has delivered. Until its next check, + such a host is sent with the status of its last check, not `pending`. The + page's history still starts over, and the per-host marks still leave out of a + report the samples a delivered one had. Unit tests reset a host as the + interval menu does and find its unreported samples in the next report, and + only the last 100 after more were cleared; `web/src/main.js` now exports + `Reporter` for them - 2026-10-14: the repo has the layout `REPO_POLICIES.md` gives ([#30](https://git.eeqj.de/sneak/netwatch/issues/30)). The Go module is at the root (`go.mod`, `go.sum`, `.golangci.yml`, `cmd/netwatch-server/`, diff --git a/test/unit/main.test.js b/test/unit/main.test.js index 360b1a7..9b8ada8 100644 --- a/test/unit/main.test.js +++ b/test/unit/main.test.js @@ -14,6 +14,7 @@ import { latencyClass, latencyHex, measureLatency, + Reporter, tick, } from "../../web/src/main.js"; @@ -378,6 +379,98 @@ test("a check that fails in the first round, which is discarded, writes nothing assert.deepEqual(consoleLines("info"), []); }); +// The reports Reporter sent in this test, each as the hosts it sent. +function reportedHosts() { + return fetch.mock.calls.map( + (call) => JSON.parse(call.arguments[1].body).hosts, + ); +} + +// In the next two tests every report is delivered, and only Target has +// checks to report. target.reset() does to Target what the interval menu +// does to each target. + +// The interval changes after the first report. The second report is sent +// before Target's first check since then, and the third after it. A fourth +// would hold no check the first three did not, so it is not sent. +test("checks recorded before an interval change and not yet reported go in the next report, with the status of the last check, and no check is reported twice", async (t) => { + const state = new AppState([ + { name: "Target", url: "https://target.test" }, + ]); + const target = state.local[0]; + const reporter = new Reporter(state, "client", CONFIG.reportInterval); + t.mock.method(globalThis, "fetch", async () => ({ ok: true })); + + target.pushSample(1000, { latency: 10, error: null }); + await reporter.flush(); + target.pushSample(2000, { latency: 20, error: null }); + target.pushSample(3000, { latency: null, error: "timeout" }); + target.reset(); + await reporter.flush(); + target.pushSample(4000, { latency: 40, error: null }); + await reporter.flush(); + await reporter.flush(); + + const name = "Target"; + const url = "https://target.test"; + assert.deepEqual(reportedHosts(), [ + [ + { + name, + url, + status: "online", + history: [{ t: 1000, latency: 10, error: null }], + }, + ], + [ + { + name, + url, + status: "error", + history: [ + { t: 2000, latency: 20, error: null }, + { t: 3000, latency: null, error: "timeout" }, + ], + }, + ], + [ + { + name, + url, + status: "online", + history: [{ t: 4000, latency: 40, error: null }], + }, + ], + ]); + // The page's history starts over. + assert.deepEqual(target.history, [ + { timestamp: 4000, latency: 40, error: null }, + ]); +}); + +// The interval changes twice, after 60 checks each time, before the first +// report. Of the 120 checks the interval menu cleared, the report has the +// last 100 (CONFIG.maxHistoryPoints). +test("of the checks interval changes cleared before they were reported, the next report has the last 100", async (t) => { + const state = new AppState([ + { name: "Target", url: "https://target.test" }, + ]); + const target = state.local[0]; + const reporter = new Reporter(state, "client", CONFIG.reportInterval); + t.mock.method(globalThis, "fetch", async () => ({ ok: true })); + + for (let n = 1; n <= 120; n++) { + target.pushSample(n * 1000, { latency: 10, error: null }); + if (n % 60 === 0) target.reset(); + } + await reporter.flush(); + + const sent = reportedHosts()[0][0].history; + assert.equal(sent.length, 100); + assert.equal(sent[0].t, 21000); + assert.equal(sent.at(-1).t, 120000); +}); + // The page shows < > " & and ' in a row's markup as // < > " & and '. test(`a target whose name and URL hold < > " & and ' shows those characters in its row`, () => { diff --git a/web/src/main.js b/web/src/main.js index 821e381..077c7e1 100644 --- a/web/src/main.js +++ b/web/src/main.js @@ -257,6 +257,14 @@ function latencyStats(latencies) { }; } +// The status a check's error gives its host: "error" for a timeout, +// "offline" for any other error, and "online" for none. +function statusFromError(error) { + if (error === "timeout") return "error"; + if (error) return "offline"; + return "online"; +} + export class HostState { constructor(host, pinned = false) { this.name = host.name; @@ -265,6 +273,10 @@ export class HostState { // error }, or a round skipped while paused, { timestamp, // latency: null, paused: true }. this.history = []; + // The last 100 (CONFIG.maxHistoryPoints) entries the interval menu + // has cleared from the history, which the reporter may not have + // sent yet. + this.clearedHistory = []; this.lastLatency = null; this.status = "pending"; // 'online' | 'offline' | 'error' | 'pending' this.pinned = pinned; @@ -280,9 +292,7 @@ export class HostState { }); this._trim(); this.lastLatency = result.latency; - if (result.error === "timeout") this.status = "error"; - else if (result.error) this.status = "offline"; - else this.status = "online"; + this.status = statusFromError(result.error); this.consecutiveFailures = result.error ? this.consecutiveFailures + 1 : 0; @@ -293,6 +303,18 @@ export class HostState { this._trim(); } + // What the interval menu does to each host: the history starts over and + // the host waits for its next check. The history's entries move to + // clearedHistory, so the reporter still sends those it has not. + reset() { + this.clearedHistory = [...this.clearedHistory, ...this.history].slice( + -CONFIG.maxHistoryPoints, + ); + this.history = []; + this.lastLatency = null; + this.status = "pending"; + } + // The min, max, median and average latency of the checks in the history // that got an answer. historyStats() { @@ -429,9 +451,12 @@ function getClientId() { // Build the delta report body the backend decodes, plus the new per-host // high-water marks. Pure function of the passed state: `hosts` is an array -// of { name, url, status, history }, `since` maps a host url to the Unix-ms -// timestamp of the last sample already reported for it, and `now` is a Date. -// Only non-paused samples newer than the mark are included. Returns null +// of { name, url, status, clearedHistory, history }, `since` maps a host url +// to the Unix-ms timestamp of the last sample already reported for it, and +// `now` is a Date. Only non-paused samples newer than the mark are included, +// from clearedHistory as well as history. A host whose status is pending, +// as the interval menu leaves it until its next check, is sent with the +// status of its last check, which is the last sample sent. Returns null // when no host has an unreported sample. export function buildReport(hosts, clientId, now, since) { const reportHosts = []; @@ -440,7 +465,7 @@ export function buildReport(hosts, clientId, now, since) { const mark = since.get(host.url) ?? 0; const samples = []; let high = mark; - for (const p of host.history) { + for (const p of [...host.clearedHistory, ...host.history]) { if (p.paused) continue; if (p.timestamp <= mark) continue; samples.push({ @@ -454,7 +479,10 @@ export function buildReport(hosts, clientId, now, since) { reportHosts.push({ name: host.name, url: host.url, - status: host.status, + status: + host.status === "pending" + ? statusFromError(samples.at(-1).error) + : host.status, history: samples, }); marks.set(host.url, high); @@ -481,7 +509,7 @@ export function buildReport(hosts, clientId, now, since) { // Only one report is ever in flight, and it is abandoned after half the // interval, so a slow POST can neither overlap the next report (which would // carry the same samples) nor stall reporting for good. -class Reporter { +export class Reporter { constructor(state, clientId, intervalMs) { this.state = state; this.clientId = clientId; @@ -1502,9 +1530,7 @@ async function init() { // Clear all host history and reset state for (const host of state.allHosts) { - host.history = []; - host.lastLatency = null; - host.status = "pending"; + host.reset(); } state.tickCount = 0;