diff --git a/README.md b/README.md index e8d492f..05184c1 100644 --- a/README.md +++ b/README.md @@ -266,6 +266,10 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs: password as their basic auth credentials. With neither set, there are no metrics and `/metrics` is not found. One set without the other, or a user name containing `:`, stops the container + - `SENTRY_DSN`, default empty: set to a Sentry project's DSN, the backend + sends its errors to that Sentry project: each request whose handling + crashes, which still gets a 500 response. A value Sentry does not accept + stops the container. Empty, the backend sends nothing to Sentry - **Health check:** the image's `HEALTHCHECK` requests `/.well-known/healthcheck` through nginx every 30 seconds, so it fails unless both nginx and the backend answer. upaas reads the container's health 60 diff --git a/TODO.md b/TODO.md index 1113703..6b802e0 100644 --- a/TODO.md +++ b/TODO.md @@ -23,6 +23,14 @@ latest run passes. # Completed Steps +- 2026-10-04: the backend reports errors to Sentry (issue #95). With + `SENTRY_DSN` set, it sets up `sentry-go` with the release `netwatch-server-` + and its version, reports each panic in a handler through `sentryhttp`, the + last of the middleware every request goes through, which panics again so the + request still gets the 500 from the panic recovery, and waits up to 2 seconds + on shutdown for Sentry to finish sending. A DSN Sentry refuses stops the start + with an error naming `SENTRY_DSN`. With it empty, Sentry is not set up and + nothing is sent to it - 2026-10-04: a target's name and URL and a debug log message show as the characters they are and are never read as HTML (issue #29): a host row escapes the name and URL it writes into its markup, and the debug log sets each line diff --git a/backend/README.md b/backend/README.md index 80ca89f..5ac252d 100644 --- a/backend/README.md +++ b/backend/README.md @@ -90,6 +90,7 @@ project layout: | `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 `127.0.0.1/32,::1/128,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`. The loopback @@ -198,6 +199,16 @@ is recorded and `/metrics` answers 404. One without the other stops the server from starting, with an error naming both; so does a `METRICS_USERNAME` containing `:`, which basic auth cannot carry, with an error naming it. +### Sentry + +With `SENTRY_DSN` set, the server sends its errors to that Sentry project: each +panic in a handler is reported there, under the release `netwatch-server-` +followed by the server's version, and the request still gets 500 from the +server's panic recovery. On shutdown the server waits up to 2 seconds for Sentry +to finish sending. A DSN Sentry refuses stops the server from starting, with an +error naming `SENTRY_DSN`. With it empty, Sentry is not set up, and nothing is +sent to it. + ## TODO - Add integration test that POSTs a report and verifies the compressed output diff --git a/backend/cmd/netwatch-server/main_test.go b/backend/cmd/netwatch-server/main_test.go index 964e867..5d9eee5 100644 --- a/backend/cmd/netwatch-server/main_test.go +++ b/backend/cmd/netwatch-server/main_test.go @@ -115,6 +115,28 @@ func TestMalformedConfigFileStopsTheStart(t *testing.T) { } } +// TestRefusedSentryDSNStopsTheStart: a SENTRY_DSN that Sentry refuses +// stops the start, and the error, naming SENTRY_DSN, is logged as JSON. +func TestRefusedSentryDSNStopsTheStart(t *testing.T) { + t.Setenv("SENTRY_DSN", "not-a-dsn") + + ctx, cancel := context.WithTimeout(t.Context(), childTimeout) + defer cancel() + + child, stdout, stderr := startServer(ctx, t, t.TempDir(), freePort(ctx, t)) + + err := child.Wait() + if child.ProcessState.ExitCode() != 1 { + t.Fatalf("server exit = %v, want exit status 1", err) + } + + requireJSONLines(t, stdout, stderr) + + if !strings.Contains(stdout.String(), "SENTRY_DSN") { + t.Fatalf("no error naming SENTRY_DSN in stdout:\n%s", stdout) + } +} + // startServer runs main() in a child process listening on // 127.0.0.1:port, with home as its HOME and working directory and its // data directory in home, so it touches nothing outside home. Its diff --git a/backend/go.mod b/backend/go.mod index 881081a..8e3cf0b 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -4,6 +4,7 @@ go 1.25.5 require ( github.com/99designs/basicauth-go v0.0.0-20230316000542-bf6f9cbbf0f8 + github.com/getsentry/sentry-go v0.49.0 github.com/go-chi/chi/v5 v5.2.5 github.com/go-chi/cors v1.2.2 github.com/go-chi/httprate v0.16.0 diff --git a/backend/go.sum b/backend/go.sum index ea56244..2aec8c6 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -4,18 +4,22 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= -github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= +github.com/getsentry/sentry-go v0.49.0 h1:Ehejknu1l023Ub7QoRBVLAI7g3Jnhqku4oWx4B4Sh5s= +github.com/getsentry/sentry-go v0.49.0/go.mod h1:nuMJAoCfe1u0Bts2ocyNI+TW8HT84vRMqwA5Qq/SKUI= github.com/go-chi/chi/v5 v5.2.5 h1:Eg4myHZBjyvJmAFjFvWgrqDTXFyOzjj7YIm3L3mu6Ug= github.com/go-chi/chi/v5 v5.2.5/go.mod h1:X7Gx4mteadT3eDOMTsXzmI4/rwUpOwBHLpAfupzFJP0= github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE= github.com/go-chi/cors v1.2.2/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58= github.com/go-chi/httprate v0.16.0 h1:8V5DH9j6pSK6UQoBsTpvMyFxycqaKEIToyPKzHJjUa8= github.com/go-chi/httprate v0.16.0/go.mod h1:A8lo+qRhk+s9LiuP5saS7XCGDXRXMcrueq0NfIuCa/I= +github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA= +github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og= github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= @@ -36,8 +40,12 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4= github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= -github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4= +github.com/pingcap/errors v0.11.4/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prometheus/client_golang v1.24.1 h1:JnJkREXzWxUdCuPFpIWZiPispT9xVV59uiuyR2bPlnU= github.com/prometheus/client_golang v1.24.1/go.mod h1:F+oSRECHg4sse5ucfYpYDeIv/hu68Zo0uoHKetWnzcE= github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk= @@ -46,8 +54,8 @@ github.com/prometheus/common v0.70.1 h1:1HvjP4D5oL3t8RsPlwxA9onvvStjtIHYE5XuuwOi github.com/prometheus/common v0.70.1/go.mod h1:VdFUQDMZK3VLkurFUVhia6uys/0suUp86TJz5qbJRhc= github.com/prometheus/procfs v0.21.1 h1:GljZCt+zSTS+NZq88cyQ1LjZ+RCHp3uVuabBWA5+OJI= github.com/prometheus/procfs v0.21.1/go.mod h1:aB55Cww9pdSJVHk0hUf0inxWyyjPogFIjmHKYgMKmtY= -github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= -github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= +github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= +github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/sagikazarmark/locafero v0.11.0 h1:1iurJgmM9G3PA/I+wWYIOw/5SyBtxapeHDcg+AAIFXc= github.com/sagikazarmark/locafero v0.11.0/go.mod h1:nVIGvgyzw595SUSUE6tvCp3YYTeHs15MvlmU87WwIik= github.com/slok/go-http-metrics v0.13.0 h1:lQDyJJx9wKhmbliyUsZ2l6peGnXRHjsjoqPt5VYzcP8= @@ -93,7 +101,7 @@ golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/backend/internal/server/export_test.go b/backend/internal/server/export_test.go index 84cdd90..01e263e 100644 --- a/backend/internal/server/export_test.go +++ b/backend/internal/server/export_test.go @@ -1,5 +1,13 @@ package server +import "github.com/go-chi/chi/v5" + +// Router exposes the router to the external tests, which add routes +// of their own to it after SetupRoutes. +func (s *Server) Router() *chi.Mux { + return s.router +} + // MaxRequestBodyBytes exposes the router-wide body limit to the // external tests. const MaxRequestBodyBytes = maxRequestBodyBytes diff --git a/backend/internal/server/routes.go b/backend/internal/server/routes.go index 9f91301..836bbf2 100644 --- a/backend/internal/server/routes.go +++ b/backend/internal/server/routes.go @@ -3,6 +3,7 @@ package server import ( "time" + sentryhttp "github.com/getsentry/sentry-go/http" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" "github.com/prometheus/client_golang/prometheus" @@ -32,6 +33,12 @@ func (s *Server) SetupRoutes() { 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. + if s.params.Config.SentryDSN != "" { + s.router.Use(sentryhttp.New(sentryhttp.Options{Repanic: true}).Handle) + } + // The metrics go in a registry of this server's own, not in // Prometheus' default one, which takes them only once per process. registry := prometheus.NewRegistry() diff --git a/backend/internal/server/routes_test.go b/backend/internal/server/routes_test.go index 8418aa1..d39e86d 100644 --- a/backend/internal/server/routes_test.go +++ b/backend/internal/server/routes_test.go @@ -1,10 +1,12 @@ package server_test import ( + "io" "net/http" "net/http/httptest" "strings" "testing" + "time" "sneak.berlin/go/netwatch/internal/config" "sneak.berlin/go/netwatch/internal/globals" @@ -15,6 +17,7 @@ import ( "sneak.berlin/go/netwatch/internal/reportbuf" "sneak.berlin/go/netwatch/internal/server" + "github.com/getsentry/sentry-go" "go.uber.org/fx" "go.uber.org/fx/fxtest" ) @@ -196,6 +199,70 @@ func TestMetricsInTwoServers(t *testing.T) { } } +// TestSentry: with SENTRY_DSN empty there is no Sentry client. With it +// pointing at a local server standing in for Sentry, a panic in a +// handler reaches that server, and the request still gets the 500 from +// the panic recovery. +func TestSentry(t *testing.T) { + const panicMessage = "handler panic for TestSentry" + + // sentry.Init sets the client for the whole process; take it away + // again so that no other test reports to Sentry. + t.Cleanup(func() { sentry.CurrentHub().BindClient(nil) }) + + t.Setenv("SENTRY_DSN", "") + newServer(t) + + if sentry.CurrentHub().Client() != nil { + t.Fatal("a Sentry client exists with SENTRY_DSN empty") + } + + // The body of the first request the stand-in for Sentry receives. + received := make(chan string, 1) + + sentryServer := httptest.NewServer(http.HandlerFunc( + func(_ http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + + select { + case received <- string(body): + default: + } + }, + )) + defer sentryServer.Close() + + t.Setenv("SENTRY_DSN", + "http://key@"+sentryServer.Listener.Addr().String()+"/1") + + srv := newServer(t) + srv.SetupRoutes() + srv.Router().Get("/panic", func(http.ResponseWriter, *http.Request) { + panic(panicMessage) + }) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodGet, "/panic", http.NoBody) + srv.ServeHTTP(rec, req) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want %d", + rec.Code, http.StatusInternalServerError) + } + + // Sentry sends from a goroutine of its own. + select { + case body := <-received: + if !strings.Contains(body, panicMessage) { + t.Fatalf("the Sentry server received no report of the panic:\n%s", + body) + } + case <-time.After(5 * time.Second): + t.Fatal("nothing reached the Sentry server") + } +} + // TestHealthCheckRejectsOversizeBody sends the health check, which // never reads its body, a body one byte over the limit. Only the // router-wide body limit can reject it. diff --git a/backend/internal/server/server.go b/backend/internal/server/server.go index d14dd25..1e828de 100644 --- a/backend/internal/server/server.go +++ b/backend/internal/server/server.go @@ -7,8 +7,10 @@ package server import ( "context" + "fmt" "log/slog" "net/http" + "time" "sneak.berlin/go/netwatch/internal/config" "sneak.berlin/go/netwatch/internal/globals" @@ -16,10 +18,15 @@ import ( "sneak.berlin/go/netwatch/internal/logger" "sneak.berlin/go/netwatch/internal/middleware" + "github.com/getsentry/sentry-go" "github.com/go-chi/chi/v5" "go.uber.org/fx" ) +// sentryFlushTimeout is how long shutdown waits for Sentry to send +// what it still holds. +const sentryFlushTimeout = 2 * time.Second + // Params defines the dependencies for Server. type Params struct { fx.In @@ -56,6 +63,11 @@ func New( s.log = params.Logger.Get() s.shutdowner = params.Shutdowner + err := s.enableSentry() + if err != nil { + return nil, err + } + lc.Append(fx.Hook{ OnStart: func(_ context.Context) error { // Build the router and http.Server synchronously @@ -87,10 +99,37 @@ func (s *Server) ServeHTTP( s.router.ServeHTTP(w, r) } +// enableSentry sets Sentry up when SENTRY_DSN is set, so that +// SetupRoutes can report panics to it. With SENTRY_DSN empty it does +// nothing. A DSN Sentry refuses stops the start. +func (s *Server) enableSentry() error { + if s.params.Config.SentryDSN == "" { + return nil + } + + err := sentry.Init(sentry.ClientOptions{ + Dsn: s.params.Config.SentryDSN, + Release: s.params.Globals.Appname + "-" + s.params.Globals.Version, + }) + if err != nil { + return fmt.Errorf("SENTRY_DSN: %w", err) + } + + s.log.Info("sentry error reporting activated") + + return nil +} + // shutdown gracefully stops the HTTP server within the -// deadline of the context fx provides for OnStop. +// deadline of the context fx provides for OnStop, then gives +// Sentry, if set up, time to send what it still holds. func (s *Server) shutdown(ctx context.Context) error { err := s.httpServer.Shutdown(ctx) + + if s.params.Config.SentryDSN != "" { + sentry.Flush(sentryFlushTimeout) + } + if err != nil { s.log.Error("server clean shutdown failed", "error", err)