Let a handler's flush reach the client through the access log (closes #191)
check / check (push) Successful in 3m15s
check / check (push) Successful in 3m15s
The access log's response writer had no Unwrap method, so http.ResponseController stopped at it and a handler's Flush returned http.ErrNotSupported. It now has one. With metrics on, adding Unwrap alone is not enough: go-http-metrics' writer only passes a flush on when the writer inside it has a Flush method, and it sat inside Logging, so the flush silently did nothing. Metrics is now registered outside Logging, and the README's middleware list follows. A new test flushes through the production router, with the defaults and with metrics and Sentry on, on a global route and inside an admin page route group. Model: opus-5-5
This commit is contained in:
@@ -0,0 +1,96 @@
|
||||
package server_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
"sneak.berlin/go/webhooker/internal/server"
|
||||
)
|
||||
|
||||
// TestFlushThroughProductionRouter drives http.ResponseController.Flush
|
||||
// through the shipped router and checks that the flush reaches the
|
||||
// writer the server handed in.
|
||||
//
|
||||
// Every middleware that wraps the writer has to pass a flush through,
|
||||
// with an Unwrap method or a Flush of its own. One that does neither
|
||||
// makes Flush return http.ErrNotSupported, or do nothing at all when
|
||||
// the wrapper outside it only looks for a Flush method, and the
|
||||
// handler that trips over it is far from the cause.
|
||||
//
|
||||
// It runs once with the defaults and once with metrics and Sentry on,
|
||||
// because those two add middleware to the chain, and through both the
|
||||
// global middleware and an admin page route group, which adds its own.
|
||||
func TestFlushThroughProductionRouter(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
cfg func(t *testing.T) *config.Config
|
||||
sentryEnabled bool
|
||||
}{
|
||||
{
|
||||
name: "defaults",
|
||||
cfg: func(t *testing.T) *config.Config {
|
||||
t.Helper()
|
||||
|
||||
return &config.Config{
|
||||
DataDir: t.TempDir(),
|
||||
Environment: config.EnvironmentDev,
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "metrics and Sentry on",
|
||||
cfg: func(t *testing.T) *config.Config {
|
||||
t.Helper()
|
||||
|
||||
return metricsConfig(t, metricsUser, metricsAuthValue)
|
||||
},
|
||||
sentryEnabled: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := newTestEnvWithConfig(t, tc.cfg(t))
|
||||
|
||||
var flushErr error
|
||||
|
||||
probe := func(w http.ResponseWriter, _ *http.Request) {
|
||||
flushErr = http.NewResponseController(w).Flush()
|
||||
}
|
||||
|
||||
routers := map[string]http.Handler{
|
||||
server.ProbePattern: server.NewRouterWithProbeForTest(
|
||||
env.log.Get(), env.cfg, env.mw, env.hnd,
|
||||
tc.sentryEnabled, probe,
|
||||
),
|
||||
server.PageProbePattern: server.NewRouterWithPageProbeForTest(
|
||||
env.log.Get(), env.cfg, env.mw, env.hnd,
|
||||
tc.sentryEnabled, probe,
|
||||
),
|
||||
}
|
||||
|
||||
for path, router := range routers {
|
||||
flushErr = nil
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, httptest.NewRequestWithContext(
|
||||
t.Context(), http.MethodGet, path, nil,
|
||||
))
|
||||
|
||||
require.NoError(t, flushErr, path)
|
||||
assert.True(
|
||||
t, w.Flushed,
|
||||
"%s: the flush must reach the server's writer", path,
|
||||
)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -61,16 +61,21 @@ func (s *Server) SetupRoutes() {
|
||||
func (s *Server) setupGlobalMiddleware() {
|
||||
s.router.Use(middleware.RequestID)
|
||||
s.router.Use(s.mw.SecurityHeaders())
|
||||
s.router.Use(s.mw.Logging())
|
||||
|
||||
// Metrics recording middleware, registered only when the
|
||||
// endpoint that exposes what it records is served. The
|
||||
// condition is the same MetricsAuthEnabled the /metrics mount
|
||||
// in setupRoutes reads.
|
||||
//
|
||||
// It goes outside Logging. go-http-metrics' writer passes a flush
|
||||
// on only when the writer inside it has a Flush method, and the
|
||||
// access log's writer has none, so inside Logging a handler's
|
||||
// flush would silently do nothing.
|
||||
if s.params.Config.MetricsAuthEnabled() {
|
||||
s.router.Use(s.mw.Metrics())
|
||||
}
|
||||
|
||||
s.router.Use(s.mw.Logging())
|
||||
s.router.Use(s.mw.CORS())
|
||||
s.router.Use(middleware.Timeout(requestTimeout))
|
||||
|
||||
|
||||
Reference in New Issue
Block a user