Reports refused while report files cannot be written (closes #123)

A failed write of a report file used to drop its reports, and
POST /api/v1/reports kept answering 200 for as long as writes failed.
Now a failed write puts its reports back in the buffer, still counted
against DATA_DIR_MAX_BYTES, and Append refuses every report, which the
handler answers 500, until a write succeeds. The minute flush tries
the held reports again, so storing resumes with no restart once
writing works, and the buffer takes nothing new while writes fail.
Tests also cover a report appended while a write that fails runs, and
the cap once a failed write's reports are written.

Model: opus-5-5

Dockerfile

@@ -5,11 +5,11 @@
 #
 # The lint and test phases are the gates: `make lint` (script/lint)
 # builds the lint stage alone and `make test` (script/test) the test
-# stage alone, and the two build stages, frontend-build and builder,
+# stage alone, and the builder stage depends on both, so the image
-# depend on both, so the image cannot be built unless they pass. Each
+# cannot be built unless they pass. Each covers the frontend as well,
-# phase covers the frontend as well, through a copy from a node stage.
+# through a copy from a node stage. Inside them each tool is invoked
-# Inside them each tool is invoked directly, never through make or
+# directly, never through make or script/, whose lint and test are
-# script/, whose lint and test are themselves docker builds.
+# themselves docker builds.
 
 # Frontend lint stage: eslint with the rules in eslint.config.js. The
 # lint phase below runs it.
@@ -35,26 +35,30 @@ RUN golangci-lint run --config .golangci.yml ./...
 # phase run it.
 COPY --from=frontend-lint /app/yarn.lock /dev/null
 
-# Frontend test stage: the unit tests in test/unit/. The test phase
+# Frontend stage: the unit tests in test/unit/, then the production
-# below runs it, and the frontend-build stage starts from it. The tests
+# build into dist/, which the runtime stage serves. The test phase below
-# print a dot each; if any fails, they run again with every test listed,
+# runs it. The tests print a dot each; if any fails, they run again with
-# and the step fails even if that run passes. NODE_OPTIONS chooses the
+# every test listed, and the step fails even if that run passes.
-# reporter because yarn adds its arguments after the test files, where
+# NODE_OPTIONS chooses the reporter because yarn adds its arguments
-# node would take a reporter option for one more file.
+# after the test files, where node would take a reporter option for one
+# more file.
 # node:22-alpine as of 2026-02-22
-FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34 AS frontend-test
+FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34 AS frontend
 WORKDIR /app
 COPY package.json yarn.lock ./
 RUN yarn install --frozen-lockfile
+# vite.config.js reads the commit for the page's footer with git.
+RUN apk add --no-cache git
 COPY . .
 RUN NODE_OPTIONS=--test-reporter=dot timeout 90 yarn --silent run test || \
     { echo "--- Rerunning with every test listed for details ---"; \
       NODE_OPTIONS=--test-reporter=spec timeout 90 yarn --silent run test; \
       exit 1; }
+RUN yarn build
 
 # Test phase: the backend's tests with the race detector and coverage,
-# and the frontend's through the copy from the frontend-test stage at
+# and the frontend's through the copy from the frontend stage at the
-# the end. -race needs cgo and so a C compiler, which the Debian Go image
+# end. -race needs cgo and so a C compiler, which the Debian Go image
 # ships and the alpine one does not. -timeout 90s is a backstop above
 # the 60-second cap on the suite. The rerun with -v only shows details:
 # the step fails however it ends, because the first run already failed.
@@ -71,21 +75,9 @@ RUN --mount=type=tmpfs,target=/go/pkg/mod \
     go test -timeout 90s -race -cover ./... || \
     { echo "--- Rerunning with -v for details ---"; \
       go test -timeout 90s -race -v ./...; exit 1; }
-# Nothing is wanted from the frontend-test stage; the copy is what makes
+# Nothing is wanted from the frontend stage; the copy is what makes
 # this phase run its tests.
-COPY --from=frontend-test /app/yarn.lock /dev/null
+COPY --from=frontend /app/yarn.lock /dev/null
-
-# Frontend build stage: the production build into dist/, which the
-# runtime stage serves, on the yarn packages and source of the
-# frontend-test stage. Nothing is wanted from the two phases; the copies
-# are what make BuildKit build them first, so this stage cannot run
-# unless lint and test passed.
-FROM frontend-test AS frontend-build
-COPY --from=lint /src/go.sum /dev/null
-COPY --from=test /src/go.sum /dev/null
-# vite.config.js reads the commit for the page's footer with git.
-RUN apk add --no-cache git
-RUN yarn build
 
 # Backend build stage. Nothing is wanted from the two phases; the copies
 # are what make BuildKit build them first, so this stage cannot run
@@ -139,7 +131,7 @@ RUN addgroup -g 1000 -S netwatch && \
 RUN rm /etc/nginx/conf.d/default.conf
 COPY configs/nginx.conf /etc/nginx/templates/netwatch.conf.template
 COPY configs/security-headers.conf /etc/nginx/security-headers.conf
-COPY --from=frontend-build /app/dist /usr/share/nginx/html
+COPY --from=frontend /app/dist /usr/share/nginx/html
 COPY --from=builder /src/netwatch-server /usr/local/bin/netwatch-server
 COPY bin/entrypoint.sh /usr/local/bin/entrypoint.sh
 

README.md

@@ -59,9 +59,8 @@ either one is broken. We provide:
 - `script/clean` — remove what `script/build` writes
 - `script/projectname` — print the project name (used for the Docker image tag)
 - `script/test` — build the `test` phase of `Dockerfile` without the cache: the
-  frontend's unit tests in its `frontend-test` stage, and the backend's Go tests
+  frontend's unit tests and production build in its `frontend` stage, and the
-  with the race detector and coverage. The frontend's production build is left
+  backend's Go tests with the race detector and coverage
-  to the image build, in the `frontend-build` stage
 - `script/lint` — build the `lint` phase of `Dockerfile` without the cache:
   eslint in its `frontend-lint` stage, and golangci-lint over the Go packages
 - `script/fmt` — format all files (writes): prettier over the JavaScript, CSS,
@@ -79,8 +78,8 @@ either one is broken. We provide:
 - `script/frontend-test` — run the unit tests in `test/unit/` on the host with
   Node's built-in test runner, through the `test` script in `package.json`, and
   if any fails, run them again listing every test, and fail; then the production
-  build. Each test run has a 90-second timeout. `make test` runs the same tests
+  build. Each test run has a 90-second timeout. `make test` runs the same in
-  in Docker, without the production build
+  Docker
 - `script/frontend-fmt` — format the JavaScript, CSS, HTML and Markdown with
   prettier (writes)
 - `script/frontend-fmt-check` — check prettier formatting (read-only)
@@ -150,11 +149,9 @@ 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. Changing the interval clears
+delivered report, and while paused nothing is sent. Delivery failure is quiet —
-each host's history on the page; the samples in it not yet reported still go in
+one debug-log line per outage, retried at the next interval, never blocking
-the next report. Delivery failure is quiet — one debug-log line per outage,
+probing. The report-building step is a pure function of host state.
-retried at the next interval, never blocking probing. The report-building step
-is a pure function of host state.
 
 ### Backend
 
@@ -200,25 +197,23 @@ image, `make run` builds it and runs it on port 8080.
 | `DATA_DIR`             | `./data/reports`     | Directory for compressed reports                                                                         |
 | `DATA_DIR_MAX_BYTES`   | `1073741824` (1 GiB) | Most bytes of report files kept in `DATA_DIR`, oldest deleted first; see [Report limits](#report-limits) |
 | `DEBUG`                | `false`              | Enable debug logging                                                                                     |
-| `TRUSTED_PROXIES`      | RFC 1918 ranges      | Comma-separated CIDRs whose `X-Forwarded-For` / `X-Real-IP` headers are trusted for client IP resolution |
+| `TRUSTED_PROXIES`      | loopback + RFC1918   | Comma-separated CIDRs whose `X-Forwarded-For` / `X-Real-IP` headers are trusted for client IP resolution |
 | `REPORTS_PER_MINUTE`   | `60`                 | Reports each client address may send a minute; see [Report limits](#report-limits)                       |
 | `CORS_ALLOWED_ORIGINS` | empty                | Comma-separated origins whose pages may call the API; see [CORS](#cors)                                  |
 | `METRICS_USERNAME`     | empty                | Basic auth user name for `/metrics`; see [Metrics](#metrics)                                             |
 | `METRICS_PASSWORD`     | empty                | Basic auth password for `/metrics`; see [Metrics](#metrics)                                              |
 | `SENTRY_DSN`           | empty                | DSN of the Sentry project to send errors to; see [Sentry](#sentry)                                       |
 
-`TRUSTED_PROXIES` defaults to `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`. A list
+`TRUSTED_PROXIES` defaults to
-replaces the default; set and empty, the server trusts no proxy. Loopback is not
+`127.0.0.1/32,::1/128,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`. The loopback
-in the default, so a reverse proxy on the same host has to be named, as in
+entries cover a reverse proxy on the same host. A request whose direct peer is
-`TRUSTED_PROXIES=127.0.0.1/32`. A request whose direct peer is outside this set
+outside this set has its forwarded headers ignored, and the direct peer is
-has its forwarded headers ignored, and the direct peer is logged and
+logged and rate-limited instead. The container image does not use this default;
-rate-limited instead. The container image does not use this default; see
+see [Deployment](#deployment).
-[Deployment](#deployment).
 
 A variable set to a value the server cannot use, such as `PORT=abc`,
 `DEBUG=maybe` or a `BIND_ADDRESS` that is not an IP address, stops it from
-starting, with an error naming the variable. An empty variable counts as unset,
+starting, with an error naming the variable. An empty variable counts as unset.
-except `TRUSTED_PROXIES`.
 
 #### Report storage
 
@@ -242,15 +237,9 @@ non-zero.
 #### Report limits
 
 `POST /api/v1/reports` takes reports from anyone who can reach it, without
-credentials, so it is bounded instead. Each refusal below answers with the same
+credentials, so it is bounded instead. Both refusals below answer with the same
 `{"status":"error"}` body as any other error.
 
-- **JSON only.** A report must be sent with `Content-Type: application/json`,
-  parameters such as `charset` allowed, as the page sends it; any other is
-  refused with 415, and nothing of it is stored. A page on another origin can
-  send a `text/plain` POST, or a form, without a CORS preflight; refusing them
-  keeps any site from making its visitors' browsers store reports (see
-  [CORS](#cors)).
 - **Rate limit.** Each client address, resolved through `TRUSTED_PROXIES`, may
   send `REPORTS_PER_MINUTE` reports a minute; past that it gets 429 with
   `Retry-After: 60`. The minute slides: reports from the minute before still
@@ -423,10 +412,9 @@ address nginx passes on and no other. The image:
 
 - Listens on port 8080 by default (override with `PORT` env var)
 - Takes the client address from `X-Forwarded-For` only on requests from the
-  reverse proxies named in `TRUSTED_PROXIES`, by default from any address in
+  reverse proxies named in `TRUSTED_PROXIES`, and by default from none.
-  `10.0.0.0/8`, `172.16.0.0/12` or `192.168.0.0/16`. `bin/entrypoint.sh` checks
+  `bin/entrypoint.sh` checks each entry with `netwatch-server check-cidr`, which
-  each entry with `netwatch-server check-cidr`, which parses it as the backend
+  parses it as the backend parses its own `TRUSTED_PROXIES`
-  parses its own `TRUSTED_PROXIES`
 - Sends access logs to stdout
 - Caches static assets with immutable headers
 - Sends the security headers `REPO_POLICIES.md` requires on every response, as
@@ -445,9 +433,9 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs:
 
 - **Port:** container port `8080`.
 - **Volume:** container path `/data`; the reports are kept in `/data/reports`.
-- **Environment variables:** none is required. An empty one counts as unset,
+- **Environment variables:** none is required. An empty one counts as unset, and
-  except `TRUSTED_PROXIES`, and one set to a value netwatch cannot use stops the
+  one set to a value netwatch cannot use stops the container at start, with the
-  container at start, with the reason in its log.
+  reason in its log.
     - `PORT`, default `8080`: the container port, from 1 to 65535. `8081` cannot
       be used: the backend listens on it inside the container
     - `REPORTS_PER_MINUTE`, default `60`: reports each client address may send a
@@ -461,17 +449,16 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs:
       in: `/data` or a path below it, with no `.` or `..` part and no extra `/`.
       The container also stops if the path goes through a symbolic link that
       leads out of `/data` or is written as a full path
-    - `TRUSTED_PROXIES`, default `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`: the
+    - `TRUSTED_PROXIES`, default empty: set it to the address the reverse proxy
-      addresses the reverse proxy in front of the container connects from, as IP
+      in front of the container connects from, as an IP address or CIDR; several
-      addresses or CIDRs separated by commas. nginx takes the client address
+      are separated by commas. nginx takes the client address from
-      from `X-Forwarded-For` only on a request from one of them, and the rate
+      `X-Forwarded-For` only on a request from one of them, and the rate limit
-      limit counts that address. A list replaces the default. Set and empty,
+      counts that address. Unset, `X-Forwarded-For` is ignored and every client
-      `X-Forwarded-For` is ignored and every client behind the proxy shares the
+      behind the proxy shares the proxy's one allowance of `REPORTS_PER_MINUTE`.
-      proxy's one allowance of `REPORTS_PER_MINUTE`. Any client that connects
+      Name only addresses nothing but the proxy connects from: any client that
-      from one of these addresses can write its own `X-Forwarded-For`, and
+      connects from one can write its own `X-Forwarded-For`, and through a port
-      through a port Docker publishes, every client may connect from the Docker
+      Docker publishes, every client may connect from the Docker network's
-      network's gateway, such as `172.17.0.1`, which the default includes; where
+      gateway, such as `172.17.0.1`.
-      that matters, name only the addresses nothing but the proxy connects from.
     - `METRICS_USERNAME` and `METRICS_PASSWORD`, default empty: with both set,
       the backend records Prometheus metrics of its requests and serves them at
       `/metrics` on the container port, to requests with this user name and

TODO.md

@@ -21,43 +21,6 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
 
 # Completed Steps
 
-- 2026-10-14: stopping the server waits for a report file write already under
-  way, the periodic flush's or the one `Append` starts when 10 MiB are waiting,
-  before the final flush
-  ([#122](https://git.eeqj.de/sneak/netwatch/issues/122)). It used to return at
-  once, and the process exiting cut the write off, losing its reports. A write
-  under way that fails puts its reports back in memory, so the final flush
-  writes them; the stop fails, and the process exits non-zero, only if that
-  flush fails too. Two tests hold the periodic write and the one at 10 MiB open
-  and find the stop waiting until each is released; a third makes the held write
-  fail and finds its report written by the final flush. They run in a
-  `testing/synctest` bubble, so the periodic flush comes without waiting a
-  minute
-- 2026-10-14: `make test` no longer runs the frontend's production build, which
-  the tests do not need ([#118](https://git.eeqj.de/sneak/netwatch/issues/118)).
-  `yarn build` moved out of the stage that runs the frontend's unit tests, now
-  named `frontend-test`, into a new `frontend-build` stage, which starts from it
-  and, like the `builder` stage, waits on the lint and test phases, so the image
-  still cannot be built unless they pass. The runtime stage copies `dist/` from
-  it, and the `git` that `vite.config.js` reads the commit with is installed
-  there. No build cache was added
-- 2026-10-14: opening the debug log no longer raises the measured latencies
-  ([#131](https://git.eeqj.de/sneak/netwatch/issues/131)). With the panel open,
-  each line written drew the panel again with every line before it, up to 1000,
-  and a check that ends while the page draws is timed only when the drawing is
-  done. Each line is now added at the end of the open panel, its first line is
-  dropped once it holds more than 1000, and it scrolls to its newest line once a
-  frame rather than once a line; the whole panel is drawn only when it is
-  opened. A unit test writes more lines than the panel keeps and checks that the
-  lines already shown are left as they are
-- 2026-10-14: unset, the backend's own `TRUSTED_PROXIES` trusts `10.0.0.0/8`,
-  `172.16.0.0/12` and `192.168.0.0/16`, and no longer loopback; a list replaces
-  that; set and empty, it trusts no proxy, where it used to get the default, as
-  viper reads an empty environment variable as unset
-  ([#129](https://git.eeqj.de/sneak/netwatch/issues/129)). A value it cannot
-  parse still stops the start, and the image still starts the backend with
-  `TRUSTED_PROXIES=127.0.0.1/32`. Tests load the config with it unset, empty and
-  set to a list
 - 2026-10-14: while report files cannot be written, `POST /api/v1/reports`
   answers 500, and storing recovers once writing works again, with no restart
   ([#123](https://git.eeqj.de/sneak/netwatch/issues/123)). A failed write puts
@@ -69,41 +32,6 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
   others check that a report appended while a write runs is kept, behind that
   write's own, when the write fails, and that the cap still holds exactly once a
   failed write's reports are written
-- 2026-10-14: a request whose handler panics is logged with status 500 and
-  counted with `code` 500 in the metrics
-  ([#125](https://git.eeqj.de/sneak/netwatch/issues/125)), not 200. `Recoverer`
-  is still the first middleware; `Write500OnPanic`, after the request log and
-  after the metrics, writes the 500 as the panic passes and lets it go on to
-  Sentry and `Recoverer`, which now writes a 500 only when no status was written
-- 2026-10-14: with `TRUSTED_PROXIES` unset, the container's nginx trusts the
-  three private ranges of RFC 1918, `10.0.0.0/8`, `172.16.0.0/12` and
-  `192.168.0.0/16` ([#119](https://git.eeqj.de/sneak/netwatch/issues/119)), as
-  sneak ruled on 2026-09-22 for every app; it trusted no proxy before. A list
-  replaces them, and set and empty it trusts none. The backend inside the
-  container still trusts only `127.0.0.1/32`
-- 2026-10-14: `POST /api/v1/reports` takes only a report sent with
-  `Content-Type: application/json`, parameters such as `charset` allowed, and
-  refuses any other with 415 and the usual error body, storing nothing
-  ([#126](https://git.eeqj.de/sneak/netwatch/issues/126)). A page on another
-  origin can send a `text/plain` POST without a CORS preflight, so any site
-  could make its visitors' browsers store reports; a JSON POST waits for the
-  preflight, which only an origin in `CORS_ALLOWED_ORIGINS` passes. The page
-  already sent `application/json`. A test posts a report as `text/plain`, as
-  `application/x-www-form-urlencoded`, as `multipart/form-data`, with no
-  `Content-Type`, and as `application/json` with and without a charset, and
-  checks which are stored
-- 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/`,
@@ -523,9 +451,3 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
 - A configurable host list (an environment variable or a config file)
 - Export of the latency history (CSV or JSON)
 - A notification when the health status changes to DEGRADED
-- `make test` within the 20-second target
-  ([#118](https://git.eeqj.de/sneak/netwatch/issues/118)): every run compiles
-  the Go standard library and every dependency with the race detector from
-  nothing. Whether test builds may keep Go's compiled packages between runs
-  waits on sneak's ruling in
-  [prompts#113](https://git.eeqj.de/sneak/prompts/issues/113)

bin/entrypoint.sh

@@ -34,11 +34,9 @@ fi
 
 # TRUSTED_PROXIES names the reverse proxies in front of the container,
 # as IP addresses or CIDRs separated by commas. nginx takes the client
-# address from X-Forwarded-For only on a request from one of them.
+# address from X-Forwarded-For only on a request from one of them, so
-# Unset, they are the three private ranges of RFC 1918; set and empty,
+# unset or empty, it trusts no one. configs/nginx.conf includes the file
-# nginx trusts no one, which is why the default below follows - and not
+# written here, one set_real_ip_from line per entry.
-# :-. configs/nginx.conf includes the file written here, one
-# set_real_ip_from line per entry.
 #
 # nginx looks up an entry it cannot read as an address as a hostname,
 # and trusts what it finds (1.2.3 is found as 1.2.0.3). So each entry
@@ -47,7 +45,7 @@ fi
 # TRUSTED_PROXIES. Its error, naming the CIDR, is dropped for the one
 # below, naming the entry as written. set -f keeps a * in an entry from
 # becoming a list of file names.
-TRUSTED_PROXIES="${TRUSTED_PROXIES-10.0.0.0/8,172.16.0.0/12,192.168.0.0/16}"
+TRUSTED_PROXIES="${TRUSTED_PROXIES:-}"
 set -f
 for proxy in $(printf '%s' "$TRUSTED_PROXIES" | tr ',' ' '); do
     case "$proxy" in

configs/nginx.conf

@@ -18,10 +18,9 @@ server {
 
     # The client address comes from X-Forwarded-For only on a request
     # from the reverse proxies in TRUSTED_PROXIES: bin/entrypoint.sh
-    # writes one set_real_ip_from line for each into this file. Unset,
+    # writes one set_real_ip_from line for each into this file, and
-    # TRUSTED_PROXIES is the three private ranges of RFC 1918; set and
+    # leaves it empty when TRUSTED_PROXIES is unset, so that by default
-    # empty, the file is empty and the client address is the one each
+    # the client address is the one each request comes from.
-    # request comes from.
     include /etc/nginx/trusted-proxies.conf;
     real_ip_header X-Forwarded-For;
     real_ip_recursive on;

internal/config/config.go

@@ -9,7 +9,6 @@ import (
 	"math"
 	"net/netip"
 	"net/url"
-	"os"
 	"strconv"
 	"strings"
 
@@ -22,11 +21,13 @@ import (
 )
 
 // defaultTrustedProxies lists the networks whose forwarded
-// headers are honoured when TRUSTED_PROXIES is unset: the
+// headers are honoured by default: IPv4 and IPv6 loopback,
-// RFC 1918 ranges. The container image does not use it:
+// for a reverse proxy on the same host, and the RFC1918
+// ranges. The container image does not use it:
 // bin/entrypoint.sh gives the server 127.0.0.1/32, since
 // nginx is its only client there.
-const defaultTrustedProxies = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
+const defaultTrustedProxies = "127.0.0.1/32,::1/128," +
+	"10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
 
 // Default limits on stored reports; README.md gives the reasons for
 // these values.
@@ -134,14 +135,6 @@ func New(
 			viper.GetString("DEBUG"), errNotBool)
 	}
 
-	// An empty TRUSTED_PROXIES trusts no proxy, but viper reads an
-	// empty environment variable as unset and would give the default,
-	// so the environment is read first.
-	trustedProxies, set := os.LookupEnv("TRUSTED_PROXIES")
-	if !set {
-		trustedProxies = viper.GetString("TRUSTED_PROXIES")
-	}
-
 	s := &Config{
 		BindAddress:        viper.GetString("BIND_ADDRESS"),
 		CORSAllowedOrigins: splitList(viper.GetString("CORS_ALLOWED_ORIGINS")),
@@ -153,7 +146,7 @@ func New(
 		Port:               port,
 		ReportsPerMinute:   viper.GetInt("REPORTS_PER_MINUTE"),
 		SentryDSN:          viper.GetString("SENTRY_DSN"),
-		TrustedProxies:     splitList(trustedProxies),
+		TrustedProxies:     splitList(viper.GetString("TRUSTED_PROXIES")),
 		log:                log,
 		params:             &params,
 	}

internal/config/config_test.go

@@ -1,8 +1,6 @@
 package config_test
 
 import (
-	"os"
-	"slices"
 	"strings"
 	"testing"
 
@@ -31,62 +29,6 @@ func requireConfigError(t *testing.T, setting string) {
 	}
 }
 
-// requireTrustedProxies builds the config as main does and fails the
-// test unless its TrustedProxies are want.
-func requireTrustedProxies(t *testing.T, want []string) {
-	t.Helper()
-
-	var cfg *config.Config
-
-	app := fx.New(
-		fx.NopLogger,
-		fx.Provide(globals.New, logger.New, config.New),
-		fx.Populate(&cfg),
-	)
-
-	err := app.Err()
-	if err != nil {
-		t.Fatalf("config error = %v", err)
-	}
-
-	if !slices.Equal(cfg.TrustedProxies, want) {
-		t.Fatalf("TrustedProxies = %q, want %q", cfg.TrustedProxies, want)
-	}
-}
-
-// TestTrustedProxiesUnsetTrustsRFC1918: unset, the server trusts the
-// RFC 1918 ranges and nothing else, loopback included.
-func TestTrustedProxiesUnsetTrustsRFC1918(t *testing.T) {
-	// t.Setenv first, so the variable is put back as it was after the
-	// test.
-	t.Setenv("TRUSTED_PROXIES", "")
-
-	err := os.Unsetenv("TRUSTED_PROXIES")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	requireTrustedProxies(t,
-		[]string{"10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"})
-}
-
-// TestTrustedProxiesEmptyTrustsNothing: set and empty, the server
-// trusts no proxy. viper alone would read the empty variable as unset
-// and give the default.
-func TestTrustedProxiesEmptyTrustsNothing(t *testing.T) {
-	t.Setenv("TRUSTED_PROXIES", "")
-
-	requireTrustedProxies(t, []string{})
-}
-
-// TestTrustedProxiesListReplacesDefault: a list is used as given, with
-// none of the default ranges added.
-func TestTrustedProxiesListReplacesDefault(t *testing.T) {
-	t.Setenv("TRUSTED_PROXIES", "172.17.0.1/32, 2001:db8::/32")
-
-	requireTrustedProxies(t, []string{"172.17.0.1/32", "2001:db8::/32"})
-}
-
 // TestSettingsLoadAsGiven: valid values pass the checks and are used
 // as given. bin/entrypoint.sh starts the server with these
 // BIND_ADDRESS and PORT values.

internal/handlers/handlers.go

@@ -5,9 +5,7 @@ package handlers
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"log/slog"
-	"mime"
 	"net/http"
 
 	"sneak.berlin/go/netwatch/internal/globals"
@@ -20,10 +18,6 @@ import (
 
 const jsonContentType = "application/json; charset=utf-8"
 
-// errNotJSON is what decodeJSON returns for a request whose
-// Content-Type is not application/json.
-var errNotJSON = errors.New("content type is not application/json")
-
 // reportAppender is the subset of the report buffer the handlers
 // depend on. Defining it here keeps the storage failure path
 // exercisable with a stub in tests.
@@ -86,25 +80,14 @@ func (s *Handlers) respondJSON(
 	}
 }
 
-// decodeJSON decodes the request body into v. It returns errNotJSON,
+// decodeJSON decodes the request body into v. The body is
-// reading nothing, unless the request's Content-Type is
+// expected to already be bounded by the body-size middleware, so
-// application/json, with or without parameters such as charset. The
+// a caller can distinguish an over-limit body from malformed
-// body is expected to already be bounded by the body-size middleware,
+// JSON by testing the returned error for *http.MaxBytesError.
-// so a caller can distinguish an over-limit body from malformed JSON
-// by testing the returned error for *http.MaxBytesError.
 func (s *Handlers) decodeJSON(
 	_ http.ResponseWriter,
 	r *http.Request,
 	v any,
 ) error {
-	// A page on another origin can POST text/plain or a form without
-	// a CORS preflight, so its visitors' browsers would send it
-	// whatever the CORS settings. A JSON POST waits for the
-	// preflight, so only JSON is taken.
-	mediaType, _, err := mime.ParseMediaType(r.Header.Get("Content-Type"))
-	if err != nil || mediaType != "application/json" {
-		return errNotJSON
-	}
-
 	return json.NewDecoder(r.Body).Decode(v)
 }

internal/handlers/report.go

@@ -30,7 +30,7 @@ type report struct {
 }
 
 // HandleReport returns a handler that accepts telemetry
-// reports from NetWatch clients, sent as application/json.
+// reports from NetWatch clients.
 func (s *Handlers) HandleReport() http.HandlerFunc {
 	type response struct {
 		Status string `json:"status"`
@@ -66,15 +66,9 @@ func (s *Handlers) HandleReport() http.HandlerFunc {
 }
 
 // decodeErrorStatus logs a report decode failure and returns the
-// status to send: 415 when the report was not sent as JSON, 413 when
+// status to send: 413 when the body exceeded the size limit,
-// the body exceeded the size limit, otherwise 400 for malformed JSON.
+// otherwise 400 for malformed JSON.
 func (s *Handlers) decodeErrorStatus(err error) int {
-	if errors.Is(err, errNotJSON) {
-		s.log.Warn("report refused: Content-Type is not application/json")
-
-		return http.StatusUnsupportedMediaType
-	}
-
 	var tooLarge *http.MaxBytesError
 	if errors.As(err, &tooLarge) {
 		s.log.Warn("report body too large", "limit_bytes", tooLarge.Limit)

internal/handlers/report_test.go

@@ -27,17 +27,6 @@ type stubAppender struct {
 
 func (s stubAppender) Append(any) error { return s.err }
 
-// recordingAppender keeps every report the handler stores.
-type recordingAppender struct {
-	reports []any
-}
-
-func (a *recordingAppender) Append(v any) error {
-	a.reports = append(a.reports, v)
-
-	return nil
-}
-
 func newTestHandlers(buf stubAppender, out io.Writer) *handlers.Handlers {
 	return handlers.NewForTest(buf, slog.New(slog.NewJSONHandler(out, nil)))
 }
@@ -94,7 +83,6 @@ func TestHandleReportAcceptsValidReports(t *testing.T) {
 				http.MethodPost, "/api/v1/reports",
 				strings.NewReader(tt.body),
 			)
-			req.Header.Set("Content-Type", "application/json")
 
 			h.HandleReport().ServeHTTP(rec, req)
 
@@ -115,89 +103,6 @@ func TestHandleReportAcceptsValidReports(t *testing.T) {
 	}
 }
 
-// TestHandleReportStoresOnlyJSON checks that a report is stored only
-// when sent as application/json, as the page sends it. One sent as
-// text/plain or as a form, which a page on another origin can send
-// without a CORS preflight, is refused with 415.
-func TestHandleReportStoresOnlyJSON(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name        string
-		contentType string
-		wantStatus  int
-		wantStored  int
-	}{
-		{
-			name:        "text/plain",
-			contentType: "text/plain;charset=UTF-8",
-			wantStatus:  http.StatusUnsupportedMediaType,
-			wantStored:  0,
-		},
-		{
-			name:        "a urlencoded form",
-			contentType: "application/x-www-form-urlencoded",
-			wantStatus:  http.StatusUnsupportedMediaType,
-			wantStored:  0,
-		},
-		{
-			name:        "a multipart form",
-			contentType: "multipart/form-data; boundary=x",
-			wantStatus:  http.StatusUnsupportedMediaType,
-			wantStored:  0,
-		},
-		{
-			name:        "no Content-Type",
-			contentType: "",
-			wantStatus:  http.StatusUnsupportedMediaType,
-			wantStored:  0,
-		},
-		{
-			name:        "application/json",
-			contentType: "application/json",
-			wantStatus:  http.StatusOK,
-			wantStored:  1,
-		},
-		{
-			name:        "application/json with a charset",
-			contentType: "application/json;charset=UTF-8",
-			wantStatus:  http.StatusOK,
-			wantStored:  1,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			var stored recordingAppender
-
-			h := handlers.NewForTest(&stored, slog.New(slog.DiscardHandler))
-
-			rec := httptest.NewRecorder()
-			req := httptest.NewRequestWithContext(t.Context(),
-				http.MethodPost, "/api/v1/reports",
-				strings.NewReader(`{"clientId":"c1","hosts":[]}`),
-			)
-
-			if tt.contentType != "" {
-				req.Header.Set("Content-Type", tt.contentType)
-			}
-
-			h.HandleReport().ServeHTTP(rec, req)
-
-			if rec.Code != tt.wantStatus {
-				t.Errorf("status = %d, want %d", rec.Code, tt.wantStatus)
-			}
-
-			if len(stored.reports) != tt.wantStored {
-				t.Errorf("reports stored = %d, want %d",
-					len(stored.reports), tt.wantStored)
-			}
-		})
-	}
-}
-
 func TestHandleReportStorageFailureIsNon2xx(t *testing.T) {
 	t.Parallel()
 
@@ -208,7 +113,6 @@ func TestHandleReportStorageFailureIsNon2xx(t *testing.T) {
 		http.MethodPost, "/api/v1/reports",
 		strings.NewReader(`{"clientId":"c1","hosts":[]}`),
 	)
-	req.Header.Set("Content-Type", "application/json")
 
 	h.HandleReport().ServeHTTP(rec, req)
 
@@ -234,7 +138,6 @@ func TestHandleReportFullIs507(t *testing.T) {
 		http.MethodPost, "/api/v1/reports",
 		strings.NewReader(`{"clientId":"c1","hosts":[]}`),
 	)
-	req.Header.Set("Content-Type", "application/json")
 
 	h.HandleReport().ServeHTTP(rec, req)
 
@@ -258,7 +161,6 @@ func TestHandleReportMalformedJSONIs400(t *testing.T) {
 		http.MethodPost, "/api/v1/reports",
 		strings.NewReader(`{not json`),
 	)
-	req.Header.Set("Content-Type", "application/json")
 
 	h.HandleReport().ServeHTTP(rec, req)
 
@@ -283,8 +185,6 @@ func TestHandleReportOversizeIs413(t *testing.T) {
 		http.MethodPost, "/api/v1/reports",
 		strings.NewReader(`{"clientId":"`+strings.Repeat("x", 200)+`"}`),
 	)
-	req.Header.Set("Content-Type", "application/json")
-
 	// No declared length, so only the middleware's read cap can
 	// stop this body.
 	req.ContentLength = -1
@@ -313,7 +213,6 @@ func TestHandleReportDoesNotLogRawGeo(t *testing.T) {
 			`{"clientId":"c1","geo":{"raw":"`+sentinel+`"},"hosts":[]}`,
 		),
 	)
-	req.Header.Set("Content-Type", "application/json")
 
 	h.HandleReport().ServeHTTP(rec, req)
 
@@ -346,7 +245,6 @@ func TestHandleReportLogsClientIDCutToBound(t *testing.T) {
 			`{"clientId":"`+long+`","timestamp":"`+long+`","hosts":[]}`,
 		),
 	)
-	req.Header.Set("Content-Type", "application/json")
 
 	h.HandleReport().ServeHTTP(rec, req)
 
@@ -386,7 +284,6 @@ func TestHandleReportDecodeErrorLogIsBounded(t *testing.T) {
 		http.MethodPost, "/api/v1/reports",
 		strings.NewReader(`{"hosts":[{"history":[{"t":`+huge+`}]}]}`),
 	)
-	req.Header.Set("Content-Type", "application/json")
 
 	h.HandleReport().ServeHTTP(rec, req)
 

internal/middleware/middleware.go

@@ -297,16 +297,12 @@ func (s *Middleware) MaxBodyBytes(
 
 // Recoverer returns middleware that recovers from a panic in a
 // downstream handler, logs the panic and stack trace through
-// slog, and responds 500 with no body, unless a status was already
+// slog, and responds 500 with no body. http.ErrAbortHandler is
-// written, as Write500OnPanic writes one for a handler's panic.
+// re-panicked so the server can abort the response as intended.
-// http.ErrAbortHandler is re-panicked so the server can abort the
-// response as intended.
 func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(
 			func(w http.ResponseWriter, r *http.Request) {
-				ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor)
-
 				defer func() {
 					rec := recover()
 					if rec == nil {
@@ -324,43 +320,10 @@ func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
 						"stack", string(debug.Stack()),
 					)
 
-					if ww.Status() == 0 {
+					w.WriteHeader(http.StatusInternalServerError)
-						ww.WriteHeader(http.StatusInternalServerError)
-					}
 				}()
 
-				next.ServeHTTP(ww, r)
+				next.ServeHTTP(w, r)
-			},
-		)
-	}
-}
-
-// Write500OnPanic returns middleware that writes 500 when a downstream
-// handler panics before writing a status, and lets the panic go on to
-// Sentry and Recoverer. Recoverer, which comes first, would write its
-// 500 only after the request log and the metrics had recorded the
-// status they saw; placed after each of them, this makes them record
-// the 500.
-func (s *Middleware) Write500OnPanic() func(http.Handler) http.Handler {
-	return func(next http.Handler) http.Handler {
-		return http.HandlerFunc(
-			func(w http.ResponseWriter, r *http.Request) {
-				ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor)
-
-				defer func() {
-					rec := recover()
-					if rec == nil {
-						return
-					}
-
-					if ww.Status() == 0 {
-						ww.WriteHeader(http.StatusInternalServerError)
-					}
-
-					panic(rec)
-				}()
-
-				next.ServeHTTP(ww, r)
 			},
 		)
 	}

internal/middleware/middleware_test.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"errors"
-	"log"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
@@ -17,9 +16,7 @@ import (
 	"sneak.berlin/go/netwatch/internal/logger"
 	"sneak.berlin/go/netwatch/internal/middleware"
 
-	"github.com/go-chi/chi/v5"
 	chimiddleware "github.com/go-chi/chi/v5/middleware"
-	"github.com/prometheus/client_golang/prometheus"
 )
 
 const (
@@ -326,172 +323,6 @@ func TestRecovererRepanicsOnAbortHandler(t *testing.T) {
 	}
 }
 
-// TestHandlerPanicIsLoggedAndCountedAs500 sends a request to a handler
-// that panics through the middleware SetupRoutes puts around a route
-// with metrics, in its order, leaving out those that write no status
-// for a panic and Sentry, which TestSentry covers. The request log line
-// and the metrics must give the 500 the client gets, and the panic must
-// still reach Recoverer, which logs it.
-func TestHandlerPanicIsLoggedAndCountedAs500(t *testing.T) {
-	t.Parallel()
-
-	var logbuf bytes.Buffer
-
-	mw := middleware.NewWithLogger(
-		slog.New(slog.NewJSONHandler(&logbuf, nil)),
-	)
-	registry := prometheus.NewRegistry()
-
-	handler := chi.Chain(
-		mw.Recoverer(),
-		chimiddleware.RequestID,
-		mw.Logging(),
-		mw.Write500OnPanic(),
-		mw.Metrics(registry),
-		mw.Write500OnPanic(),
-	).HandlerFunc(func(http.ResponseWriter, *http.Request) {
-		panic("boom")
-	})
-
-	rec := httptest.NewRecorder()
-	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", http.NoBody)
-	handler.ServeHTTP(rec, req)
-
-	if rec.Code != http.StatusInternalServerError {
-		t.Fatalf("status = %d, want %d",
-			rec.Code, http.StatusInternalServerError)
-	}
-
-	request := findLogLine(t, logbuf.Bytes(), "request")
-	if request.Status != http.StatusInternalServerError {
-		t.Errorf("request log status = %d, want %d",
-			request.Status, http.StatusInternalServerError)
-	}
-
-	recovered := findLogLine(t, logbuf.Bytes(), "panic recovered")
-	if recovered.Panic != "boom" {
-		t.Errorf("panic recovered line has panic %q, want %q",
-			recovered.Panic, "boom")
-	}
-
-	for _, name := range []string{
-		"http_request_duration_seconds", "http_response_size_bytes",
-	} {
-		codes := metricCodes(t, registry, name)
-		if len(codes) != 1 || codes[0] != "500" {
-			t.Errorf("%s code labels = %v, want [500]", name, codes)
-		}
-	}
-}
-
-// loggedLine holds the fields the tests read from a JSON log line.
-type loggedLine struct {
-	Msg    string `json:"msg"`
-	Status int    `json:"status"`
-	Panic  string `json:"panic"`
-}
-
-// findLogLine returns the first of the JSON log lines in logged whose
-// message is msg.
-func findLogLine(t *testing.T, logged []byte, msg string) loggedLine {
-	t.Helper()
-
-	dec := json.NewDecoder(bytes.NewReader(logged))
-
-	for {
-		var line loggedLine
-
-		err := dec.Decode(&line)
-		if err != nil {
-			t.Fatalf("no %q line in the log: %v (%q)", msg, err, logged)
-		}
-
-		if line.Msg == msg {
-			return line
-		}
-	}
-}
-
-// metricCodes returns the code label of each series of the metric
-// family name in registry.
-func metricCodes(
-	t *testing.T,
-	registry *prometheus.Registry,
-	name string,
-) []string {
-	t.Helper()
-
-	families, err := registry.Gather()
-	if err != nil {
-		t.Fatalf("gather metrics: %v", err)
-	}
-
-	var codes []string
-
-	for _, family := range families {
-		if family.GetName() != name {
-			continue
-		}
-
-		for _, metric := range family.GetMetric() {
-			for _, label := range metric.GetLabel() {
-				if label.GetName() == "code" {
-					codes = append(codes, label.GetValue())
-				}
-			}
-		}
-	}
-
-	return codes
-}
-
-// TestRecovererWritesNoSecondStatus checks, on a real server, that
-// Recoverer does not write its 500 again after Write500OnPanic has
-// written one, which the server would log as a superfluous
-// WriteHeader call.
-func TestRecovererWritesNoSecondStatus(t *testing.T) {
-	t.Parallel()
-
-	var serverLog bytes.Buffer
-
-	mw := middleware.NewWithLogger(slog.New(slog.DiscardHandler))
-
-	server := httptest.NewUnstartedServer(chi.Chain(
-		mw.Recoverer(),
-		mw.Write500OnPanic(),
-	).HandlerFunc(func(http.ResponseWriter, *http.Request) {
-		panic("boom")
-	}))
-	server.Config.ErrorLog = log.New(&serverLog, "", 0)
-	server.Start()
-
-	req, err := http.NewRequestWithContext(t.Context(),
-		http.MethodGet, server.URL, http.NoBody)
-	if err != nil {
-		t.Fatalf("new request: %v", err)
-	}
-
-	resp, err := server.Client().Do(req)
-	if err != nil {
-		t.Fatalf("request: %v", err)
-	}
-
-	_ = resp.Body.Close()
-
-	// Close waits for the handler to return, so whatever the server
-	// logs for this request is in serverLog after it.
-	server.Close()
-
-	if resp.StatusCode != http.StatusInternalServerError {
-		t.Errorf("status = %d, want %d",
-			resp.StatusCode, http.StatusInternalServerError)
-	}
-
-	if serverLog.Len() != 0 {
-		t.Errorf("server logged %q", serverLog.String())
-	}
-}
-
 // TestLoggingCutsRequestStringsToBound sends an over-long URL and
 // over-long header values, and checks the request log writes each
 // one cut to logger.MaxLoggedFieldBytes.

internal/reportbuf/reportbuf.go

@@ -96,10 +96,6 @@ type Buffer struct {
 	// refuses reports, and the reports of that write wait in buf for
 	// the next flush.
 	writeErr error
-	// writers are the goroutines that write report files: flushLoop,
-	// and each one Append starts at flushSizeThreshold. OnStop waits
-	// for them before the final flush.
-	writers sync.WaitGroup
 }
 
 // New creates a Buffer and registers lifecycle hooks to
@@ -151,7 +147,7 @@ func New(
 
 			b.mu.Unlock()
 
-			b.writers.Go(b.flushLoop)
+			go b.flushLoop()
 
 			return nil
 		},
@@ -162,14 +158,7 @@ func New(
 			var err error
 
 			b.stopOnce.Do(func() {
-				// Closing done ends flushLoop after any flush it is
-				// running. Wait for it and for the writes Append
-				// started, which the process exiting would cut off;
-				// one that fails puts its reports back in buf, for the
-				// final flush to write.
 				close(b.done)
-				b.writers.Wait()
-
 				err = b.flushLocked()
 			})
 
@@ -222,12 +211,12 @@ func (b *Buffer) Append(v any) error {
 		data := b.drainBuf()
 		b.mu.Unlock()
 
-		b.writers.Go(func() {
+		go func() {
 			writeErr := b.writeFile(data)
 			if writeErr != nil {
 				b.log.Error("flush reports failed", "error", writeErr)
 			}
-		})
+		}()
 
 		return nil
 	}

internal/reportbuf/reportbuf_test.go

@@ -15,7 +15,6 @@ import (
 	"sync"
 	"sync/atomic"
 	"testing"
-	"testing/synctest"
 	"time"
 
 	"sneak.berlin/go/netwatch/internal/config"
@@ -108,174 +107,9 @@ func TestFailedFinalFlushFailsStop(t *testing.T) {
 	}
 }
 
-// TestStopWaitsForPeriodicWrite holds open the write the periodic flush
+// startBuffer starts a Buffer through fx, as main does, with the
-// starts, then stops the app. The stop must wait until the write is
+// DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set.
-// released, and the report must then be in its file. The test runs in
+func startBuffer(t *testing.T) *reportbuf.Buffer {
-// a synctest bubble, whose clock moves on at once when every goroutine
-// in it is waiting, so the periodic flush does not take a minute.
-func TestStopWaitsForPeriodicWrite(t *testing.T) {
-	dir := t.TempDir()
-	t.Setenv("DATA_DIR", dir)
-
-	synctest.Test(t, func(t *testing.T) {
-		buf, app := startApp(t)
-
-		created := make(chan struct{})
-		release := make(chan struct{})
-
-		buf.OnFileCreated(func(*os.File) {
-			created <- struct{}{}
-
-			<-release
-		})
-
-		err := buf.Append(map[string]string{"id": "periodic"})
-		if err != nil {
-			t.Fatalf("append report: %v", err)
-		}
-
-		<-created
-
-		err = stopWhileHeld(t, app, release)
-		if err != nil {
-			t.Fatalf("stop: %v", err)
-		}
-
-		files, err := readReportFiles(dir)
-		if err != nil {
-			t.Fatalf("read report files: %v", err)
-		}
-
-		want := `{"id":"periodic"}` + "\n"
-		if len(files) != 1 || files[0] != want {
-			t.Fatalf("report files = %q, want one holding %q", files, want)
-		}
-	})
-}
-
-// TestStopWaitsForWriteAtSizeThreshold holds open the write Append
-// starts when the buffer reaches FlushSizeThreshold, then stops the
-// app. The stop must wait until the write is released, and the reports
-// must then be in its file.
-func TestStopWaitsForWriteAtSizeThreshold(t *testing.T) {
-	dir := t.TempDir()
-	t.Setenv("DATA_DIR", dir)
-
-	synctest.Test(t, func(t *testing.T) {
-		buf, app := startApp(t)
-
-		created := make(chan struct{})
-		release := make(chan struct{})
-
-		buf.OnFileCreated(func(*os.File) {
-			created <- struct{}{}
-
-			<-release
-		})
-
-		appended := appendToThreshold(t, buf)
-
-		<-created
-
-		err := stopWhileHeld(t, app, release)
-		if err != nil {
-			t.Fatalf("stop: %v", err)
-		}
-
-		files, err := readReportFiles(dir)
-		if err != nil {
-			t.Fatalf("read report files: %v", err)
-		}
-
-		if len(files) != 1 || files[0] != appended {
-			t.Fatalf("%d report files, want one holding the %d bytes "+
-				"appended", len(files), len(appended))
-		}
-	})
-}
-
-// TestFinalFlushWritesReportsOfFailedWrite holds open the write the
-// periodic flush starts, makes it fail, then stops the app. The stop
-// waits for the write, which puts its report back in the buffer, and
-// the final flush writes it, so the stop succeeds with nothing lost.
-func TestFinalFlushWritesReportsOfFailedWrite(t *testing.T) {
-	dir := t.TempDir()
-	t.Setenv("DATA_DIR", dir)
-
-	synctest.Test(t, func(t *testing.T) {
-		buf, app := startApp(t)
-
-		created := make(chan *os.File)
-		release := make(chan struct{})
-
-		buf.OnFileCreated(func(f *os.File) {
-			created <- f
-
-			<-release
-		})
-
-		err := buf.Append(map[string]string{"id": "failed"})
-		if err != nil {
-			t.Fatalf("append report: %v", err)
-		}
-
-		held := <-created
-
-		// Closing the file under the write makes the write fail.
-		_ = held.Close()
-
-		// The final flush's write goes through.
-		buf.OnFileCreated(func(*os.File) {})
-
-		err = stopWhileHeld(t, app, release)
-		if err != nil {
-			t.Fatalf("stop: %v", err)
-		}
-
-		files, err := readReportFiles(dir)
-		if err != nil {
-			t.Fatalf("read report files: %v", err)
-		}
-
-		want := `{"id":"failed"}` + "\n"
-		if len(files) != 1 || files[0] != want {
-			t.Fatalf("report files = %q, want one holding %q", files, want)
-		}
-	})
-}
-
-// stopWhileHeld stops app while a report file write is held open until
-// release is closed, and fails the test if the stop returns before
-// that. It then closes release and returns the stop's error. It must
-// be called in a synctest bubble.
-func stopWhileHeld(
-	t *testing.T,
-	app *fxtest.App,
-	release chan<- struct{},
-) error {
-	t.Helper()
-
-	stopped := make(chan error, 1)
-
-	go func() { stopped <- app.Stop(t.Context()) }()
-
-	// Wait returns once every other goroutine in the bubble is waiting:
-	// the stop has got as far as it can while the write is held.
-	synctest.Wait()
-
-	if len(stopped) != 0 {
-		t.Error("stop returned while a report file write was held open")
-	}
-
-	close(release)
-
-	return <-stopped
-}
-
-// startApp starts a Buffer through fx, as main does, with the DATA_DIR
-// and DATA_DIR_MAX_BYTES the calling test has set, and returns it with
-// the app, for tests that stop the app themselves.
-func startApp(t *testing.T) (*reportbuf.Buffer, *fxtest.App) {
 	t.Helper()
 
 	var buf *reportbuf.Buffer
@@ -293,16 +127,6 @@ func startApp(t *testing.T) (*reportbuf.Buffer, *fxtest.App) {
 	app.RequireStart()
 	t.Cleanup(app.RequireStop)
 
-	return buf, app
-}
-
-// startBuffer starts a Buffer through fx, as main does, with the
-// DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set.
-func startBuffer(t *testing.T) *reportbuf.Buffer {
-	t.Helper()
-
-	buf, _ := startApp(t)
-
 	return buf
 }
 
@@ -931,7 +755,6 @@ func postReport(t *testing.T, h *handlers.Handlers, clientID string) int {
 		http.MethodPost, "/api/v1/reports",
 		strings.NewReader(`{"clientId":"`+clientID+`","hosts":[]}`),
 	)
-	req.Header.Set("Content-Type", "application/json")
 	h.HandleReport().ServeHTTP(rec, req)
 
 	return rec.Code
@@ -1068,32 +891,6 @@ func TestFlushAtSizeThreshold(t *testing.T) {
 
 	buf := startBuffer(t)
 
-	appended := appendToThreshold(t, buf)
-
-	// Append writes the file in the background, so wait for it.
-	deadline := time.Now().Add(10 * time.Second)
-
-	for {
-		files, err := readReportFiles(dir)
-		if err == nil && len(files) == 1 && files[0] == appended {
-			return
-		}
-
-		if time.Now().After(deadline) {
-			t.Fatalf("%d report files (error: %v), want one holding the "+
-				"%d bytes appended", len(files), err, len(appended))
-		}
-
-		time.Sleep(10 * time.Millisecond)
-	}
-}
-
-// appendToThreshold appends reports to buf until it holds
-// FlushSizeThreshold bytes, so that the last append starts writing them
-// to a report file, and returns the lines appended.
-func appendToThreshold(t *testing.T, buf *reportbuf.Buffer) string {
-	t.Helper()
-
 	// Large reports, so the threshold takes a few hundred appends.
 	pad := strings.Repeat("a", 64<<10)
 
@@ -1116,7 +913,22 @@ func appendToThreshold(t *testing.T, buf *reportbuf.Buffer) string {
 		appended.WriteByte('\n')
 	}
 
-	return appended.String()
+	// Append writes the file in the background, so wait for it.
+	deadline := time.Now().Add(10 * time.Second)
+
+	for {
+		files, err := readReportFiles(dir)
+		if err == nil && len(files) == 1 && files[0] == appended.String() {
+			return
+		}
+
+		if time.Now().After(deadline) {
+			t.Fatalf("%d report files (error: %v), want one holding the "+
+				"%d bytes appended", len(files), err, appended.Len())
+		}
+
+		time.Sleep(10 * time.Millisecond)
+	}
 }
 
 // reportFilesBytes returns the total size of the report files in dir.

internal/server/routes.go

@@ -34,16 +34,13 @@ func (s *Server) SetupRoutes() {
 	s.router.Use(s.mw.Recoverer())
 	s.router.Use(middleware.RequestID)
 	s.router.Use(s.mw.Logging())
-	// s.mw.Recoverer would write a panic's 500 only after the panic has
-	// passed the request log, so it is written here, where the log sees it.
-	s.router.Use(s.mw.Write500OnPanic())
 	s.router.Use(s.mw.SecurityHeaders())
 	s.router.Use(s.mw.CORS(s.params.Config.CORSAllowedOrigins))
 	s.router.Use(s.mw.MaxBodyBytes(maxRequestBodyBytes))
 	s.router.Use(middleware.Timeout(requestTimeout))
 
-	// Sentry reports a panic, then panics again, so that it still
+	// Sentry reports a panic, then panics again, so that s.mw.Recoverer
-	// reaches s.mw.Recoverer.
+	// still answers 500.
 	if s.params.Config.SentryDSN != "" {
 		s.router.Use(sentryhttp.New(sentryhttp.Options{Repanic: true}).Handle)
 	}
@@ -67,8 +64,6 @@ func (s *Server) SetupRoutes() {
 		// other.
 		if s.params.Config.MetricsUsername != "" {
 			r.Use(s.mw.Metrics(registry))
-			// Likewise inside the metrics.
-			r.Use(s.mw.Write500OnPanic())
 		}
 
 		r.Get("/.well-known/healthcheck", s.h.HandleHealthCheck())

internal/server/routes_test.go

@@ -66,7 +66,6 @@ func TestReportsAreRateLimited(t *testing.T) {
 			http.MethodPost, "/api/v1/reports",
 			strings.NewReader(`{"clientId":"c1","hosts":[]}`),
 		)
-		req.Header.Set("Content-Type", "application/json")
 		srv.ServeHTTP(rec, req)
 
 		return rec.Code

script/frontend-test

@@ -2,10 +2,9 @@
 # script/frontend-test: run the frontend test suite on the host: the
 # unit tests in test/unit/, through the test script in package.json,
 # then the production build, which fails on broken code. make test runs
-# the same tests in the frontend-test stage of Dockerfile, and the image
+# the same in the frontend stage of Dockerfile. The tests print a dot
-# build runs the production build in its frontend-build stage. The tests
+# each; if any fails, they run again with every test listed, and the
-# print a dot each; if any fails, they run again with every test listed,
+# script fails even if that run passes. NODE_OPTIONS chooses the
-# and the script fails even if that run passes. NODE_OPTIONS chooses the
 # reporter because yarn adds its arguments after the test files, where
 # node would take a reporter option for one more file.
 set -eu

script/frontend-viewport-test

@@ -64,8 +64,8 @@ main() {
     # configs/nginx.conf is a template: the image renders it over its own
     # default.conf, with the same port and limit bin/entrypoint.sh uses.
     # The empty file it includes trusts no proxy, as bin/entrypoint.sh
-    # writes it when TRUSTED_PROXIES is set and empty. It also includes
+    # writes it when TRUSTED_PROXIES is unset. It also includes the
-    # the security headers, so the page runs under the shipped policy.
+    # security headers, so the page runs under the shipped policy.
     docker run -d --rm --name "$SERVER" \
         --network "$NETWORK" --network-alias netwatch \
         -e PORT=8080 -e NGINX_ENVSUBST_FILTER='^PORT$' \

test/unit/main.test.js

@@ -13,22 +13,19 @@ import {
     humanDuration,
     latencyClass,
     latencyHex,
-    log,
     measureLatency,
-    Reporter,
     tick,
 } from "../../web/src/main.js";
 
 // There is no page here, so the tests stand in for it. The debug log looks
-// for its panel by id and finds none, an element the page creates is a
+// for its panel by id and finds none. Each element of a host's row that
-// plain object, and no frame the page asks for ever comes. Each element of
+// tick or greyOutUI draws into is a plain object, made the first time a
-// a host's row that tick or greyOutUI draws into is a plain object, made
+// test looks it up and kept in elements under its selector until the next
-// the first time a test looks it up and kept in elements under its
+// test starts. As on a page, writing its text replaces its markup; the
-// selector until the next test starts. As on a page, writing its text
+// status dot greyOutUI looks for in it is not there. Drawing a sparkline
-// replaces its markup; the status dot greyOutUI looks for in it is not
+// does nothing; it looks for the pixel ratio on window and finds none. In
-// there. Drawing a sparkline does nothing; it looks for the pixel ratio on
+// each test, console.error and console.info print nothing and keep what
-// window and finds none. In each test, console.error and console.info
+// they are given.
-// print nothing and keep what they are given.
 const doNothing = () => {};
 let elements;
 beforeEach((t) => {
@@ -48,11 +45,8 @@ const canvasContext = {
     arc: doNothing,
 };
 globalThis.window = {};
-globalThis.requestAnimationFrame = doNothing;
-globalThis.cancelAnimationFrame = doNothing;
 globalThis.document = {
     getElementById: () => null,
-    createElement: () => ({}),
     querySelector: (selector) =>
         (elements[selector] ??= {
             getContext: () => canvasContext,
@@ -384,131 +378,6 @@ 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);
-});
-
-// panel stands in for the open debug log panel, and for the element in it
-// that holds its lines, whichever the page looks up by id; the lines are
-// kept in order in lines. Drawing the whole panel, as opening it does,
-// replaces every line with a new one.
-test("with the debug log open, writing more lines than it keeps adds each to the panel and leaves the lines already shown as they are", (t) => {
-    const lines = [];
-    const panel = {
-        classList: { contains: () => false },
-        children: lines,
-        get firstElementChild() {
-            return lines[0];
-        },
-        append: (line) => lines.push(line),
-        removeChild: (line) => lines.splice(lines.indexOf(line), 1),
-        replaceChildren: (...newLines) =>
-            lines.splice(0, lines.length, ...newLines),
-    };
-    t.mock.method(globalThis.document, "getElementById", () => panel);
-    const kept = CONFIG.maxLogEntries;
-
-    for (let i = 1; i <= kept; i++) log.info(`line ${i}`);
-    const shown = [...lines];
-    for (let i = kept + 1; i <= kept + 5; i++) log.info(`line ${i}`);
-
-    assert.deepEqual(
-        lines.map((line) => line.textContent.match(/line \d+$/)?.[0]),
-        Array.from({ length: kept }, (_, i) => `line ${i + 6}`),
-    );
-    // The first five lines shown were dropped, and the rest are the same
-    // lines, not drawn again.
-    shown.slice(5).forEach((line, i) => assert.equal(lines[i], line));
-});
-
 // The page shows &lt; &gt; &quot; &amp; and &#39; in a row's markup as
 // < > " & and '.
 test(`a target whose name and URL hold < > " & and ' shows those characters in its row`, () => {

web/src/main.js

@@ -144,15 +144,13 @@ const WAN_HOSTS = [
 
 const debugLog = [];
 
-export const log = (() => {
+const log = (() => {
     function append(level, message) {
-        const entry = { timestamp: new Date(), level, message };
+        debugLog.push({ timestamp: new Date(), level, message });
-        debugLog.push(entry);
         if (debugLog.length > CONFIG.maxLogEntries)
             debugLog.splice(0, debugLog.length - CONFIG.maxLogEntries);
         const panel = document.getElementById("debug-panel");
-        if (panel && !panel.classList.contains("hidden"))
+        if (panel && !panel.classList.contains("hidden")) renderDebugLog();
-            appendDebugLogLine(entry);
     }
     return {
         error: (msg) => append("error", msg),
@@ -259,14 +257,6 @@ 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;
@@ -275,10 +265,6 @@ 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;
@@ -294,7 +280,9 @@ export class HostState {
         });
         this._trim();
         this.lastLatency = result.latency;
-        this.status = statusFromError(result.error);
+        if (result.error === "timeout") this.status = "error";
+        else if (result.error) this.status = "offline";
+        else this.status = "online";
         this.consecutiveFailures = result.error
             ? this.consecutiveFailures + 1
             : 0;
@@ -305,18 +293,6 @@ 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() {
@@ -453,12 +429,9 @@ 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, clearedHistory, history }, `since` maps a host url
+// of { name, url, status, history }, `since` maps a host url to the Unix-ms
-// to the Unix-ms timestamp of the last sample already reported for it, and
+// timestamp of the last sample already reported for it, and `now` is a Date.
-// `now` is a Date. Only non-paused samples newer than the mark are included,
+// Only non-paused samples newer than the mark are included. Returns null
-// 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 = [];
@@ -467,7 +440,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.clearedHistory, ...host.history]) {
+        for (const p of host.history) {
             if (p.paused) continue;
             if (p.timestamp <= mark) continue;
             samples.push({
@@ -481,10 +454,7 @@ export function buildReport(hosts, clientId, now, since) {
         reportHosts.push({
             name: host.name,
             url: host.url,
-            status:
+            status: host.status,
-                host.status === "pending"
-                    ? statusFromError(samples.at(-1).error)
-                    : host.status,
             history: samples,
         });
         marks.set(host.url, high);
@@ -511,7 +481,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.
-export class Reporter {
+class Reporter {
     constructor(state, clientId, intervalMs) {
         this.state = state;
         this.clientId = clientId;
@@ -1109,8 +1079,9 @@ function updateHealthBox(state) {
 
 // --- Debug Log Renderer ------------------------------------------------------
 
-// One line of the debug log panel, its message set as text.
+function renderDebugLog() {
-function debugLogLine(entry) {
+    const el = document.getElementById("debug-log");
+    if (!el) return;
     const levelColors = {
         error: "text-red-400",
         warning: "text-orange-400",
@@ -1118,43 +1089,19 @@ function debugLogLine(entry) {
         info: "text-gray-300",
         debug: "text-gray-500",
     };
-    const ts = formatUTCTimestamp(entry.timestamp);
+    el.replaceChildren(
-    const lvl = entry.level.toUpperCase().padEnd(7);
+        ...debugLog.map((entry) => {
-    const line = document.createElement("div");
+            const ts = formatUTCTimestamp(entry.timestamp);
-    line.className = levelColors[entry.level] || "text-gray-400";
+            const lvl = entry.level.toUpperCase().padEnd(7);
-    line.textContent = `${ts} ${lvl} ${entry.message}`;
+            const line = document.createElement("div");
-    return line;
+            line.className = levelColors[entry.level] || "text-gray-400";
-}
+            line.textContent = `${ts} ${lvl} ${entry.message}`;
-
+            return line;
-// Draws every line of the debug log into the panel, when it is opened.
+        }),
-function renderDebugLog() {
+    );
-    const el = document.getElementById("debug-log");
-    if (!el) return;
-    el.replaceChildren(...debugLog.map((entry) => debugLogLine(entry)));
     el.scrollTop = el.scrollHeight;
 }
 
-// The request that scrolls the open panel to its newest line in the next
-// frame. One waits at most, also in a hidden tab, where frames do not run.
-let debugLogScrollFrame = 0;
-
-// Adds entry to the open panel as its last line, and drops the first line
-// once the panel holds more than maxLogEntries. A check that ends while the
-// page is busy is timed only when the page is done, so the lines already
-// shown are left as they are, and the panel is scrolled once a frame
-// rather than once a line: scrolling it lays out all of its lines again.
-function appendDebugLogLine(entry) {
-    const el = document.getElementById("debug-log");
-    if (!el) return;
-    el.append(debugLogLine(entry));
-    if (el.children.length > CONFIG.maxLogEntries)
-        el.removeChild(el.firstElementChild);
-    cancelAnimationFrame(debugLogScrollFrame);
-    debugLogScrollFrame = requestAnimationFrame(() => {
-        el.scrollTop = el.scrollHeight;
-    });
-}
-
 // --- Sorting -----------------------------------------------------------------
 
 // Sort WAN hosts: pinned hosts first (alphabetically), then unpinned hosts
@@ -1555,7 +1502,9 @@ async function init() {
 
             // Clear all host history and reset state
             for (const host of state.allHosts) {
-                host.reset();
+                host.history = [];
+                host.lastLatency = null;
+                host.status = "pending";
             }
             state.tickCount = 0;