Compare commits
4
Commits
85b71b1d70
...
16f7b917d7
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
16f7b917d7 | ||
|
|
cbcde098dc | ||
|
|
73b39e485b | ||
|
|
03bc282871 |
@@ -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
|
||||
|
||||
@@ -229,9 +231,15 @@ Files are created with `O_EXCL` to prevent overwrites.
|
||||
#### Report limits
|
||||
|
||||
`POST /api/v1/reports` takes reports from anyone who can reach it, without
|
||||
credentials, so it is bounded instead. Both refusals below answer with the same
|
||||
credentials, so it is bounded instead. Each refusal below answers 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
|
||||
@@ -404,9 +412,10 @@ 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`, and by default from none.
|
||||
`bin/entrypoint.sh` checks each entry with `netwatch-server check-cidr`, which
|
||||
parses it as the backend parses its own `TRUSTED_PROXIES`
|
||||
reverse proxies named in `TRUSTED_PROXIES`, by default from any address in
|
||||
`10.0.0.0/8`, `172.16.0.0/12` or `192.168.0.0/16`. `bin/entrypoint.sh` checks
|
||||
each entry with `netwatch-server check-cidr`, which parses it as the backend
|
||||
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
|
||||
@@ -425,9 +434,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, and
|
||||
one set to a value netwatch cannot use stops the container at start, with the
|
||||
reason in its log.
|
||||
- **Environment variables:** none is required. An empty one counts as unset,
|
||||
except `TRUSTED_PROXIES`, and one set to a value netwatch cannot use stops the
|
||||
container at start, with the 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
|
||||
@@ -441,16 +450,17 @@ 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 empty: set it to the address the reverse proxy
|
||||
in front of the container connects from, as an IP address or CIDR; several
|
||||
are separated by commas. nginx takes the client address from
|
||||
`X-Forwarded-For` only on a request from one of them, and the rate limit
|
||||
counts that address. Unset, `X-Forwarded-For` is ignored and every client
|
||||
behind the proxy shares the proxy's one allowance of `REPORTS_PER_MINUTE`.
|
||||
Name only addresses nothing but the proxy connects from: any client that
|
||||
connects from one can write its own `X-Forwarded-For`, and through a port
|
||||
Docker publishes, every client may connect from the Docker network's
|
||||
gateway, such as `172.17.0.1`.
|
||||
- `TRUSTED_PROXIES`, default `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`: the
|
||||
addresses the reverse proxy in front of the container connects from, 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, and the rate
|
||||
limit counts that address. A list replaces the default. Set and empty,
|
||||
`X-Forwarded-For` is ignored and every client behind the proxy shares the
|
||||
proxy's one allowance of `REPORTS_PER_MINUTE`. Any client that connects
|
||||
from one of these addresses can write its own `X-Forwarded-For`, and
|
||||
through a port Docker publishes, every client may connect from the Docker
|
||||
network's gateway, such as `172.17.0.1`, which the default includes; where
|
||||
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
|
||||
|
||||
@@ -21,6 +21,41 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 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/`,
|
||||
|
||||
+6
-4
@@ -34,9 +34,11 @@ 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, so
|
||||
# unset or empty, it trusts no one. configs/nginx.conf includes the file
|
||||
# written here, one set_real_ip_from line per entry.
|
||||
# address from X-Forwarded-For only on a request from one of them.
|
||||
# Unset, they are the three private ranges of RFC 1918; set and empty,
|
||||
# nginx trusts no one, which is why the default below follows - and not
|
||||
# :-. 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
|
||||
@@ -45,7 +47,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:-}"
|
||||
TRUSTED_PROXIES="${TRUSTED_PROXIES-10.0.0.0/8,172.16.0.0/12,192.168.0.0/16}"
|
||||
set -f
|
||||
for proxy in $(printf '%s' "$TRUSTED_PROXIES" | tr ',' ' '); do
|
||||
case "$proxy" in
|
||||
|
||||
+4
-3
@@ -18,9 +18,10 @@ 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, and
|
||||
# leaves it empty when TRUSTED_PROXIES is unset, so that by default
|
||||
# the client address is the one each request comes from.
|
||||
# writes one set_real_ip_from line for each into this file. Unset,
|
||||
# TRUSTED_PROXIES is the three private ranges of RFC 1918; set and
|
||||
# empty, the file is empty and the client address is the one each
|
||||
# request comes from.
|
||||
include /etc/nginx/trusted-proxies.conf;
|
||||
real_ip_header X-Forwarded-For;
|
||||
real_ip_recursive on;
|
||||
|
||||
@@ -5,7 +5,9 @@ package handlers
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"mime"
|
||||
"net/http"
|
||||
|
||||
"sneak.berlin/go/netwatch/internal/globals"
|
||||
@@ -18,6 +20,10 @@ 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.
|
||||
@@ -80,14 +86,25 @@ func (s *Handlers) respondJSON(
|
||||
}
|
||||
}
|
||||
|
||||
// decodeJSON decodes the request body into v. The body is
|
||||
// expected to already be bounded by the body-size middleware, so
|
||||
// a caller can distinguish an over-limit body from malformed
|
||||
// JSON by testing the returned error for *http.MaxBytesError.
|
||||
// decodeJSON decodes the request body into v. It returns errNotJSON,
|
||||
// reading nothing, unless the request's Content-Type is
|
||||
// application/json, with or without parameters such as charset. The
|
||||
// body is expected to already be bounded by the body-size middleware,
|
||||
// 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)
|
||||
}
|
||||
|
||||
@@ -30,7 +30,7 @@ type report struct {
|
||||
}
|
||||
|
||||
// HandleReport returns a handler that accepts telemetry
|
||||
// reports from NetWatch clients.
|
||||
// reports from NetWatch clients, sent as application/json.
|
||||
func (s *Handlers) HandleReport() http.HandlerFunc {
|
||||
type response struct {
|
||||
Status string `json:"status"`
|
||||
@@ -66,9 +66,15 @@ func (s *Handlers) HandleReport() http.HandlerFunc {
|
||||
}
|
||||
|
||||
// decodeErrorStatus logs a report decode failure and returns the
|
||||
// status to send: 413 when the body exceeded the size limit,
|
||||
// otherwise 400 for malformed JSON.
|
||||
// status to send: 415 when the report was not sent as JSON, 413 when
|
||||
// the body exceeded the size limit, 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)
|
||||
|
||||
@@ -27,6 +27,17 @@ 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)))
|
||||
}
|
||||
@@ -83,6 +94,7 @@ 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)
|
||||
|
||||
@@ -103,6 +115,89 @@ 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()
|
||||
|
||||
@@ -113,6 +208,7 @@ 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)
|
||||
|
||||
@@ -138,6 +234,7 @@ 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)
|
||||
|
||||
@@ -161,6 +258,7 @@ 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)
|
||||
|
||||
@@ -185,6 +283,8 @@ 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
|
||||
@@ -213,6 +313,7 @@ func TestHandleReportDoesNotLogRawGeo(t *testing.T) {
|
||||
`{"clientId":"c1","geo":{"raw":"`+sentinel+`"},"hosts":[]}`,
|
||||
),
|
||||
)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.HandleReport().ServeHTTP(rec, req)
|
||||
|
||||
@@ -245,6 +346,7 @@ func TestHandleReportLogsClientIDCutToBound(t *testing.T) {
|
||||
`{"clientId":"`+long+`","timestamp":"`+long+`","hosts":[]}`,
|
||||
),
|
||||
)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.HandleReport().ServeHTTP(rec, req)
|
||||
|
||||
@@ -284,6 +386,7 @@ 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)
|
||||
|
||||
|
||||
@@ -297,12 +297,16 @@ 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. http.ErrAbortHandler is
|
||||
// re-panicked so the server can abort the response as intended.
|
||||
// slog, and responds 500 with no body, unless a status was already
|
||||
// written, as Write500OnPanic writes one for a handler's panic.
|
||||
// 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 {
|
||||
@@ -320,10 +324,43 @@ func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
|
||||
"stack", string(debug.Stack()),
|
||||
)
|
||||
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
if ww.Status() == 0 {
|
||||
ww.WriteHeader(http.StatusInternalServerError)
|
||||
}
|
||||
}()
|
||||
|
||||
next.ServeHTTP(w, r)
|
||||
next.ServeHTTP(ww, 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)
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"log"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
@@ -16,7 +17,9 @@ 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 (
|
||||
@@ -323,6 +326,172 @@ 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.
|
||||
|
||||
@@ -34,13 +34,16 @@ 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 s.mw.Recoverer
|
||||
// still answers 500.
|
||||
// Sentry reports a panic, then panics again, so that it still
|
||||
// reaches s.mw.Recoverer.
|
||||
if s.params.Config.SentryDSN != "" {
|
||||
s.router.Use(sentryhttp.New(sentryhttp.Options{Repanic: true}).Handle)
|
||||
}
|
||||
@@ -64,6 +67,8 @@ 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())
|
||||
|
||||
@@ -66,6 +66,7 @@ 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
|
||||
|
||||
@@ -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 unset. It also includes the
|
||||
# security headers, so the page runs under the shipped policy.
|
||||
# writes it when TRUSTED_PROXIES is set and empty. It also includes
|
||||
# the 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$' \
|
||||
|
||||
@@ -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`, () => {
|
||||
|
||||
+38
-12
@@ -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;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user