From cb7f3bb02407e6da196c57928d28e4ae3298a9b3 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sun, 9 Aug 2026 00:03:04 +0300 Subject: [PATCH] chimw: the chi-shaped helpers of the custom services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetHead registers a read route under GET and HEAD both, which cover, bench and tokens had each spelled out — five copies of three lines, under three names. RenderRefusals points chi's NotFound and MethodNotAllowed at the service's own error page, which spec and dolt install neither of, so a mistyped URL there is the one refusal on the instance that answers in plain text. RequestLogger is a chi LogFormatter emitting slog records, in place of chi's colourised stdlib line on stdout — the highest-volume line a service writes and the only one that was neither structured nor on stderr. It is a second middleware package and not a wing of the first because sr-ht-ecore/middleware needs nothing but net/http and that rule is worth keeping; the boundary is whether the helper has to know what a route is. A request line is Info, a 5xx is Error, and 499 stays Info by being below 500, which is what that code was chosen for. Skipping probe noise is the caller's call, through a predicate. The logger goes outside RecoverPanics so the line reports the error page that was rendered rather than the nothing an unwinding stack has written. --- chimw/chimw.go | 186 +++++++++++++++++++++ chimw/chimw_test.go | 199 ++++++++++++++++++++++ chimw/logger.go | 256 +++++++++++++++++++++++++++++ chimw/logger_test.go | 347 +++++++++++++++++++++++++++++++++++++++ go.mod | 1 + go.sum | 57 +++++++ middleware/middleware.go | 5 +- 7 files changed, 1049 insertions(+), 2 deletions(-) create mode 100644 chimw/chimw.go create mode 100644 chimw/chimw_test.go create mode 100644 chimw/logger.go create mode 100644 chimw/logger_test.go diff --git a/chimw/chimw.go b/chimw/chimw.go new file mode 100644 index 0000000000000000000000000000000000000000..c847113f4731715806fea6bdaf0e4d01d024f907 --- /dev/null +++ b/chimw/chimw.go @@ -0,0 +1,186 @@ +// Package chimw is the chi half of the shared HTTP middleware for the custom +// services of a self-hosted SourceHut instance (compare, spec, dolt, cover, +// bench, tokens). +// +// It is a second middleware package because the first one may not grow these. +// sr-ht-ecore/middleware imports nothing but net/http, on purpose: it is the +// package a handler, an API surface or a plain http.ServeMux can reach for +// without inheriting anybody's router. That rule is what kept the donors' getHead +// out of it — three lines, all three of them chi's — and the rule is right. It is +// not, however, an argument for leaving the helper copied into every router that +// wants it. Every custom service on this instance routes with chi; a package +// that says so in its name costs a service that already imports chi exactly +// nothing, and costs middleware's rule nothing either. +// +// The boundary between the two is one question: does the helper have to know +// what a route is? A cache header, a panic guard and a status constant do not, +// and live in middleware. Registering a pair of methods for one pattern, +// installing the two handlers chi calls when routing fails, and reading the +// request id chi's RequestID middleware put in the context all do, and live +// here. +// +// Three things, each of them either copied identically by the services that have +// it or absent from the ones that do not — which is the same finding twice over: +// +// - GetHead, the read route registered under GET and HEAD both. Five copies: +// cover, bench and tokens on their web surface, cover and bench again on +// their API, where the same three lines carry a different name each time. +// - RenderRefusals, chi's NotFound and MethodNotAllowed pointed at the +// service's own error page. spec and dolt install neither, so an unrouted GET +// there answers with net/http's plain-text 404 while every other refusal on +// the instance is a rendered page. +// - RequestLogger, the request line as a slog record instead of chi's +// stdlib-log line on stdout. +// +// Usage, in the order a service installs them: +// +// r.Use(chimiddleware.RequestID) +// r.Use(chimiddleware.RealIP) +// r.Use(chimw.RequestLogger(chimw.SlogFormatter{Skip: chimw.SkipPaths("/healthz")})) +// r.Use(middleware.RecoverPanics(func(w http.ResponseWriter, r *http.Request, _ any) { +// s.renderError(w, r, http.StatusInternalServerError, pages.InternalMessage) +// })) +// r.Use(middleware.PrivateCache) +// +// chimw.RenderRefusals(r, s.renderError) +// chimw.GetHead(r, "/healthz", s.handleHealthz) +// +// The daemons that adopt this will have to rename an import: every one of them +// already aliases chi's own middleware package as chimw, which is exactly the +// name this one takes by default. chimiddleware, as spelled above and in this +// package's own files, is the alias to move them to — the shorter name belongs +// to the package a service writes against. +// +// The request logger is the one middleware that belongs outside +// middleware.RecoverPanics rather than inside it. It has to observe the status +// that actually went out, and for a panicking handler that status is the 500 +// RecoverPanics renders — a logger installed underneath would see the unwinding +// stack and nothing written, and would log the request that produced the error +// page as if it had produced nothing. chi's own Logger carries the same +// instruction for the same reason. Nothing is left uncovered by moving it out: +// this middleware builds a record and calls slog, and a panic in slog is not a +// failure any error page is going to survive either. +// +// Records go to slog's default logger unless one is named, exactly as +// middleware.RecoverPanics does and for the same reason: a library has no +// business choosing a handler. The service installs its own — scribe's tinted +// one on this instance — with slog.SetDefault at startup, and the request lines +// land in the same stream, with the same masking rules, as everything else it +// logs. +package chimw + +import ( + "net/http" + + "github.com/go-chi/chi/v5" + + "sourcecraft.dev/bigbes/sr-ht-ecore/pages" +) + +// GetHead registers one read route under GET and HEAD both. +// +// chi's Get registers GET alone, and net/http synthesizes nothing for a custom +// handler, so every read route of a donor answered `curl -I` — and every uptime +// probe, and every cache revalidating what it holds — with a 405 naming GET as +// the only method it would accept, plus a kilobyte of rendered error page from a +// path whose whole job is to say "yes" cheaply. RFC 9110 §9.3.2 makes HEAD +// mandatory for any resource that serves GET, and HEAD is the one method a +// monitor reaches for precisely because it costs no body. +// +// The two share the handler rather than getting one each, and that is the whole +// design: nothing in these services looks at r.Method on a read path, so a HEAD +// produces exactly the status line and the headers its GET would have, computed +// by the same code on the same query. There is no second path that could answer +// 200 where the GET answers 404, which on these pages would be a visibility leak +// (SPEC ch. 6.3). The body is dropped by net/http, which discards writes on a +// HEAD response after counting them, so the Content-Length a client gets is the +// real one. +// +// It is this and not chi's own middleware.GetHead, which registers nothing and +// instead rewrites, per request, the method the routing tree is walked with. +// +// The donors' comments say that rewrite lands on r.Method, so that every line +// written about the request — the log record of RequestLogger below, the panic +// report of middleware.RecoverPanics — names a method the viewer did not send. +// Read chi v5's source and it does not: it sets RouteMethod on the route context +// and leaves the request alone. The correction is worth keeping written down, +// because what is left is the half of the objection that no ordering or logging +// discipline can work around. +// +// The tree stops describing the service. A route registered for GET alone is a +// route that does not serve HEAD as far as anything reading the tree is +// concerned — a chi.Walk, which is how the donors' TestEveryGetRouteHasAHeadTwin +// asks the question at all, and chi's own 405 handler, which builds its Allow +// header out of the methods that were registered. The service answers HEAD +// anyway, from a middleware, and the two records disagree; the request context +// then says GET while the request says HEAD, so which one a handler or a later +// middleware believes depends on which it happened to read. Registering the pair +// leaves one record of what is served, and it is the tree. +// +// It is also two lines against a middleware that re-enters routing on every HEAD +// that missed. +// +// Mutating routes deliberately do not go through it. A HEAD that writes is not a +// HEAD, so a form's POST is registered with r.Post next to its GET, and a HEAD +// on that path resolves to the page. +func GetHead(r chi.Router, pattern string, h http.HandlerFunc) { + r.Get(pattern, h) + r.Head(pattern, h) +} + +// An ErrorRenderer is a service's own error page, as every donor already spells +// it: the status it is answering with and the sentence the viewer can act on. +// +// It is this shape and not middleware.RecoverPanics's (w, r, recovered) because +// the two callbacks are answering different questions. A panic has one status +// and one message and hands over a value to classify; a refusal has no value and +// two statuses, and the renderer is the donors' existing renderError method, +// which is passed by name rather than wrapped in a closure per call site. +type ErrorRenderer func(w http.ResponseWriter, r *http.Request, status int, message string) + +// RenderRefusals points chi's two routing failures at the service's error page. +// +// A path the router does not serve and a method it does not allow are answered +// by chi with net/http's Error: text/plain, no chrome, no nav, no way out for a +// viewer who mistyped a URL. Five of the donors' routers install a pair of +// closures to fix that and spell them identically, compare installs the 404 +// alone and lives with chi's 405, and spec and dolt install neither — so a wrong +// address on those two is the only refusal on the instance that does not look +// like the service it came from. +// +// The messages are the shared ones — pages.NotFoundMessage and +// pages.MethodMessage — rather than the caller's. The 404 above all is not free +// prose: the visibility rules of these services require "somebody else's private +// thing" and "no such thing" to be indistinguishable (SPEC ch. 6.3), and a +// router's 404 that differed in its wording from a handler's would rebuild by +// hand the distinction the status code was chosen to erase. A service that wants +// other prose has the seam anyway, in the renderer it passes. +// +// One thing is lost by taking the 405 over from chi and is worth knowing rather +// than discovering: chi hands the list of methods that *would* have matched only +// to its own default handler, through unexported types, so a custom one cannot +// emit the Allow header RFC 9110 §15.5.6 asks a 405 for. Every donor already +// made that trade, silently. It is the right way round for these services, whose +// 405s are answered to browsers rather than to clients negotiating a method, and +// it is recorded here so that a service that does need Allow knows it has to +// walk the tree for it. +// +// It is a function taking a router, not a middleware, because chi's NotFound and +// MethodNotAllowed are registrations on the routing tree and not links in a +// chain: they run after routing has failed, so a chain has nothing left to hand +// them. That also means they are inherited by every sub-router mounted later — +// installing them once on the root is enough. +// +// A nil renderer is a wiring mistake and panics here, at construction, rather +// than at the first mistyped URL. +func RenderRefusals(r chi.Router, render ErrorRenderer) { + if render == nil { + panic("chimw: RenderRefusals needs a render callback") + } + r.NotFound(func(w http.ResponseWriter, r *http.Request) { + render(w, r, http.StatusNotFound, pages.NotFoundMessage) + }) + r.MethodNotAllowed(func(w http.ResponseWriter, r *http.Request) { + render(w, r, http.StatusMethodNotAllowed, pages.MethodMessage) + }) +} diff --git a/chimw/chimw_test.go b/chimw/chimw_test.go new file mode 100644 index 0000000000000000000000000000000000000000..51c30fa9095fbf2e4f39b331a8e7a7eb199f802f --- /dev/null +++ b/chimw/chimw_test.go @@ -0,0 +1,199 @@ +package chimw + +import ( + "io" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sourcecraft.dev/bigbes/sr-ht-ecore/pages" +) + +// refusal is what a renderer was asked for, so a test can assert on the call +// rather than on prose rendered by a template this package does not own. +type refusal struct { + status int + message string + method string +} + +// recordRefusals returns an ErrorRenderer of the donors' shape, plus the slice +// it appends every call to. +func recordRefusals() (ErrorRenderer, *[]refusal) { + var calls []refusal + render := func(w http.ResponseWriter, r *http.Request, status int, message string) { + calls = append(calls, refusal{status: status, message: message, method: r.Method}) + w.WriteHeader(status) + _, _ = io.WriteString(w, message) + } + return render, &calls +} + +func TestGetHeadServesAHeadThroughTheGetHandler(t *testing.T) { + r := chi.NewRouter() + + var seen []string + GetHead(r, "/page", func(w http.ResponseWriter, r *http.Request) { + seen = append(seen, r.Method) + _, _ = io.WriteString(w, "body") + }) + + for _, method := range []string{http.MethodGet, http.MethodHead} { + rec := httptest.NewRecorder() + r.ServeHTTP(rec, httptest.NewRequest(method, "/page", nil)) + require.Equal(t, http.StatusOK, rec.Code, "%s /page", method) + } + + // The point of registering the pair rather than reaching for chi's + // middleware.GetHead: the handler sees the method that arrived. + assert.Equal(t, []string{http.MethodGet, http.MethodHead}, seen) +} + +func TestGetHeadAnswersAHeadWithTheHeadersOfItsGetAndNoBody(t *testing.T) { + r := chi.NewRouter() + GetHead(r, "/page", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + _, _ = io.WriteString(w, "a body of some length") + }) + + // A real server, because dropping the body of a HEAD is net/http's job and + // a recorder does not do it. + srv := httptest.NewServer(r) + t.Cleanup(srv.Close) + + get, err := http.Get(srv.URL + "/page") + require.NoError(t, err) + t.Cleanup(func() { _ = get.Body.Close() }) + getBody, err := io.ReadAll(get.Body) + require.NoError(t, err) + + head, err := http.Head(srv.URL + "/page") + require.NoError(t, err) + t.Cleanup(func() { _ = head.Body.Close() }) + headBody, err := io.ReadAll(head.Body) + require.NoError(t, err) + + assert.Equal(t, get.StatusCode, head.StatusCode) + assert.Equal(t, get.Header.Get("Content-Type"), head.Header.Get("Content-Type")) + assert.Empty(t, headBody) + // The length a HEAD promises is the one a GET delivers, counted by net/http + // off the writes it then discards. + assert.Equal(t, strconv.Itoa(len(getBody)), head.Header.Get("Content-Length")) +} + +func TestGetHeadRegistersHeadInTheRoutingTree(t *testing.T) { + r := chi.NewRouter() + GetHead(r, "/page", func(http.ResponseWriter, *http.Request) {}) + r.Post("/page", func(http.ResponseWriter, *http.Request) {}) + + // This is the question chi's middleware.GetHead leaves the tree unable to + // answer, and the one the donors' every-GET-has-a-HEAD-twin test asks. + methods := map[string]bool{} + require.NoError(t, chi.Walk(r, func(method, route string, _ http.Handler, _ ...func(http.Handler) http.Handler) error { + if route == "/page" { + methods[method] = true + } + return nil + })) + assert.True(t, methods[http.MethodGet], "GET is registered") + assert.True(t, methods[http.MethodHead], "HEAD is registered") +} + +func TestGetHeadLeavesEveryOtherMethodUnregistered(t *testing.T) { + r := chi.NewRouter() + GetHead(r, "/page", func(http.ResponseWriter, *http.Request) {}) + + rec := httptest.NewRecorder() + r.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/page", nil)) + assert.Equal(t, http.StatusMethodNotAllowed, rec.Code) +} + +func TestRenderRefusalsAnswersAnUnroutedPathThroughTheRenderer(t *testing.T) { + render, calls := recordRefusals() + + r := chi.NewRouter() + RenderRefusals(r, render) + GetHead(r, "/page", func(http.ResponseWriter, *http.Request) {}) + + rec := httptest.NewRecorder() + r.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/nowhere", nil)) + + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.Equal(t, pages.NotFoundMessage, rec.Body.String()) + require.Len(t, *calls, 1) + assert.Equal(t, refusal{ + status: http.StatusNotFound, + message: pages.NotFoundMessage, + method: http.MethodGet, + }, (*calls)[0]) +} + +func TestRenderRefusalsAnswersAnUnallowedMethodThroughTheRenderer(t *testing.T) { + render, calls := recordRefusals() + + r := chi.NewRouter() + RenderRefusals(r, render) + GetHead(r, "/page", func(http.ResponseWriter, *http.Request) {}) + + rec := httptest.NewRecorder() + r.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/page", nil)) + + assert.Equal(t, http.StatusMethodNotAllowed, rec.Code) + assert.Equal(t, pages.MethodMessage, rec.Body.String()) + require.Len(t, *calls, 1) + assert.Equal(t, refusal{ + status: http.StatusMethodNotAllowed, + message: pages.MethodMessage, + method: http.MethodPost, + }, (*calls)[0]) +} + +func TestRenderRefusalsIsInheritedBySubRouters(t *testing.T) { + render, calls := recordRefusals() + + r := chi.NewRouter() + RenderRefusals(r, render) + r.Route("/repo", func(r chi.Router) { + GetHead(r, "/", func(http.ResponseWriter, *http.Request) {}) + }) + + rec := httptest.NewRecorder() + r.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/repo/nowhere", nil)) + + assert.Equal(t, http.StatusNotFound, rec.Code) + require.Len(t, *calls, 1) +} + +func TestRenderRefusalsRequiresARenderCallback(t *testing.T) { + assert.PanicsWithValue(t, "chimw: RenderRefusals needs a render callback", func() { + RenderRefusals(chi.NewRouter(), nil) + }) +} + +// TestRenderRefusalsTakesAMethodValueOfTheDonorsShape pins the seam down: the +// services pass their existing renderError by name, with no closure per call +// site, and that only keeps working while ErrorRenderer has that signature. +func TestRenderRefusalsTakesAMethodValueOfTheDonorsShape(t *testing.T) { + var s server + r := chi.NewRouter() + RenderRefusals(r, s.renderError) + + rec := httptest.NewRecorder() + r.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/nowhere", nil)) + + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.True(t, strings.HasPrefix(rec.Body.String(), "page:"), "body %q", rec.Body.String()) +} + +type server struct{} + +func (s *server) renderError(w http.ResponseWriter, _ *http.Request, status int, message string) { + w.WriteHeader(status) + _, _ = io.WriteString(w, "page: "+message) +} diff --git a/chimw/logger.go b/chimw/logger.go new file mode 100644 index 0000000000000000000000000000000000000000..9a419f2a00cb24bac9c592e9cfc70ad59b138bb4 --- /dev/null +++ b/chimw/logger.go @@ -0,0 +1,256 @@ +package chimw + +import ( + "context" + "log/slog" + "net/http" + "time" + + chimiddleware "github.com/go-chi/chi/v5/middleware" + + "sourcecraft.dev/bigbes/sr-ht-ecore/middleware" +) + +// defaultLogMessage is the message of the request record. It is a constant +// rather than a formatted sentence because everything that varies is an +// attribute: a structured record whose message changed per request would make +// the one field a log pipeline groups by useless. +const defaultLogMessage = "request" + +// SlogFormatter is chi's middleware.LogFormatter emitting slog records instead +// of chi's own line. +// +// chi's Logger writes through a package-level stdlib log.Logger to *stdout*, in +// a colourised human format, with no fields. On this instance that is the +// highest-volume line a service emits and the only one that is not structured +// and not on stderr: an operator who greps the journal for a request id finds +// every panic and every audit line of the daemon, and none of its requests. +// Replacing it is the entire point of this type. +// +// It is a LogFormatter rather than a middleware of our own because chi already +// owns the parts that are tedious to get right — wrapping the ResponseWriter so +// that status and byte count are observable at all, and writing the entry from a +// defer so that a request that panics is still logged. What was missing was +// somewhere to send the result. +// +// The zero value works: records go to slog.Default(), under the message +// "request", for every request. Use it as +// +// r.Use(chimw.RequestLogger(chimw.SlogFormatter{})) +type SlogFormatter struct { + // Logger is where records go. Nil means slog.Default(), read at write time, + // so a service that calls slog.SetDefault after building its router still + // gets its own handler — this is middleware.RecoverPanics's rule, for the + // reason given in the package doc. + Logger *slog.Logger + + // Message overrides the record's message. Empty means "request". + Message string + + // Skip decides which requests produce no line. Nil logs everything. + // + // The decision is the caller's and not this package's, deliberately. The + // noise worth dropping is a probe hitting /healthz every second — but + // "/healthz" is this instance's spelling, a service may also be polled at + // /metrics, at a badge a README embeds, or at a static prefix, and which of + // those is noise depends on what the operator is looking for that week. A + // list baked in here would silently drop the one line somebody debugging + // the probe needs, in a package they would have to read the source of to + // find out why. SkipPaths covers the common case in one call. + // + // It is evaluated once per request, before the handler runs, and it + // silences only the request line: a panic on a skipped path is still + // reported, because a probe path that panics is not noise. + Skip func(r *http.Request) bool +} + +// RequestLogger is the middleware for a formatter: chi's RequestLogger with this +// package's formatter already in it, so that a service need not import chi's +// middleware package to install one. +// +// It goes outermost, ahead of middleware.RecoverPanics — see the package doc for +// why — and after chi's RequestID and RealIP, which must have run before the +// entry is built for the record to carry an id and for RemoteAddr to be the +// viewer's. +func RequestLogger(f SlogFormatter) func(http.Handler) http.Handler { + return chimiddleware.RequestLogger(f) +} + +// SkipPaths builds a Skip predicate matching a fixed set of exact paths, which +// is what a probe endpoint is. It matches on the path alone: a query string +// cannot turn /healthz into something worth a line. +func SkipPaths(paths ...string) func(r *http.Request) bool { + set := make(map[string]struct{}, len(paths)) + for _, p := range paths { + set[p] = struct{}{} + } + return func(r *http.Request) bool { + _, ok := set[r.URL.Path] + return ok + } +} + +// NewLogEntry captures what is known before the handler runs. It implements +// chi's middleware.LogFormatter. +func (f SlogFormatter) NewLogEntry(r *http.Request) chimiddleware.LogEntry { + ctx := r.Context() + return &logEntry{ + formatter: f, + ctx: ctx, + method: r.Method, + path: r.URL.Path, + requestID: chimiddleware.GetReqID(ctx), + quiet: f.Skip != nil && f.Skip(r), + } +} + +// logEntry is one request's record, filled in when the response is done. +// +// The request itself is not held on to. What is logged is copied out here, at +// the top of the chain, where r.Method is still the method that arrived and the +// path has not been rewritten by anything mounted below; keeping the *http.Request +// would mean logging whatever the last middleware to rewrite it decided. +type logEntry struct { + formatter SlogFormatter + ctx context.Context + method string + path string + requestID string + quiet bool +} + +// Write emits the request line. chi calls it from a defer, so it runs for a +// handler that returned normally, one that panicked, and one whose client hung +// up halfway. +// +// The attributes are the five that answer "what happened to this request" — +// method, path, status, bytes, duration — plus the request id when chi's +// RequestID middleware is installed above this one, which is the field that ties +// the line to the panic report and to whatever the handler logged in between. +// +// path is r.URL.Path and deliberately not RequestURI: the query string of these +// services carries search terms a viewer typed and, on the pages that come back +// from an OAuth round trip, parameters nobody wants written to disk twice. The +// route pattern is not logged either — it is available from the route context by +// the time this runs, but it is derivable from the path by anyone reading, and +// the path is the thing an operator has in front of them when a report comes in. +// +// The header is ignored. It is the response's, it is large, and the two fields +// of it worth having (status and length) are already arguments. +func (e *logEntry) Write(status, bytes int, _ http.Header, elapsed time.Duration, _ any) { + if e.quiet { + return + } + + status = e.reportedStatus(status) + + attrs := make([]slog.Attr, 0, 6) + attrs = append(attrs, + slog.String("method", e.method), + slog.String("path", e.path), + slog.Int("status", status), + slog.Int("bytes", bytes), + slog.Duration("duration", elapsed), + ) + if e.requestID != "" { + attrs = append(attrs, slog.String("request_id", e.requestID)) + } + + e.logger().LogAttrs(e.ctx, levelFor(status), e.message(), attrs...) +} + +// Panic is what chi's Recoverer reports through when a log entry is in context; +// without it that report is printed to stdout as a pretty-coloured stack, which +// is the same escape from the log this type exists to close. +// +// It does not double up with middleware.RecoverPanics. That one recovers the +// panic and renders a page, so the only panics still travelling when Recoverer +// looks are the ones it re-raises deliberately — http.ErrAbortHandler, which +// chi's Recoverer re-panics without calling this method. +// +// Skip does not silence it. A request nobody wanted a line for is still a +// request whose panic somebody needs. +func (e *logEntry) Panic(v any, stack []byte) { + attrs := make([]slog.Attr, 0, 5) + attrs = append(attrs, + slog.String("method", e.method), + slog.String("path", e.path), + slog.Any("panic", v), + slog.String("stack", string(stack)), + ) + if e.requestID != "" { + attrs = append(attrs, slog.String("request_id", e.requestID)) + } + + e.logger().LogAttrs(e.ctx, slog.LevelError, "panic serving a request", attrs...) +} + +// reportedStatus turns chi's "nothing was written" into the status the client +// actually saw. +// +// chi's wrapped writer reports 0 when no WriteHeader and no Write ever happened. +// Two things produce that. A handler that returned without touching the writer, +// which net/http answers with an empty 200 — so 200 is what the client got, and +// logging a 0 would send an operator looking for a bug in a redirect that worked. +// And a handler that gave up because the caller was already gone, which is what a +// cancelled request context means here: no status line was sent because there was +// nobody left to send it to. +// +// That second case is reported as middleware.StatusClientClosedRequest — nginx's +// 499, the code the log pipelines in front of this instance already read as "the +// client hung up". It is deliberately not any 5xx, which is what levelFor turns +// into an Error record and what the alert rate is written against: a viewer who +// navigated away mid-render and a CI job that pressed ^C must not page anybody. +// That reasoning is the constant's own (middleware.StatusClientClosedRequest); +// this is the place it gets applied to the highest-volume line the service emits. +// +// A handler that answered 499 itself — the donors' status mapping does, on both +// surfaces — needs no help from here: 499 is below 500, so it is already an Info +// record, which is the whole reason that number was picked over 500. +func (e *logEntry) reportedStatus(status int) int { + if status != 0 { + return status + } + if e.ctx.Err() != nil { + return middleware.StatusClientClosedRequest + } + return http.StatusOK +} + +// levelFor picks the level of a request line from its status. +// +// Everything is Info except a 5xx, which is Error. A request line is not a +// finding — it is the record that something was served — so the default is the +// level a service's ordinary progress is logged at, and a deployment that +// dropped it to Warn would be trading away the only per-request evidence it has. +// +// A 5xx is promoted because it is the one status the service is confessing to: +// nothing the viewer typed produces it, and the record is worth its own line in +// an operator's filter without them having to know the field name for status. +// 4xx stays at Info on purpose. A 404 is a crawler, a stale bookmark or somebody +// mistyping a repository name, a 403 is the visibility rules working, and +// promoting either would hand a stranger with a URL bar the ability to set the +// warning rate of the instance. +// +// Nothing here special-cases 499: it is below 500 and lands at Info by the same +// rule as a 404, which is what makes it the right code for a client that hung up. +func levelFor(status int) slog.Level { + if status >= http.StatusInternalServerError { + return slog.LevelError + } + return slog.LevelInfo +} + +func (e *logEntry) logger() *slog.Logger { + if e.formatter.Logger != nil { + return e.formatter.Logger + } + return slog.Default() +} + +func (e *logEntry) message() string { + if e.formatter.Message != "" { + return e.formatter.Message + } + return defaultLogMessage +} diff --git a/chimw/logger_test.go b/chimw/logger_test.go new file mode 100644 index 0000000000000000000000000000000000000000..67db22239d2f792979e9025870cc8c8adff58640 --- /dev/null +++ b/chimw/logger_test.go @@ -0,0 +1,347 @@ +package chimw + +import ( + "bytes" + "context" + "encoding/json" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + chimiddleware "github.com/go-chi/chi/v5/middleware" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sourcecraft.dev/bigbes/sr-ht-ecore/middleware" +) + +// capture builds a logger writing JSON records into a buffer, so a test can +// assert on the fields an operator would filter by rather than on a line of +// text. +func capture() (*slog.Logger, *bytes.Buffer) { + var buf bytes.Buffer + logger := slog.New(slog.NewJSONHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + return logger, &buf +} + +// records parses everything written to the buffer. +func records(t *testing.T, buf *bytes.Buffer) []map[string]any { + t.Helper() + + var out []map[string]any + for _, line := range strings.Split(strings.TrimSpace(buf.String()), "\n") { + if line == "" { + continue + } + var record map[string]any + require.NoError(t, json.Unmarshal([]byte(line), &record), "line %q", line) + out = append(out, record) + } + return out +} + +// only asserts that exactly one record was written and returns it. +func only(t *testing.T, buf *bytes.Buffer) map[string]any { + t.Helper() + + got := records(t, buf) + require.Len(t, got, 1, "one record per request") + return got[0] +} + +// serve runs one request through a router carrying the logger and the given +// handler at /page. +func serve(f SlogFormatter, req *http.Request, h http.HandlerFunc) *httptest.ResponseRecorder { + r := chi.NewRouter() + r.Use(RequestLogger(f)) + GetHead(r, "/page", h) + + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + return rec +} + +func TestRequestLoggerWritesOneRecordWithTheRequestsFields(t *testing.T) { + logger, buf := capture() + + rec := serve(SlogFormatter{Logger: logger}, + httptest.NewRequest(http.MethodGet, "/page?q=secret", nil), + func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, "twelve bytes") + }) + require.Equal(t, http.StatusOK, rec.Code) + + record := only(t, buf) + assert.Equal(t, "request", record["msg"]) + assert.Equal(t, "INFO", record["level"]) + assert.Equal(t, http.MethodGet, record["method"]) + assert.Equal(t, float64(http.StatusOK), record["status"]) + assert.Equal(t, float64(len("twelve bytes")), record["bytes"]) + assert.Greater(t, record["duration"], float64(0)) + assert.NotContains(t, record, "request_id", "no RequestID middleware is installed") + + // The path and not the request URI: the query string of these services + // carries what a viewer typed. + assert.Equal(t, "/page", record["path"]) +} + +func TestRequestLoggerLogsTheMethodThatArrived(t *testing.T) { + logger, buf := capture() + + serve(SlogFormatter{Logger: logger}, + httptest.NewRequest(http.MethodHead, "/page", nil), + func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, "body") + }) + + assert.Equal(t, http.MethodHead, only(t, buf)["method"]) +} + +func TestRequestLoggerPromotesAServerErrorAndNothingElse(t *testing.T) { + for _, tc := range []struct { + name string + status int + level string + }{ + {"a page", http.StatusOK, "INFO"}, + {"a redirect", http.StatusFound, "INFO"}, + {"a refusal", http.StatusNotFound, "INFO"}, + {"a forbidden page", http.StatusForbidden, "INFO"}, + // 499 is below 500, which is the whole reason that number was picked: + // a client that hung up must not read as a server error. + {"a client that hung up", middleware.StatusClientClosedRequest, "INFO"}, + {"a bug", http.StatusInternalServerError, "ERROR"}, + {"a dependency that is down", http.StatusServiceUnavailable, "ERROR"}, + } { + t.Run(tc.name, func(t *testing.T) { + logger, buf := capture() + + serve(SlogFormatter{Logger: logger}, + httptest.NewRequest(http.MethodGet, "/page", nil), + func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(tc.status) }) + + record := only(t, buf) + assert.Equal(t, float64(tc.status), record["status"]) + assert.Equal(t, tc.level, record["level"]) + }) + } +} + +func TestRequestLoggerReportsAHandlerThatWroteNothingAs200(t *testing.T) { + logger, buf := capture() + + serve(SlogFormatter{Logger: logger}, + httptest.NewRequest(http.MethodGet, "/page", nil), + func(http.ResponseWriter, *http.Request) {}) + + record := only(t, buf) + assert.Equal(t, float64(http.StatusOK), record["status"], "net/http answers an empty handler with 200") + assert.Equal(t, "INFO", record["level"]) +} + +func TestRequestLoggerReportsACancelledRequestAs499(t *testing.T) { + logger, buf := capture() + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + req := httptest.NewRequest(http.MethodGet, "/page", nil).WithContext(ctx) + + // The handler gave up without writing, because there was nobody left to + // write to. + serve(SlogFormatter{Logger: logger}, req, func(http.ResponseWriter, *http.Request) {}) + + record := only(t, buf) + assert.Equal(t, float64(middleware.StatusClientClosedRequest), record["status"]) + assert.Equal(t, "INFO", record["level"], "a client that hung up must not page anybody") +} + +func TestRequestLoggerCarriesTheRequestIDWhenChiSetsOne(t *testing.T) { + logger, buf := capture() + + r := chi.NewRouter() + r.Use(chimiddleware.RequestID) + r.Use(RequestLogger(SlogFormatter{Logger: logger})) + GetHead(r, "/page", func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, chimiddleware.GetReqID(r.Context())) + }) + + rec := httptest.NewRecorder() + r.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/page", nil)) + + record := only(t, buf) + require.Contains(t, record, "request_id") + assert.Equal(t, rec.Body.String(), record["request_id"], "the id the handler saw") + assert.NotEmpty(t, record["request_id"]) +} + +func TestRequestLoggerSkipsWhatThePredicateSkips(t *testing.T) { + logger, buf := capture() + + r := chi.NewRouter() + r.Use(RequestLogger(SlogFormatter{Logger: logger, Skip: SkipPaths("/healthz")})) + GetHead(r, "/healthz", func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, "ok") + }) + GetHead(r, "/page", func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, "page") + }) + + // A query string does not turn a probe into a page. + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/healthz?probe=1", nil)) + assert.Empty(t, records(t, buf)) + + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/page", nil)) + assert.Equal(t, "/page", only(t, buf)["path"]) +} + +func TestRequestLoggerReportsAPanicOnASkippedPath(t *testing.T) { + logger, buf := capture() + + r := chi.NewRouter() + r.Use(RequestLogger(SlogFormatter{Logger: logger, Skip: SkipPaths("/healthz")})) + r.Use(chimiddleware.Recoverer) + GetHead(r, "/healthz", func(http.ResponseWriter, *http.Request) { + panic("the probe is the bug") + }) + + rec := httptest.NewRecorder() + r.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/healthz", nil)) + require.Equal(t, http.StatusInternalServerError, rec.Code) + + record := only(t, buf) + assert.Equal(t, "panic serving a request", record["msg"]) + assert.Equal(t, "ERROR", record["level"]) + assert.Equal(t, "the probe is the bug", record["panic"]) + assert.Contains(t, record["stack"], "chimw") + assert.Equal(t, "/healthz", record["path"]) +} + +func TestRequestLoggerReportsAPanicAndStillWritesTheRequestLine(t *testing.T) { + logger, buf := capture() + + r := chi.NewRouter() + r.Use(RequestLogger(SlogFormatter{Logger: logger})) + r.Use(chimiddleware.Recoverer) + GetHead(r, "/page", func(http.ResponseWriter, *http.Request) { + panic("boom") + }) + + rec := httptest.NewRecorder() + r.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/page", nil)) + require.Equal(t, http.StatusInternalServerError, rec.Code) + + got := records(t, buf) + require.Len(t, got, 2) + assert.Equal(t, "panic serving a request", got[0]["msg"]) + assert.Equal(t, "request", got[1]["msg"]) + // The logger sits outside the recovery, so the line reports the status the + // viewer actually got. + assert.Equal(t, float64(http.StatusInternalServerError), got[1]["status"]) + assert.Equal(t, "ERROR", got[1]["level"]) +} + +// TestRequestLoggerOutsideRecoverPanicsLogsTheStatusTheViewerGot pins the order +// the package doc asks for: the logger goes outermost, ahead of +// middleware.RecoverPanics, so the line reports the error page that was rendered +// rather than the nothing an unwinding stack has written so far. +func TestRequestLoggerOutsideRecoverPanicsLogsTheStatusTheViewerGot(t *testing.T) { + logger, buf := capture() + + // RecoverPanics logs the panic and the stack through the default logger. + // Point that somewhere else so this test asserts on one buffer. + panics, _ := capture() + previous := slog.Default() + slog.SetDefault(panics) + t.Cleanup(func() { slog.SetDefault(previous) }) + + r := chi.NewRouter() + r.Use(RequestLogger(SlogFormatter{Logger: logger})) + r.Use(middleware.RecoverPanics(func(w http.ResponseWriter, _ *http.Request, _ any) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = io.WriteString(w, "error page") + })) + GetHead(r, "/page", func(http.ResponseWriter, *http.Request) { + panic("boom") + }) + + rec := httptest.NewRecorder() + r.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/page", nil)) + require.Equal(t, http.StatusInternalServerError, rec.Code) + require.Equal(t, "error page", rec.Body.String()) + + record := only(t, buf) + assert.Equal(t, float64(http.StatusInternalServerError), record["status"]) + assert.Equal(t, float64(len("error page")), record["bytes"]) + assert.Equal(t, "ERROR", record["level"]) +} + +func TestRequestLoggerFallsBackToTheDefaultLogger(t *testing.T) { + logger, buf := capture() + + previous := slog.Default() + slog.SetDefault(logger) + t.Cleanup(func() { slog.SetDefault(previous) }) + + // The zero value: no logger named, no message, nothing skipped. + serve(SlogFormatter{}, httptest.NewRequest(http.MethodGet, "/page", nil), + func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusTeapot) }) + + record := only(t, buf) + assert.Equal(t, "request", record["msg"]) + assert.Equal(t, float64(http.StatusTeapot), record["status"]) +} + +func TestRequestLoggerReadsTheDefaultLoggerAtWriteTime(t *testing.T) { + r := chi.NewRouter() + r.Use(RequestLogger(SlogFormatter{})) + GetHead(r, "/page", func(http.ResponseWriter, *http.Request) {}) + + // The router was built before the service installed its handler, which is + // the order a daemon does it in. + logger, buf := capture() + previous := slog.Default() + slog.SetDefault(logger) + t.Cleanup(func() { slog.SetDefault(previous) }) + + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/page", nil)) + + assert.Equal(t, "request", only(t, buf)["msg"]) +} + +func TestRequestLoggerTakesTheMessageItIsGiven(t *testing.T) { + logger, buf := capture() + + serve(SlogFormatter{Logger: logger, Message: "http"}, + httptest.NewRequest(http.MethodGet, "/page", nil), + func(http.ResponseWriter, *http.Request) {}) + + assert.Equal(t, "http", only(t, buf)["msg"]) +} + +func TestRequestLoggerLogsARefusalItNeverRouted(t *testing.T) { + logger, buf := capture() + render, _ := recordRefusals() + + r := chi.NewRouter() + r.Use(RequestLogger(SlogFormatter{Logger: logger})) + RenderRefusals(r, render) + GetHead(r, "/page", func(http.ResponseWriter, *http.Request) {}) + + rec := httptest.NewRecorder() + r.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/nowhere", nil)) + require.Equal(t, http.StatusNotFound, rec.Code) + + record := only(t, buf) + assert.Equal(t, float64(http.StatusNotFound), record["status"]) + assert.Equal(t, "/nowhere", record["path"]) + assert.Equal(t, "INFO", record["level"]) +} + +func TestSkipPathsMatchesNothingWhenGivenNothing(t *testing.T) { + skip := SkipPaths() + assert.False(t, skip(httptest.NewRequest(http.MethodGet, "/healthz", nil))) +} diff --git a/go.mod b/go.mod index a50bea008f9760f315c2f8e79a1ec889a5c829cc..c603c4a33a3e8ba8e1d4402b3dfb676112d2a39f 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module sourcecraft.dev/bigbes/sr-ht-ecore go 1.24 require ( + github.com/go-chi/chi/v5 v5.3.1 github.com/stretchr/testify v1.10.0 github.com/vaughan0/go-ini v0.0.0-20130923145212-a98ad7ee00ec sourcecraft.dev/bigbes/sr-ht-core v0.0.0-20260718185800-dd418a200152 diff --git a/go.sum b/go.sum index fe75dbaeae628f65195582b8f15fc3af59ca60fe..918d27dcc2b09827c3f1190737ccb3d37e879ae2 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +1,6 @@ +git.sr.ht/~sircmpwn/dowork v0.0.0-20241216125407-2b00aa42322c/go.mod h1:8neHEO3503w/rNtttnR0JFpQgM/GFhaafVwvkPsFIDw= git.sr.ht/~sircmpwn/getopt v0.0.0-20191230200459-23622cc906b3/go.mod h1:wMEGFFFNuPos7vHmWXfszqImLppbc0wEhh6JBfJIUgw= +git.sr.ht/~sircmpwn/getopt v1.0.0/go.mod h1:wMEGFFFNuPos7vHmWXfszqImLppbc0wEhh6JBfJIUgw= git.sr.ht/~sircmpwn/go-bare v0.0.0-20210406120253-ab86bc2846d9 h1:Ahny8Ud1LjVMMAlt8utUFKhhxJtwBAualvsbc/Sk7cE= git.sr.ht/~sircmpwn/go-bare v0.0.0-20210406120253-ab86bc2846d9/go.mod h1:BVJwbDfVjCjoFiKrhkei6NdGcZYpkDkdyCdg1ukytRA= github.com/99designs/gqlgen v0.17.36 h1:u/o/rv2SZ9s5280dyUOOrkpIIkr/7kITMXYD3rkJ9go= @@ -7,16 +9,48 @@ github.com/DATA-DOG/go-sqlmock v1.5.0 h1:Shsta01QNfFxHCfpW6YH2STWB0MudeXXEWMr20O github.com/DATA-DOG/go-sqlmock v1.5.0/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM= github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8afzqM= github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10= +github.com/ProtonMail/go-crypto v1.3.0/go.mod h1:9whxjD8Rbs29b4XWbB8irEcE8KHMqaR2e7GWU1R+/PE= github.com/agnivade/levenshtein v1.1.1/go.mod h1:veldBMzWxcCG2ZvUTKD2kJNRdCk5hVbJomOvKkmgYbo= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883 h1:bvNMNQO63//z+xNgfBlViaCIJKLlCJ6/fmUseuG0wVQ= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0/go.mod h1:t2tdKJDJF9BV14lnkjHmOQgcvEKgtqs5a1N3LNdJhGE= +github.com/aws/aws-sdk-go-v2 v1.37.1/go.mod h1:9Q0OoGQoboYIAJyslFyF1f5K1Ryddop8gqMhWx/n4Wg= +github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.0/go.mod h1:/mXlTIVG9jbxkqDnr5UQNQxW1HRYxeGklkM9vAFeabg= +github.com/aws/aws-sdk-go-v2/credentials v1.18.2/go.mod h1:v0SdJX6ayPeZFQxgXUKw5RhLpAoZUuynxWDfh8+Eknc= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.1/go.mod h1:HSksQyyJETVZS7uM54cir0IgxttTD+8aEoJMPGepHBI= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.1/go.mod h1:hyAGz30LHdm5KBZDI58MXx5lDVZ5CUfvfTZvMu4HCZo= +github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.1/go.mod h1:Z6QnHC6TmpJWUxAy8FI4JzA7rTwl6EIANkyK9OR5z5w= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.0/go.mod h1:eb3gfbVIxIoGgJsi9pGne19dhCBpK6opTYpQqAmdy44= +github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.8.1/go.mod h1:bAdfrfxENre68Hh2swNaGEVuFYE74o0SaSCAlaG9E74= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.1/go.mod h1:+2MmkvFvPYM1vsozBWduoLJUi5maxFk5B7KJFECujhY= +github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.1/go.mod h1:iikmNLrvHm2p4a3/4BPeix2S9P+nW8yM1IZW73x8bFA= +github.com/aws/aws-sdk-go-v2/service/s3 v1.85.1/go.mod h1:8Q0TAPXD68Z8YqlcIGHs/UNIDHsxErV9H4dl4vJEpgw= +github.com/aws/smithy-go v1.22.5/go.mod h1:t1ufH5HMublsJYulve2RKmHDC15xu1f26kHCp/HgceI= +github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= +github.com/cloudflare/circl v1.6.0/go.mod h1:uddAzsPgqdMAYatqJ0lsjX1oECcQLIlRpzZh3pJrofs= +github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= 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/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc= github.com/dgryski/trifles v0.0.0-20200323201526-dd97f9abfb48/go.mod h1:if7Fbed8SFyPtHLHbg49SI7NAdJiC5WIA09pe59rfAA= +github.com/emersion/go-message v0.18.2/go.mod h1:XpJyL70LwRvq2a8rVbHXikPgKj8+aI0kGdHlg16ibYA= +github.com/emersion/go-pgpmail v0.2.2/go.mod h1:mRB5P7QKiAuOvcT36tdRZvm7nSt7V+f6jbzzup3HuvU= +github.com/emersion/go-sasl v0.0.0-20231106173351-e73c9f7bad43/go.mod h1:iL2twTeMvZnrg54ZoPDNfJaJaqy0xIQFuBdrLsmspwQ= +github.com/emersion/go-smtp v0.21.3/go.mod h1:qm27SGYgoIPRot6ubfQ/GpiPy/g3PaZAVRxiO/sDUgQ= github.com/fernet/fernet-go v0.0.0-20211208181803-9f70042a33ee h1:v6Eju/FhxsACGNipFEPBZZAzGr1F/jlRQr1qiBw2nEE= github.com/fernet/fernet-go v0.0.0-20211208181803-9f70042a33ee/go.mod h1:2H9hjfbpSMHwY503FclkV/lZTBh2YlOmLLSda12uL8c= +github.com/go-chi/chi/v5 v5.3.1 h1:3j4HZLGZQ3JpMCrPJF/Jl3mYJfWLKBfNJ6quurUGCf8= +github.com/go-chi/chi/v5 v5.3.1/go.mod h1:R+tYY2hNuVUUjxoPtqUdgBqevM9s9njzkTLutVsOCto= +github.com/go-chi/cors v1.2.2/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58= +github.com/go-redis/redis/v8 v8.11.5/go.mod h1:gREzHqY1hg6oD9ngVRbLStwAWKhA0FEgq8Jd4h5lpwo= +github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= +github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= +github.com/kavu/go_reuseport v1.5.0/go.mod h1:CG8Ee7ceMFSMnx/xr25Vm0qXaj2Z4i5PWoUx+JZ5/CU= +github.com/kevinmbeaulieu/eq-go v1.0.0/go.mod h1:G3S8ajA56gKBZm4UB9AOyoOS37JO3roToPzKNM8dtdM= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= @@ -29,21 +63,44 @@ github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 h1:P6pPBnrTSX3DEVR4fDembhR github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6FmdpVm2joNMFikkuWg0EoCKLGUMNw= github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw= github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= +github.com/logrusorgru/aurora/v3 v3.0.0/go.mod h1:vsR12bk5grlLvLXAYrBsb5Oc/N+LxAlxggSjiwMnCUc= +github.com/matryer/moq v0.2.7/go.mod h1:kITsx543GOENm48TUAQyJ9+SAvFSr7iGQXPoth/VUBk= +github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= +github.com/mattn/go-isatty v0.0.19/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4= +github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= +github.com/oklog/ulid/v2 v2.1.1/go.mod h1:rcEKHmBBKfef9DhnvX7y1HZBYxjXb0cP5ExxNsTT1QQ= 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/prometheus/client_golang v1.16.0/go.mod h1:Zsulrv/L9oM40tJ7T815tM89lFEugiJ9HzIqaAx4LKc= +github.com/prometheus/client_model v0.4.0/go.mod h1:oMQmHW1/JoDwqLtg57MGgP/Fb1CJEYF2imWWhWtMkYU= +github.com/prometheus/common v0.44.0/go.mod h1:ofAIvZbQ1e/nugmZGz4/qCb9Ap1VoSTIO7x0VV9VvuY= +github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3cnaOZAZEfOo= +github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= +github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/sergi/go-diff v1.3.1 h1:xkr+Oxo4BOQKmkn/B9eMK0g5Kg/983T9DqqPHwYqD+8= github.com/sergi/go-diff v1.3.1/go.mod h1:aMJSSKb2lpPvRNec0+w3fl7LP9IOFzdc9Pa4NFbPK1I= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/urfave/cli/v2 v2.25.5/go.mod h1:GHupkWPMM0M/sj1a2b4wUrWBPzazNrIjouW6fmdJLxc= github.com/vaughan0/go-ini v0.0.0-20130923145212-a98ad7ee00ec h1:DGmKwyZwEB8dI7tbLt/I/gQuP559o/0FrAkHKlQM/Ks= github.com/vaughan0/go-ini v0.0.0-20130923145212-a98ad7ee00ec/go.mod h1:owBmyHYMLkxyrugmfwE/DLJyW8Ro9mkphwuVErQ0iUw= github.com/vektah/gqlparser/v2 v2.5.8 h1:pm6WOnGdzFOCfcQo9L3+xzW51mKrlwTEg4Wr7AH1JW4= github.com/vektah/gqlparser/v2 v2.5.8/go.mod h1:z8xXUff237NntSuH8mLFijZ+1tjV1swDbpDqjJmk6ME= +github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673/go.mod h1:N3UwUGtsrSj3ccvlPHLoLsHnpR27oXr4ZE984MbSER8= +golang.org/x/crypto v0.33.0/go.mod h1:bVdXmD7IV/4GdElGPozy6U7lWdRXA4qyRVGJV57uQ5M= +golang.org/x/mod v0.10.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4= +golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/text v0.22.0/go.mod h1:YRoo4H8PVmsu+E3Ou7cqLVH8oXWIHVoX0jqUWALQhfY= +golang.org/x/tools v0.9.3/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc= +google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= 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= diff --git a/middleware/middleware.go b/middleware/middleware.go index aaeecb6530d57b8aafe759e34755a55291d57dae..2ab0c94756dc427e6f0ea8c5df78714c42fa5574 100644 --- a/middleware/middleware.go +++ b/middleware/middleware.go @@ -25,8 +25,9 @@ // What deliberately did not move here is the donors' getHead helper. It is three // lines, and all three are chi's — it takes a chi.Router and calls Get and Head // on it — so hoisting it would put a router dependency in a package whose whole -// point is that it needs nothing but net/http. It stays in each service, where -// the router it talks to already is. +// point is that it needs nothing but net/http. It lives in the sibling package +// chimw instead, which is allowed the router dependency because knowing what a +// route is is what that package is for. // // Panics are reported through slog's default logger rather than a logger this // package is handed. A library has no business choosing a handler: the service