server: report HTTP handler panics to Sentry (closes #107)

DNSWATCHER_SENTRY_DSN was read but never used. This ports the Sentry
integration from gohttpserver with sentry-go v0.49.0. The server's start
hook calls sentry.Init when the DSN is set; a DSN Sentry cannot parse
fails the hook, so startup stops with the parse error. sentryhttp, with
Repanic, reports handler panics and passes them on to chi's Recoverer.
Shutdown sends queued reports once the HTTP server has stopped. The
client uses the older transport (DisableTelemetryBuffer): with the
default one, Flush can return before sending a report made just before
it. Client reports are off, so only panics are sent. The DSN is checked
at server start, not in config, so the config test is unchanged.
sentry-go raises several golang.org/x modules and moves go-spew and
go-difflib to untagged commits.

Model: opus-5-5

internal/server/export_test.go

@@ -3,6 +3,8 @@ package server
 import (
 	"net/http"
 	"time"
+
+	"github.com/go-chi/chi/v5"
 )
 
 // RequestTimeout exports the handler execution budget applied by
@@ -22,3 +24,15 @@ func SetListenPort(s *Server, port int) {
 func HTTPServerOf(s *Server) *http.Server {
 	return s.httpServer
 }
+
+// EnableSentry runs the Sentry setup that the start hook runs, without
+// starting the HTTP server.
+func EnableSentry(s *Server) error {
+	return s.enableSentry()
+}
+
+// RouterOf returns the router SetupRoutes built, so a test can add a
+// route that panics.
+func RouterOf(s *Server) *chi.Mux {
+	return s.router
+}

internal/server/routes.go

@@ -4,6 +4,7 @@ import (
 	"net/http"
 	"time"
 
+	sentryhttp "github.com/getsentry/sentry-go/http"
 	"github.com/go-chi/chi/v5"
 	chimw "github.com/go-chi/chi/v5/middleware"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
@@ -25,6 +26,16 @@ func (s *Server) SetupRoutes() {
 	s.router.Use(s.mw.Logging())
 	s.router.Use(chimw.Timeout(requestTimeout))
 
+	// Report panics in handlers to Sentry when DNSWATCHER_SENTRY_DSN is
+	// set. Repanic passes each panic on to chimw.Recoverer above, which
+	// still answers the request.
+	if s.sentryEnabled {
+		sentryHandler := sentryhttp.New(sentryhttp.Options{
+			Repanic: true,
+		})
+		s.router.Use(sentryHandler.Handle)
+	}
+
 	// Public, unauthenticated, read-only routes, the only ones
 	// REPO_POLICIES.md allows wildcard CORS on. CORS is middleware of
 	// this whole router, not of a Group, so that it also answers

internal/server/sentry_test.go

@@ -0,0 +1,190 @@
+package server_test
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/getsentry/sentry-go"
+	"github.com/spf13/viper"
+	"go.uber.org/fx"
+
+	"sneak.berlin/go/dnswatcher/internal/server"
+)
+
+// The tests below set env vars and touch the global state of viper and
+// of Sentry, so they cannot use t.Parallel.
+
+// standInDelay is how long the Sentry stand-in takes to answer. It
+// records a report only then, so a report that Shutdown did not wait
+// for has not been recorded yet when Shutdown returns.
+const standInDelay = 100 * time.Millisecond
+
+// sentryStandIn is a local HTTP server in place of Sentry's, so that
+// nothing a test reports leaves the host. It keeps the body of every
+// request it receives.
+type sentryStandIn struct {
+	server *httptest.Server
+	mu     sync.Mutex
+	bodies []string
+}
+
+func newSentryStandIn(t *testing.T) *sentryStandIn {
+	t.Helper()
+
+	standIn := &sentryStandIn{}
+	standIn.server = httptest.NewServer(http.HandlerFunc(
+		func(_ http.ResponseWriter, r *http.Request) {
+			body, err := io.ReadAll(r.Body)
+			if err != nil {
+				t.Errorf("reading request to the Sentry stand-in: %v", err)
+			}
+
+			time.Sleep(standInDelay)
+
+			standIn.mu.Lock()
+			defer standIn.mu.Unlock()
+
+			standIn.bodies = append(standIn.bodies, string(body))
+		},
+	))
+	t.Cleanup(standIn.server.Close)
+
+	return standIn
+}
+
+// dsn returns a DSN that points Sentry at the stand-in.
+func (s *sentryStandIn) dsn(t *testing.T) string {
+	t.Helper()
+
+	dsn, err := url.Parse(s.server.URL)
+	if err != nil {
+		t.Fatalf("parsing the stand-in URL: %v", err)
+	}
+
+	dsn.User = url.User("public-key")
+	dsn.Path = "/1"
+
+	return dsn.String()
+}
+
+// received reports whether a request to the stand-in contained text.
+func (s *sentryStandIn) received(text string) bool {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	for _, body := range s.bodies {
+		if strings.Contains(body, text) {
+			return true
+		}
+	}
+
+	return false
+}
+
+func TestSentryUnsetDoesNothing(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("DNSWATCHER_SENTRY_DSN", "")
+
+	srv := buildServer(t)
+
+	err := server.EnableSentry(srv)
+	if err != nil {
+		t.Fatalf("Sentry setup with no DSN: %v", err)
+	}
+
+	if sentry.CurrentHub().Client() != nil {
+		t.Error("Sentry was set up with no DSN configured")
+	}
+}
+
+// TestSentryReportsHandlerPanic checks that with a valid DSN a panic in
+// a handler is reported to Sentry, still reaches chimw.Recoverer, and
+// has been sent by the time Shutdown returns, and that nothing else is
+// sent to Sentry.
+func TestSentryReportsHandlerPanic(t *testing.T) {
+	standIn := newSentryStandIn(t)
+
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("DNSWATCHER_SENTRY_DSN", standIn.dsn(t))
+
+	srv := buildServer(t)
+
+	err := server.EnableSentry(srv)
+	if err != nil {
+		t.Fatalf("Sentry setup with a valid DSN: %v", err)
+	}
+
+	// Sentry's client is global: close it so later tests find none.
+	t.Cleanup(func() {
+		sentry.CurrentHub().Client().Close()
+		sentry.CurrentHub().BindClient(nil)
+	})
+
+	const panicMessage = "handler panic in the Sentry test"
+
+	srv.SetupRoutes()
+	server.RouterOf(srv).Get(
+		"/panic",
+		func(http.ResponseWriter, *http.Request) {
+			panic(panicMessage)
+		},
+	)
+
+	// An ordinary request first: with client reports on, Sentry would
+	// add a count of its dropped transaction to the panic report.
+	serve(srv, httptest.NewRequestWithContext(
+		t.Context(), http.MethodGet, "/.well-known/healthcheck", nil,
+	))
+
+	rec := serve(srv, httptest.NewRequestWithContext(
+		t.Context(), http.MethodGet, "/panic", nil,
+	))
+	if rec.Code != http.StatusInternalServerError {
+		t.Errorf(
+			"status %d, want %d from the recoverer",
+			rec.Code, http.StatusInternalServerError,
+		)
+	}
+
+	err = srv.Shutdown(t.Context())
+	if err != nil {
+		t.Fatalf("Shutdown: %v", err)
+	}
+
+	if !standIn.received(panicMessage) {
+		t.Error("the panic had not been sent to Sentry when Shutdown returned")
+	}
+
+	if standIn.received("client_report") {
+		t.Error("Sentry was sent a client report, not only the panic")
+	}
+}
+
+func TestSentryInvalidDSNStopsStartup(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("DNSWATCHER_DATA_DIR", t.TempDir())
+	// Sentry cannot parse this: it has no public key before the host.
+	t.Setenv("DNSWATCHER_SENTRY_DSN", "https://sentry.test/1")
+
+	app := newServerApp(fx.Invoke(func(*server.Server) {}))
+
+	err := app.Start(t.Context())
+	if err == nil {
+		_ = app.Stop(t.Context())
+
+		t.Fatal("startup succeeded with an invalid DSN")
+	}
+
+	if !strings.Contains(err.Error(), "invalid DNSWATCHER_SENTRY_DSN") {
+		t.Errorf("startup error does not name the setting: %v", err)
+	}
+}

internal/server/server.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"time"
 
+	"github.com/getsentry/sentry-go"
 	"github.com/go-chi/chi/v5"
 	"go.uber.org/fx"
 
@@ -33,6 +34,10 @@ type Params struct {
 // shutdownTimeout is how long to wait for graceful shutdown.
 const shutdownTimeout = 30 * time.Second
 
+// sentryFlushTimeout is how long shutdown waits for Sentry to send the
+// error reports it still holds.
+const sentryFlushTimeout = 2 * time.Second
+
 // Socket-level timeouts for the HTTP server.
 //
 // These bound time spent on the connection itself and are a distinct
@@ -82,14 +87,15 @@ const (
 
 // Server is the HTTP server.
 type Server struct {
-	startupTime time.Time
-	port        int
-	log         *slog.Logger
-	router      *chi.Mux
-	httpServer  *http.Server
-	params      Params
-	mw          *middleware.Middleware
-	handlers    *handlers.Handlers
+	startupTime   time.Time
+	port          int
+	sentryEnabled bool
+	log           *slog.Logger
+	router        *chi.Mux
+	httpServer    *http.Server
+	params        Params
+	mw            *middleware.Middleware
+	handlers      *handlers.Handlers
 }
 
 // New creates a new Server instance.
@@ -108,6 +114,12 @@ func New(
 	lifecycle.Append(fx.Hook{
 		OnStart: func(_ context.Context) error {
 			srv.startupTime = time.Now()
+
+			err := srv.enableSentry()
+			if err != nil {
+				return err
+			}
+
 			go srv.Run()
 
 			return nil
@@ -152,8 +164,11 @@ func (s *Server) Run() {
 	}
 }
 
-// Shutdown gracefully shuts down the server.
+// Shutdown gracefully shuts down the server, then sends the error
+// reports Sentry still holds.
 func (s *Server) Shutdown(ctx context.Context) error {
+	defer s.flushSentry()
+
 	if s.httpServer == nil {
 		return nil
 	}
@@ -184,3 +199,45 @@ func (s *Server) ServeHTTP(
 ) {
 	s.router.ServeHTTP(writer, request)
 }
+
+// enableSentry turns on Sentry error reporting when
+// DNSWATCHER_SENTRY_DSN is set, and does nothing when it is not. A DSN
+// that Sentry cannot parse is an error, so that startup stops instead
+// of running without the error reporting the operator asked for.
+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,
+		// Use the transport that queues each report as it is made. With
+		// the default one, Flush can return before sending a report made
+		// just before it, such as one from the last request at shutdown.
+		DisableTelemetryBuffer: true,
+		// Send panic reports only, not Sentry's counts of what it dropped,
+		// such as the transaction it starts for every request.
+		DisableClientReports: true,
+	})
+	if err != nil {
+		return fmt.Errorf("invalid DNSWATCHER_SENTRY_DSN: %w", err)
+	}
+
+	s.log.Info("sentry error reporting activated")
+	s.sentryEnabled = true
+
+	return nil
+}
+
+// flushSentry sends the error reports Sentry still holds, waiting at
+// most sentryFlushTimeout.
+func (s *Server) flushSentry() {
+	if !s.sentryEnabled {
+		return
+	}
+
+	if !sentry.Flush(sentryFlushTimeout) {
+		s.log.Warn("sentry flush timed out; some error reports were not sent")
+	}
+}

internal/server/server_test.go

@@ -17,18 +17,13 @@ import (
 	"sneak.berlin/go/dnswatcher/internal/state"
 )
 
-// buildServer wires a *server.Server exactly as cmd/dnswatcher does,
-// minus the watcher/resolver subtree that would touch live DNS. fx
-// builds the object graph but the lifecycle is never started, so no
-// OnStart hook runs and nothing listens or resolves. The caller must
-// first configure viper (config.New reads it), which is also why the
-// caller cannot run in parallel.
-func buildServer(t *testing.T) *server.Server {
-	t.Helper()
-
-	var srv *server.Server
-
-	app := fx.New(
+// newServerApp builds an fx app holding a *server.Server wired exactly
+// as cmd/dnswatcher wires it, minus the watcher/resolver subtree that
+// would touch live DNS, plus the given option. config.New reads viper,
+// so the caller must first configure it, which is also why the caller
+// cannot run in parallel.
+func newServerApp(option fx.Option) *fx.App {
+	return fx.New(
 		fx.NopLogger,
 		fx.Provide(
 			globals.New,
@@ -41,8 +36,18 @@ func buildServer(t *testing.T) *server.Server {
 			handlers.New,
 			server.New,
 		),
-		fx.Populate(&srv),
+		option,
 	)
+}
+
+// buildServer builds the server without starting the app's lifecycle,
+// so no OnStart hook runs and nothing listens or resolves.
+func buildServer(t *testing.T) *server.Server {
+	t.Helper()
+
+	var srv *server.Server
+
+	app := newServerApp(fx.Populate(&srv))
 
 	err := app.Err()
 	if err != nil {