From 28c8b5c92eee9594c74dfeec5f17145956ae07b0 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sun, 8 Nov 2026 22:50:56 +0100 Subject: [PATCH 01/10] test(pluginkit): pin how a panic in a plugin's route is answered and how its routes are read --- pluginkit/features/features_test.go | 4 + pluginkit/features/routes.feature | 184 ++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+) create mode 100644 pluginkit/features/routes.feature diff --git a/pluginkit/features/features_test.go b/pluginkit/features/features_test.go index 705cf88..4b94a7a 100644 --- a/pluginkit/features/features_test.go +++ b/pluginkit/features/features_test.go @@ -49,3 +49,7 @@ func TestRegistration(t *testing.T) { func TestBoot(t *testing.T) { runFeature(t, "boot.feature", initializeBoot(t)) } + +func TestRoutes(t *testing.T) { + runFeature(t, "routes.feature", func(*godog.ScenarioContext) {}) +} diff --git a/pluginkit/features/routes.feature b/pluginkit/features/routes.feature new file mode 100644 index 0000000..334d3b1 --- /dev/null +++ b/pluginkit/features/routes.feature @@ -0,0 +1,184 @@ +Feature: Serving each plugin's routes and reading them at boot + An application mounts each plugin's routes behind its own middleware, and + the paths the plugin declares public skip that middleware. A panic in the + plugin's handler is caught on both. While nothing of the answer went out, + the client gets the application's own answer instead. Once the answer + had begun, the connection is cut, so a half answer never looks complete. + The report is the application's own function that receives each panic + with the plugin's id and its stack. At boot, each plugin's routes are + read on their own, so the application can pause a plugin whose routes + fail to load before it starts. + + @wip + Scenario: A route that answers normally is served untouched + Given the plugin "feed" serves "/items" with a handler that answers status 201 and "created" + When a client asks "feed" for "/items" + Then the client gets status 201 and "created" + And the middleware ran + And the report holds nothing + + @wip + Scenario: A route that panics before it answers gets the application's answer + Given the plugin "feed" serves "/items" with a handler that panics with "boom" + When a client asks "feed" for "/items" + Then the client gets status 500 and "internal error" + And the report holds one failure of "feed" for GET "/items" that the application answered + And that failure reads "pluginkit: feed route panicked: boom" with the stack of the panic + + @wip + Scenario: Headers the route set before it panicked stay off the application's answer + Given the plugin "feed" serves "/items" with a handler that sets the header "X-Feed" and a cookie, then panics with "boom" + When a client asks "feed" for "/items" + Then the client gets status 500 and "internal error" + And the answer holds no header "X-Feed" and no cookie + And the answer holds the header the middleware set + + @wip + Scenario: An informational answer does not begin the answer + Given the plugin "feed" serves "/items" with a handler that sends 103 Early Hints, then panics with "boom" + When a client asks "feed" for "/items" + Then the client gets status 500 and "internal error" + And the report holds one failure of "feed" for GET "/items" that the application answered + + @wip + Scenario Outline: A route that panics after its answer began is cut + Given the plugin "feed" serves "/items" with a handler that , then panics with "boom" + When a client asks "feed" for "/items" + Then the client's answer is cut + And the report holds one failure of "feed" for GET "/items" that the application did not answer + + Examples: + | begins | + | writes the status 200 | + | writes the body "partial" | + | flushes the body "partial" | + | hijacks the connection | + | hijacks the connection through http.Hijacker | + + @wip + Scenario Outline: A route that aborts on purpose is left to the server + Given the plugin "feed" serves "/items" with a handler that + When a client asks "feed" for "/items" + Then the client's answer is cut + And the report holds nothing + + Examples: + | acts | + | aborts | + | flushes the body "partial", then aborts | + | hijacks the connection, then aborts | + + @wip + Scenario Outline: A route reaches the server's writer through the recover + Given the plugin "feed" serves "/items" with a handler that , then writes the body "done" + When a client asks "feed" for "/items" + Then every call the handler made on its writer succeeded + And the client gets status 200 and "done" + + Examples: + | reaches | + | flushes through http.Flusher | + | flushes through a response controller | + | sets a read deadline through a response controller | + | enables full duplex through a response controller | + + @wip + Scenario Outline: A route that takes over the connection answers on its own + Given the plugin "feed" serves "/items" with a handler that , then answers status 200 and "done" on the connection + When a client asks "feed" for "/items" + Then every call the handler made on its writer succeeded + And the client gets status 200 and "done" + And the report holds nothing + + Examples: + | takes | + | hijacks the connection | + | hijacks the connection through http.Hijacker | + + @wip + Scenario: A panic in one plugin's route leaves the other plugins answering + Given the plugin "feed" serves "/items" with a handler that panics with "boom" + And the plugin "events" serves "/list" with a handler that answers status 200 and "listed" + When a client asks "feed" for "/items" + Then the client gets status 500 and "internal error" + And a client asking "events" for "/list" gets status 200 and "listed" + And the report holds one failure of "feed" for GET "/items" that the application answered + + @wip + Scenario: A plugin that gave no handler gets the application's answer + Given the plugin "feed" serves no handler + When a client asks "feed" for "/items" + Then the client gets status 500 and "internal error" + And the report holds one failure of "feed" for GET "/items" that the application answered + + @wip + Scenario: A public route that panics gets the application's answer without the middleware + Given the plugin "feed" serves the public path "/rss.xml" with a handler that panics with "boom" + When a client asks "feed" for "/rss.xml" + Then the client gets status 500 and "internal error" + And the middleware did not run + And the report holds one failure of "feed" for GET "/rss.xml" that the application answered + + @wip + Scenario: A panic in the application's middleware is never blamed on the plugin + Given the plugin "feed" serves "/items" with a handler that answers status 200 and "listed" + And the middleware panics with "session store down" + When a client asks "feed" for "/items" + Then the client's answer is cut + And the report holds nothing + + @wip + Scenario: Over HTTP/2, a route that panics before it answers gets the application's answer + Given the server speaks HTTP/2 + And the plugin "feed" serves "/items" with a handler that panics with "boom" + When a client asks "feed" for "/items" + Then the client gets status 500 and "internal error" + And the answer came over HTTP/2 + And the report holds one failure of "feed" for GET "/items" that the application answered + + @wip + Scenario: Over HTTP/2, a route that panics after it flushed is cut + Given the server speaks HTTP/2 + And the plugin "feed" serves "/items" with a handler that flushes the body "partial", then panics with "boom" + When a client asks "feed" for "/items" + Then the client's answer is cut + And the report holds one failure of "feed" for GET "/items" that the application did not answer + + @wip + Scenario Outline: Protect refuses route settings it cannot serve with + When the application protects a plugin's routes with + Then the protect panics with "" + + Examples: + | missing | text | + | no plugin id | pluginkit: Protect: the plugin id is empty | + | no middleware | pluginkit: Protect: RouteSettings.Wrap is nil | + | no answer | pluginkit: Protect: RouteSettings.Answer is nil | + | no report | pluginkit: Protect: RouteSettings.Report is nil | + + @wip + Scenario: Each plugin's routes and public paths are read on their own + Given the registered plugins "events", "feed" and "theme" + And "events" has routes and the public path "/rss.xml" + And "feed" has routes + When the application reads the routes of each plugin + Then the routes hold "events" and "feed" + And the public paths hold "/rss.xml" for "events" only + And no plugin failed to load its routes + + @wip + Scenario Outline: A plugin whose routes fail to load fails at the step routes + Given the registered plugins "events" and "feed" + And "events" has routes and the public path "/rss.xml" + And "feed" has routes and the public path "/hook" + And + When the application reads the routes of each plugin + Then the routes hold "events" only + And the public paths hold "/rss.xml" for "events" only + And "feed" failed at the step "routes" with "" + + Examples: + | trouble | text | how | + | the Routes of "feed" panics with "boom" | pluginkit: feed routes panicked: boom | as a panic | + | the PublicPaths of "feed" panics with "boom" | pluginkit: feed routes panicked: boom | as a panic | + | the Routes of "feed" answers no handler | pluginkit: feed routes: Routes answered no handler | not as a panic | From 91b72b44f183822b46a133af728830ece2dd9b60 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sun, 8 Nov 2026 22:50:56 +0100 Subject: [PATCH 02/10] feat(pluginkit): recover each panic of a plugin's route handler --- pluginkit/protect.go | 35 ++- pluginkit/protect_test.go | 162 +++++++++++++- pluginkit/recover.go | 113 ++++++++++ pluginkit/recover_test.go | 435 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 733 insertions(+), 12 deletions(-) create mode 100644 pluginkit/recover.go create mode 100644 pluginkit/recover_test.go diff --git a/pluginkit/protect.go b/pluginkit/protect.go index 0696303..af06576 100644 --- a/pluginkit/protect.go +++ b/pluginkit/protect.go @@ -4,19 +4,44 @@ package pluginkit import "net/http" -// Protect wraps a plugin handler in the caller-supplied middleware and serves the -// plugin's declared public paths untouched (exact match, per [PublicPathProvider]). -func Protect(handler http.Handler, publicPaths []string, wrap func(http.Handler) http.Handler) http.Handler { +// RouteSettings are what every plugin's routes are protected under, one value an application reuses for each plugin. +type RouteSettings struct { + // Wrap is the application's middleware, which every path but a plugin's public ones goes through. + Wrap func(http.Handler) http.Handler + // Answer answers a request whose plugin handler panicked before anything went out, and must not panic. + Answer http.Handler + // Report receives every panic of a plugin's handler, from many requests at once, and must not panic. + Report func(RouteFailure) +} + +// Protect serves the handler of plugin id behind settings.Wrap and its public paths without it, recovering each panic. +func Protect(id string, handler http.Handler, publicPaths []string, settings RouteSettings) http.Handler { + checkRouteSettings(id, settings) public := make(map[string]struct{}, len(publicPaths)) for _, path := range publicPaths { public[path] = struct{}{} } - protected := wrap(handler) + plugin := &recovered{id: id, handler: handler, settings: settings} + protected := settings.Wrap(plugin) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if _, ok := public[r.URL.Path]; ok { - handler.ServeHTTP(w, r) + plugin.ServeHTTP(w, r) return } protected.ServeHTTP(w, r) }) } + +// checkRouteSettings panics naming the first value [Protect] cannot serve a plugin's routes without. +func checkRouteSettings(id string, settings RouteSettings) { + switch { + case id == "": + panic("pluginkit: Protect: the plugin id is empty") + case settings.Wrap == nil: + panic("pluginkit: Protect: RouteSettings.Wrap is nil") + case isNil(settings.Answer): + panic("pluginkit: Protect: RouteSettings.Answer is nil") + case settings.Report == nil: + panic("pluginkit: Protect: RouteSettings.Report is nil") + } +} diff --git a/pluginkit/protect_test.go b/pluginkit/protect_test.go index 95ac2fc..03d4dc5 100644 --- a/pluginkit/protect_test.go +++ b/pluginkit/protect_test.go @@ -3,24 +3,61 @@ package pluginkit_test import ( + "fmt" + "io" "net/http" "net/http/httptest" + "slices" + "sync" "testing" "github.com/gopherium/framework/pluginkit" ) +// routeReport keeps every failure a test's Report receives. +type routeReport struct { + mu sync.Mutex + failures []pluginkit.RouteFailure +} + +// report keeps the failure. +func (r *routeReport) report(failure pluginkit.RouteFailure) { + r.mu.Lock() + defer r.mu.Unlock() + r.failures = append(r.failures, failure) +} + +// held returns every failure kept so far. +func (r *routeReport) held() []pluginkit.RouteFailure { + r.mu.Lock() + defer r.mu.Unlock() + return slices.Clone(r.failures) +} + +// answerPanic is the tests' answer to a request whose plugin handler panicked. +func answerPanic(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = io.WriteString(w, "internal error") +} + +// markWrapped is the tests' middleware, which sets X-Wrapped before it calls next. +func markWrapped(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Wrapped", "yes") + next.ServeHTTP(w, r) + }) +} + +// routeSettings returns the tests' route settings, reporting into report. +func routeSettings(report *routeReport) pluginkit.RouteSettings { + return pluginkit.RouteSettings{Wrap: markWrapped, Answer: http.HandlerFunc(answerPanic), Report: report.report} +} + func protectFixture() http.Handler { plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) - wrap := func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("X-Wrapped", "yes") - next.ServeHTTP(w, r) - }) - } - return pluginkit.Protect(plugin, []string{"/webhook", "/rss.xml"}, wrap) + return pluginkit.Protect("feed", plugin, []string{"/webhook", "/rss.xml"}, routeSettings(&routeReport{})) } func TestProtectLetsPublicPathsThroughUntouched(t *testing.T) { @@ -66,3 +103,114 @@ func TestProtectWrapsEverythingElse(t *testing.T) { }) } } + +func TestProtectRecoversAPanicOnAPublicPathWithoutTheMiddleware(t *testing.T) { + t.Parallel() + + report := &routeReport{} + plugin := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { panicWith("boom") }) + recorder := httptest.NewRecorder() + + pluginkit.Protect("feed", plugin, []string{"/rss.xml"}, routeSettings(report)). + ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/rss.xml", nil)) + + if recorder.Code != http.StatusInternalServerError || recorder.Header().Get("X-Wrapped") != "" { + t.Errorf("answered %d with X-Wrapped %q, want 500 unwrapped", recorder.Code, recorder.Header().Get("X-Wrapped")) + } + if failures := report.held(); len(failures) != 1 || failures[0].Path != "/rss.xml" { + t.Errorf("reported %+v, want one failure for /rss.xml", failures) + } +} + +func TestProtectBuildsTheMiddlewareOnce(t *testing.T) { + t.Parallel() + + built := 0 + settings := routeSettings(&routeReport{}) + settings.Wrap = func(next http.Handler) http.Handler { + built++ + return next + } + protected := pluginkit.Protect("feed", http.NotFoundHandler(), nil, settings) + + for range 3 { + protected.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/items", nil)) + } + + if built != 1 { + t.Errorf("the middleware was built %d times, want once", built) + } +} + +func TestProtectLetsAPanicOfTheMiddlewareReachTheServer(t *testing.T) { + t.Parallel() + + report := &routeReport{} + settings := routeSettings(report) + settings.Wrap = func(http.Handler) http.Handler { + return http.HandlerFunc(func(http.ResponseWriter, *http.Request) { panicWith("session store down") }) + } + protected := pluginkit.Protect("feed", http.NotFoundHandler(), nil, settings) + + escaped := serveCatching(protected, httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/items", nil)) + + if escaped != "session store down" { + t.Errorf("the server got the panic %v, want %q", escaped, "session store down") + } + if failures := report.held(); len(failures) != 0 { + t.Errorf("reported %+v, want nothing for a panic of the middleware", failures) + } +} + +func TestProtectRefusesRouteSettingsItCannotServeWith(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + id string + change func(*pluginkit.RouteSettings) + want string + }{ + "empty id": { + change: func(*pluginkit.RouteSettings) {}, + want: "pluginkit: Protect: the plugin id is empty", + }, + "nil Wrap": { + id: "feed", change: func(s *pluginkit.RouteSettings) { s.Wrap = nil }, + want: "pluginkit: Protect: RouteSettings.Wrap is nil", + }, + "nil Answer": { + id: "feed", change: func(s *pluginkit.RouteSettings) { s.Answer = nil }, + want: "pluginkit: Protect: RouteSettings.Answer is nil", + }, + "nil HandlerFunc as Answer": { + id: "feed", change: func(s *pluginkit.RouteSettings) { s.Answer = http.HandlerFunc(nil) }, + want: "pluginkit: Protect: RouteSettings.Answer is nil", + }, + "nil Report": { + id: "feed", change: func(s *pluginkit.RouteSettings) { s.Report = nil }, + want: "pluginkit: Protect: RouteSettings.Report is nil", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + settings := routeSettings(&routeReport{}) + test.change(&settings) + + got := protectPanic(test.id, settings) + + if fmt.Sprint(got) != test.want { + t.Errorf("Protect panicked with %v, want %q", got, test.want) + } + }) + } +} + +// protectPanic returns what Protect panicked with for the id and settings, nil when it did not. +func protectPanic(id string, settings pluginkit.RouteSettings) (escaped any) { + defer func() { escaped = recover() }() + pluginkit.Protect(id, http.NotFoundHandler(), nil, settings) + return nil +} diff --git a/pluginkit/recover.go b/pluginkit/recover.go new file mode 100644 index 0000000..caf3d50 --- /dev/null +++ b/pluginkit/recover.go @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pluginkit + +import ( + "bufio" + "maps" + "net" + "net/http" + "runtime/debug" +) + +// RouteFailure is one panic of one plugin's handler while it served one request. +type RouteFailure struct { + ID string + Method string + // Path is the request path the handler saw. + Path string + // Answered reports whether RouteSettings.Answer wrote the answer, false when the plugin's own had begun. + Answered bool + // Err names the plugin and holds the text of the panic. + Err error + // Stack is the stack of the panic. + Stack string +} + +// recovered serves one plugin's handler, reporting each panic of it and answering it while nothing went out. +type recovered struct { + id string + handler http.Handler + settings RouteSettings +} + +// ServeHTTP serves the plugin's handler through a writer that notes whether the plugin's answer began. +func (h *recovered) ServeHTTP(w http.ResponseWriter, r *http.Request) { + writer := &routeWriter{ResponseWriter: w, http1: r.ProtoMajor == 1} + defer h.finish(writer, maps.Clone(w.Header()), r) + h.handler.ServeHTTP(writer, r) +} + +// finish recovers a panic of the plugin's handler, reports it, then answers it or cuts the answer the plugin began. +func (h *recovered) finish(writer *routeWriter, header http.Header, r *http.Request) { + caught := recover() + if caught == nil { + return + } + if writer.hijacked != nil { + _ = writer.hijacked.Close() + } + if caught == http.ErrAbortHandler { + panic(caught) + } + h.settings.Report(RouteFailure{ + ID: h.id, Method: r.Method, Path: r.URL.Path, Answered: !writer.begun, + Err: panicError("pluginkit: " + h.id + " route panicked: " + describe(caught)), Stack: string(debug.Stack()), + }) + if writer.begun { + panic(http.ErrAbortHandler) + } + clear(writer.Header()) + maps.Copy(writer.Header(), header) + h.settings.Answer.ServeHTTP(writer.ResponseWriter, r) +} + +// routeWriter is the writer a plugin's handler answers through, noting whether its answer began. +type routeWriter struct { + http.ResponseWriter + // http1 makes a 101 status begin the answer, as only HTTP/1 switches protocols with it. + http1 bool + begun bool + hijacked net.Conn +} + +// WriteHeader writes the status, noting that the answer began unless the status is informational. +func (w *routeWriter) WriteHeader(status int) { + w.ResponseWriter.WriteHeader(status) + if status >= http.StatusOK || status == http.StatusSwitchingProtocols && w.http1 { + w.begun = true + } +} + +// Write writes part of the body, noting that the answer began. +func (w *routeWriter) Write(body []byte) (int, error) { + w.begun = true + return w.ResponseWriter.Write(body) +} + +// Flush sends what the handler wrote so far to the client. +func (w *routeWriter) Flush() { + _ = w.FlushError() +} + +// FlushError sends what the handler wrote so far, writing the status 200 first when nothing began the answer. +func (w *routeWriter) FlushError() error { + if !w.begun { + w.WriteHeader(http.StatusOK) + } + return http.NewResponseController(w.ResponseWriter).Flush() +} + +// Hijack takes over the client's connection, noting that the answer began. +func (w *routeWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { + conn, buffered, err := http.NewResponseController(w.ResponseWriter).Hijack() + if err == nil { + w.begun, w.hijacked = true, conn + } + return conn, buffered, err +} + +// Unwrap returns the server's writer, so a response controller reaches its deadlines and full duplex. +func (w *routeWriter) Unwrap() http.ResponseWriter { + return w.ResponseWriter +} diff --git a/pluginkit/recover_test.go b/pluginkit/recover_test.go new file mode 100644 index 0000000..a1efca7 --- /dev/null +++ b/pluginkit/recover_test.go @@ -0,0 +1,435 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pluginkit_test + +import ( + "bufio" + "errors" + "io" + "net" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" + "time" + + "github.com/gopherium/framework/pluginkit" +) + +// panicWith panics with value, so the stack of a test's panic names it. +func panicWith(value any) { + panic(value) +} + +// serveCatching serves r through handler and returns what the handler panicked with, nil when it did not. +func serveCatching(handler http.Handler, w http.ResponseWriter, r *http.Request) (escaped any) { + defer func() { escaped = recover() }() + handler.ServeHTTP(w, r) + return nil +} + +// protectedFeed returns handler as the plugin "feed" serves it under the tests' settings, reporting into report. +func protectedFeed(handler http.Handler, report *routeReport) http.Handler { + return pluginkit.Protect("feed", handler, nil, routeSettings(report)) +} + +// getItems returns a GET request for /items over HTTP/1.1. +func getItems() *http.Request { + return httptest.NewRequest(http.MethodGet, "/items", nil) +} + +// informedWriter is a recorder that sends a status below 200 aside, as a server sends an informational answer. +type informedWriter struct { + *httptest.ResponseRecorder + informed []int +} + +// WriteHeader keeps a status below 200 aside and records any other. +func (w *informedWriter) WriteHeader(status int) { + if status < http.StatusOK { + w.informed = append(w.informed, status) + return + } + w.ResponseRecorder.WriteHeader(status) +} + +// hijackWriter is a recorder whose connection a handler can take over. +type hijackWriter struct { + *httptest.ResponseRecorder + conn net.Conn + err error +} + +// Hijack hands over the writer's connection, or its error. +func (w *hijackWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { + if w.err != nil { + return nil, nil, w.err + } + return w.conn, nil, nil +} + +// deadlineWriter is a recorder that keeps the read deadline a handler sets. +type deadlineWriter struct { + *httptest.ResponseRecorder + deadline time.Time +} + +// SetReadDeadline keeps the deadline. +func (w *deadlineWriter) SetReadDeadline(deadline time.Time) error { + w.deadline = deadline + return nil +} + +// bareWriter is a writer that can do nothing but answer. +type bareWriter struct { + header http.Header + status int +} + +// Header returns the writer's header. +func (w *bareWriter) Header() http.Header { + return w.header +} + +// Write takes the body and drops it. +func (w *bareWriter) Write(body []byte) (int, error) { + return len(body), nil +} + +// WriteHeader keeps the status. +func (w *bareWriter) WriteHeader(status int) { + w.status = status +} + +func TestRecoverAnswersAPanicBeforeTheAnswerBegan(t *testing.T) { + t.Parallel() + + report := &routeReport{} + plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-Feed", "plugin") + w.Header().Add("X-Wrapped", "plugin") + http.SetCookie(w, &http.Cookie{Name: "feed", Value: "plugin"}) + panicWith("boom") + }) + recorder := httptest.NewRecorder() + + escaped := serveCatching(protectedFeed(plugin, report), recorder, httptest.NewRequest(http.MethodPost, "/items", nil)) + + answer := recorder.Result() + if escaped != nil || answer.StatusCode != http.StatusInternalServerError || + recorder.Body.String() != "internal error" { + t.Fatalf("answered %d %q with the panic %v, want the application's answer", answer.StatusCode, + recorder.Body.String(), escaped) + } + if answer.Header.Get("X-Feed") != "" || len(answer.Cookies()) != 0 || + !slices.Equal(answer.Header.Values("X-Wrapped"), []string{"yes"}) { + t.Errorf("the answer holds %v, want only the middleware's header", answer.Header) + } + failures := report.held() + if len(failures) != 1 { + t.Fatalf("reported %+v, want one failure", failures) + } + got := failures[0] + if got.ID != "feed" || got.Method != http.MethodPost || got.Path != "/items" || !got.Answered || + got.Err == nil || got.Err.Error() != "pluginkit: feed route panicked: boom" { + t.Errorf("reported %+v, want the answered panic of feed for POST /items", got) + } + if !strings.Contains(got.Stack, "pluginkit_test.panicWith") { + t.Errorf("the stack does not hold the panic:\n%s", got.Stack) + } +} + +func TestRecoverCutsAnAnswerThatBegan(t *testing.T) { + t.Parallel() + + tests := map[string]func(http.ResponseWriter){ + "a status 200": func(w http.ResponseWriter) { w.WriteHeader(http.StatusOK) }, + "a body": func(w http.ResponseWriter) { _, _ = io.WriteString(w, "partial") }, + "an empty write": func(w http.ResponseWriter) { _, _ = w.Write(nil) }, + "a flush": func(w http.ResponseWriter) { w.(http.Flusher).Flush() }, + "a controller flush": func(w http.ResponseWriter) { _ = http.NewResponseController(w).Flush() }, + "a 101 over HTTP/1": func(w http.ResponseWriter) { w.WriteHeader(http.StatusSwitchingProtocols) }, + } + + for name, begin := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + report := &routeReport{} + plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + begin(w) + panicWith("boom") + }) + writer := &informedWriter{ResponseRecorder: httptest.NewRecorder()} + + escaped := serveCatching(protectedFeed(plugin, report), writer, getItems()) + + if escaped != http.ErrAbortHandler { + t.Errorf("the server got the panic %v, want http.ErrAbortHandler", escaped) + } + if failures := report.held(); len(failures) != 1 || failures[0].Answered { + t.Errorf("reported %+v, want one failure not answered", failures) + } + }) + } +} + +func TestRecoverAnswersAfterAnInformationalStatus(t *testing.T) { + t.Parallel() + + tests := map[string]struct{ status, proto int }{ + "a 103 over HTTP/1": {http.StatusEarlyHints, 1}, + "a 103 over HTTP/2": {http.StatusEarlyHints, 2}, + "a 101 over HTTP/2": {http.StatusSwitchingProtocols, 2}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + report := &routeReport{} + plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(test.status) + panicWith("boom") + }) + writer := &informedWriter{ResponseRecorder: httptest.NewRecorder()} + request := getItems() + request.ProtoMajor = test.proto + + escaped := serveCatching(protectedFeed(plugin, report), writer, request) + + if escaped != nil || writer.Code != http.StatusInternalServerError { + t.Errorf("answered %d with the panic %v, want the application's answer", writer.Code, escaped) + } + if !slices.Equal(writer.informed, []int{test.status}) { + t.Errorf("informed %v, want %d", writer.informed, test.status) + } + if failures := report.held(); len(failures) != 1 || !failures[0].Answered { + t.Errorf("reported %+v, want one answered failure", failures) + } + }) + } +} + +func TestRecoverAnswersAStatusTheServerRefused(t *testing.T) { + t.Parallel() + + report := &routeReport{} + plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(42) }) + recorder := httptest.NewRecorder() + + escaped := serveCatching(protectedFeed(plugin, report), recorder, getItems()) + + if escaped != nil || recorder.Code != http.StatusInternalServerError { + t.Errorf("answered %d with the panic %v, want the application's answer", recorder.Code, escaped) + } + want := "pluginkit: feed route panicked: invalid WriteHeader code 42" + if failures := report.held(); len(failures) != 1 || failures[0].Err.Error() != want { + t.Errorf("reported %+v, want %q", failures, want) + } +} + +func TestRecoverClosesAConnectionTheHandlerHijacked(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + value any + reported int + }{ + "before a panic": {value: "boom", reported: 1}, + "before an abort": {value: http.ErrAbortHandler, reported: 0}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + server, client := net.Pipe() + t.Cleanup(func() { _ = client.Close() }) + report := &routeReport{} + plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if _, _, err := http.NewResponseController(w).Hijack(); err != nil { + t.Errorf("Hijack failed: %v", err) + } + panicWith(test.value) + }) + writer := &hijackWriter{ResponseRecorder: httptest.NewRecorder(), conn: server} + + escaped := serveCatching(protectedFeed(plugin, report), writer, getItems()) + + if escaped != http.ErrAbortHandler { + t.Errorf("the server got the panic %v, want http.ErrAbortHandler", escaped) + } + _ = client.SetReadDeadline(time.Now().Add(time.Second)) + if _, err := client.Read(make([]byte, 1)); !errors.Is(err, io.EOF) { + t.Errorf("reading the hijacked connection answered %v, want io.EOF", err) + } + if failures := report.held(); len(failures) != test.reported || slices.ContainsFunc(failures, + func(failure pluginkit.RouteFailure) bool { return failure.Answered }) { + t.Errorf("reported %+v, want %d failures not answered", failures, test.reported) + } + }) + } +} + +func TestRecoverAnswersAPanicAfterAHijackThatFailed(t *testing.T) { + t.Parallel() + + report := &routeReport{} + plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if _, _, err := http.NewResponseController(w).Hijack(); err == nil { + t.Error("Hijack worked, want the writer's error") + } + panicWith("boom") + }) + writer := &hijackWriter{ResponseRecorder: httptest.NewRecorder(), err: errors.New("the connection is gone")} + + escaped := serveCatching(protectedFeed(plugin, report), writer, getItems()) + + if escaped != nil || writer.Code != http.StatusInternalServerError { + t.Errorf("answered %d with the panic %v, want the application's answer", writer.Code, escaped) + } + if failures := report.held(); len(failures) != 1 || !failures[0].Answered { + t.Errorf("reported %+v, want one answered failure", failures) + } +} + +func TestRecoverLeavesAnAbortToTheServer(t *testing.T) { + t.Parallel() + + report := &routeReport{} + plugin := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { panicWith(http.ErrAbortHandler) }) + recorder := httptest.NewRecorder() + + escaped := serveCatching(protectedFeed(plugin, report), recorder, getItems()) + + if escaped != http.ErrAbortHandler { + t.Errorf("the server got the panic %v, want http.ErrAbortHandler", escaped) + } + if recorder.Body.Len() != 0 || recorder.Result().StatusCode != http.StatusOK { + t.Errorf("answered %d %q, want nothing written", recorder.Code, recorder.Body.String()) + } + if failures := report.held(); len(failures) != 0 { + t.Errorf("reported %+v, want nothing for an abort", failures) + } +} + +func TestRecoverAnswersAPluginWithNoHandler(t *testing.T) { + t.Parallel() + + report := &routeReport{} + recorder := httptest.NewRecorder() + + escaped := serveCatching(protectedFeed(nil, report), recorder, getItems()) + + if escaped != nil || recorder.Code != http.StatusInternalServerError { + t.Errorf("answered %d with the panic %v, want the application's answer", recorder.Code, escaped) + } + want := "pluginkit: feed route panicked: runtime error: invalid memory address or nil pointer dereference" + if failures := report.held(); len(failures) != 1 || failures[0].Err.Error() != want { + t.Errorf("reported %+v, want %q", failures, want) + } +} + +func TestRecoverLeavesAnAnswerThatDidNotPanicAlone(t *testing.T) { + t.Parallel() + + report := &routeReport{} + plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-Feed", "plugin") + w.WriteHeader(http.StatusCreated) + _, _ = io.WriteString(w, "created") + }) + recorder := httptest.NewRecorder() + + escaped := serveCatching(protectedFeed(plugin, report), recorder, getItems()) + + if escaped != nil || recorder.Code != http.StatusCreated || recorder.Body.String() != "created" || + recorder.Header().Get("X-Feed") != "plugin" { + t.Errorf("answered %d %q %v with the panic %v, want the plugin's answer", recorder.Code, + recorder.Body.String(), recorder.Header(), escaped) + } + if failures := report.held(); len(failures) != 0 { + t.Errorf("reported %+v, want nothing", failures) + } +} + +func TestRecoverSetsAReadDeadlineOnTheServersWriter(t *testing.T) { + t.Parallel() + + at := time.Date(2026, 11, 8, 12, 0, 0, 0, time.UTC) + plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if err := http.NewResponseController(w).SetReadDeadline(at); err != nil { + t.Errorf("SetReadDeadline answered %v, want nil", err) + } + }) + writer := &deadlineWriter{ResponseRecorder: httptest.NewRecorder()} + + protectedFeed(plugin, &routeReport{}).ServeHTTP(writer, getItems()) + + if !writer.deadline.Equal(at) { + t.Errorf("the server's writer holds the deadline %v, want %v", writer.deadline, at) + } +} + +func TestRecoverFlushesThroughTheServersWriter(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + status int + want int + }{ + "before any status": {want: http.StatusOK}, + "after a status": {status: http.StatusAccepted, want: http.StatusAccepted}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if test.status != 0 { + w.WriteHeader(test.status) + } + w.(http.Flusher).Flush() + }) + recorder := httptest.NewRecorder() + + protectedFeed(plugin, &routeReport{}).ServeHTTP(recorder, getItems()) + + if !recorder.Flushed || recorder.Code != test.want { + t.Errorf("flushed %t with %d, want flushed with %d", recorder.Flushed, recorder.Code, test.want) + } + }) + } +} + +func TestRecoverAnswersTheErrorOfAWriterThatCannotFlush(t *testing.T) { + t.Parallel() + + plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if err := http.NewResponseController(w).Flush(); !errors.Is(err, http.ErrNotSupported) { + t.Errorf("Flush answered %v, want http.ErrNotSupported", err) + } + }) + writer := &bareWriter{header: http.Header{}} + + protectedFeed(plugin, &routeReport{}).ServeHTTP(writer, getItems()) + + if writer.status != http.StatusOK { + t.Errorf("the server's writer holds %d, want the 200 the flush began", writer.status) + } +} + +func BenchmarkProtectServesARoute(b *testing.B) { + plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNoContent) }) + protected := protectedFeed(plugin, &routeReport{}) + writer := &bareWriter{header: http.Header{}} + request := getItems() + + b.ReportAllocs() + for b.Loop() { + protected.ServeHTTP(writer, request) + } +} From 15f4bfc0e0e723c1a57a8d8718819f4d8a5e404e Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sun, 8 Nov 2026 22:50:56 +0100 Subject: [PATCH 03/10] feat(pluginkit): read each plugin's routes on its own with RoutesEach --- pluginkit/routes.go | 51 +++++++++++++++ pluginkit/routes_test.go | 138 +++++++++++++++++++++++++++++++++++++++ pluginkit/start_each.go | 2 + 3 files changed, 191 insertions(+) create mode 100644 pluginkit/routes.go create mode 100644 pluginkit/routes_test.go diff --git a/pluginkit/routes.go b/pluginkit/routes.go new file mode 100644 index 0000000..1ccc333 --- /dev/null +++ b/pluginkit/routes.go @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pluginkit + +import ( + "context" + "errors" + "net/http" +) + +// errNoHandler is the failure of a Routes that answered no handler. +var errNoHandler = errors.New("Routes answered no handler") + +// RoutesOutcome is what [RoutesEach] answers, its maps keyed by plugin id and its failures in the order of the plugins. +type RoutesOutcome struct { + Routes map[string]http.Handler + PublicPaths map[string][]string + Failed []Failure +} + +// RoutesEach reads each plugin's routes and public paths inside a recover, leaving out every plugin that fails. +func RoutesEach[P interface{ ID() string }](plugins []P) RoutesOutcome { + outcome := RoutesOutcome{Routes: map[string]http.Handler{}, PublicPaths: map[string][]string{}} + for _, plugin := range plugins { + id := plugin.ID() + err := guarded(context.Background(), id+" "+string(StepRoutes), func(context.Context) error { + return outcome.read(id, plugin) + }) + if err != nil { + delete(outcome.Routes, id) + delete(outcome.PublicPaths, id) + outcome.Failed = append(outcome.Failed, failureOf(id, StepRoutes, err)) + } + } + return outcome +} + +// read adds the routes and public paths the plugin id offers, failing a Routes that answered no handler. +func (o *RoutesOutcome) read(id string, plugin any) error { + if provider, ok := plugin.(RouteProvider); ok { + handler := provider.Routes() + if isNil(handler) { + return errNoHandler + } + o.Routes[id] = handler + } + if provider, ok := plugin.(PublicPathProvider); ok { + o.PublicPaths[id] = provider.PublicPaths() + } + return nil +} diff --git a/pluginkit/routes_test.go b/pluginkit/routes_test.go new file mode 100644 index 0000000..dd036dd --- /dev/null +++ b/pluginkit/routes_test.go @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pluginkit_test + +import ( + "maps" + "net/http" + "slices" + "testing" + + "github.com/gopherium/framework/pluginkit" +) + +// panickyRoutesPlugin is a plugin whose Routes panics. +type panickyRoutesPlugin struct { + fakePlugin +} + +// Routes panics. +func (p *panickyRoutesPlugin) Routes() http.Handler { + panic("boom") +} + +// servingPlugin is a plugin with routes and public paths. +type servingPlugin struct { + routedPlugin + paths func() []string +} + +// PublicPaths returns what paths answers. +func (p *servingPlugin) PublicPaths() []string { + return p.paths() +} + +// routed returns a plugin id whose Routes answers handler. +func routed(id string, handler http.Handler) *routedPlugin { + return &routedPlugin{fakePlugin: fakePlugin{id: id}, handler: handler} +} + +func TestRoutesEachReadsEachPluginsRoutesAndPublicPaths(t *testing.T) { + t.Parallel() + + events, feed := http.NewServeMux(), http.NewServeMux() + plugins := []pluginkit.Plugin{ + &servingPlugin{routedPlugin: *routed("events", events), paths: func() []string { return []string{"/rss.xml"} }}, + &fakePlugin{id: "theme"}, + routed("feed", feed), + &publicPathsPlugin{fakePlugin: fakePlugin{id: "hooks"}, paths: []string{"/hook"}}, + } + + outcome := pluginkit.RoutesEach(plugins) + + wantRoutes := map[string]http.Handler{"events": events, "feed": feed} + if !maps.Equal(outcome.Routes, wantRoutes) { + t.Errorf("Routes = %v, want %v", outcome.Routes, wantRoutes) + } + wantPaths := map[string][]string{"events": {"/rss.xml"}, "hooks": {"/hook"}} + if !maps.EqualFunc(outcome.PublicPaths, wantPaths, slices.Equal) { + t.Errorf("PublicPaths = %v, want %v", outcome.PublicPaths, wantPaths) + } + if len(outcome.Failed) != 0 { + t.Errorf("Failed = %+v, want none", outcome.Failed) + } +} + +func TestRoutesEachFailsAPluginWhoseRoutesFailToLoad(t *testing.T) { + t.Parallel() + + const panicked = "pluginkit: feed routes panicked: boom" + const noHandler = "pluginkit: feed routes: Routes answered no handler" + mux := http.NewServeMux() + tests := map[string]struct { + plugin pluginkit.Plugin + want string + panicked bool + }{ + "Routes panics": {plugin: &panickyRoutesPlugin{fakePlugin{id: "feed"}}, want: panicked, panicked: true}, + "PublicPaths panics": { + plugin: &servingPlugin{routedPlugin: *routed("feed", mux), paths: func() []string { panic("boom") }}, + want: panicked, + panicked: true, + }, + "Routes answers nil": {plugin: routed("feed", nil), want: noHandler}, + "Routes answers a nil mux": {plugin: routed("feed", (*http.ServeMux)(nil)), want: noHandler}, + "Routes answers a nil handler": {plugin: routed("feed", http.HandlerFunc(nil)), want: noHandler}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + outcome := pluginkit.RoutesEach([]pluginkit.Plugin{routed("events", mux), test.plugin, routed("gallery", mux)}) + + if got := slices.Sorted(maps.Keys(outcome.Routes)); !slices.Equal(got, []string{"events", "gallery"}) { + t.Errorf("Routes hold %q, want events and gallery", got) + } + if len(outcome.PublicPaths) != 0 { + t.Errorf("PublicPaths = %v, want none", outcome.PublicPaths) + } + if len(outcome.Failed) != 1 { + t.Fatalf("Failed = %+v, want one failure", outcome.Failed) + } + got := outcome.Failed[0] + if got.ID != "feed" || got.Step != pluginkit.StepRoutes || got.Panicked != test.panicked || + got.Err.Error() != test.want { + t.Errorf("Failed = %+v, want feed at %s with %q panicked %t", got, pluginkit.StepRoutes, test.want, + test.panicked) + } + }) + } +} + +func TestRoutesEachListsTheFailuresInTheOrderOfThePlugins(t *testing.T) { + t.Parallel() + + outcome := pluginkit.RoutesEach([]*routedPlugin{routed("gallery", nil), routed("events", nil)}) + + var got []string + for _, failure := range outcome.Failed { + got = append(got, failure.ID) + } + if !slices.Equal(got, []string{"gallery", "events"}) { + t.Errorf("Failed lists %q, want gallery then events", got) + } +} + +func TestRoutesEachAnswersEmptyMapsForNoPlugins(t *testing.T) { + t.Parallel() + + outcome := pluginkit.RoutesEach[pluginkit.Plugin](nil) + + if outcome.Routes == nil || outcome.PublicPaths == nil || len(outcome.Routes)+len(outcome.PublicPaths) != 0 { + t.Errorf("RoutesEach(nil) = %+v, want two empty maps", outcome) + } + if outcome.Failed != nil { + t.Errorf("Failed = %+v, want none", outcome.Failed) + } +} diff --git a/pluginkit/start_each.go b/pluginkit/start_each.go index aa27ddd..01c65bd 100644 --- a/pluginkit/start_each.go +++ b/pluginkit/start_each.go @@ -19,6 +19,8 @@ const ( StepStart Step = "start" // StepRegister is a plugin's Register, called by [RegisterEach]. StepRegister Step = "register" + // StepRoutes is a plugin's Routes and PublicPaths, read by [RoutesEach]. + StepRoutes Step = "routes" ) // Failure is one step one plugin failed, or was still waiting for when the context ended. From ba8e64eb9ee1081f9be5720ff11fc9cf5a1c4336 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sun, 8 Nov 2026 22:50:57 +0100 Subject: [PATCH 04/10] docs(pluginkit): list the route recover and RoutesEach under Unreleased --- pluginkit/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pluginkit/CHANGELOG.md b/pluginkit/CHANGELOG.md index fb0279b..5c062f0 100644 --- a/pluginkit/CHANGELOG.md +++ b/pluginkit/CHANGELOG.md @@ -10,6 +10,9 @@ Releases of this module are tagged `pluginkit/vX.Y.Z`. Releases up to - A boot whose context ends pauses nothing, and a store read or write that fails is returned to the application. - `pluginstoretest.Run` checks any `PluginStore` against the contract, and `pluginstoretest.NewMemory` is a store in memory that keeps it. - `RegisterEach` and `Register` take only plugins with an `ID() string` method, and fail at `StepRegister` a plugin whose `ID` is not its entry's id, or a `Register` that answers no plugin, a breaking change. +- `Protect` takes the plugin's id and a `RouteSettings`, which holds the middleware as `Wrap`, a breaking change. +- `Protect` recovers each panic of a plugin's handler, on its public paths too. While nothing went out, `RouteSettings.Answer` answers it, else the connection is cut, and `RouteSettings.Report` receives a `RouteFailure` with the stack. +- `RoutesEach` reads each plugin's routes and public paths inside a recover, and fails at the new `StepRoutes` a plugin whose `Routes` or `PublicPaths` panics, or whose `Routes` answers no handler. ## 0.12.0 - 2026-11-07 From 3f75fb3c98d02c46c0bcc5411a79ca5b24e0b26c Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sun, 8 Nov 2026 22:50:57 +0100 Subject: [PATCH 05/10] test(pluginkit): run the route scenarios --- pluginkit/features/features_test.go | 2 +- pluginkit/features/routes.feature | 17 - pluginkit/features/steps_routes_test.go | 706 ++++++++++++++++++++++++ 3 files changed, 707 insertions(+), 18 deletions(-) create mode 100644 pluginkit/features/steps_routes_test.go diff --git a/pluginkit/features/features_test.go b/pluginkit/features/features_test.go index 4b94a7a..c4ef5bc 100644 --- a/pluginkit/features/features_test.go +++ b/pluginkit/features/features_test.go @@ -51,5 +51,5 @@ func TestBoot(t *testing.T) { } func TestRoutes(t *testing.T) { - runFeature(t, "routes.feature", func(*godog.ScenarioContext) {}) + runFeature(t, "routes.feature", initializeRoutes()) } diff --git a/pluginkit/features/routes.feature b/pluginkit/features/routes.feature index 334d3b1..e453d62 100644 --- a/pluginkit/features/routes.feature +++ b/pluginkit/features/routes.feature @@ -9,7 +9,6 @@ Feature: Serving each plugin's routes and reading them at boot read on their own, so the application can pause a plugin whose routes fail to load before it starts. - @wip Scenario: A route that answers normally is served untouched Given the plugin "feed" serves "/items" with a handler that answers status 201 and "created" When a client asks "feed" for "/items" @@ -17,7 +16,6 @@ Feature: Serving each plugin's routes and reading them at boot And the middleware ran And the report holds nothing - @wip Scenario: A route that panics before it answers gets the application's answer Given the plugin "feed" serves "/items" with a handler that panics with "boom" When a client asks "feed" for "/items" @@ -25,7 +23,6 @@ Feature: Serving each plugin's routes and reading them at boot And the report holds one failure of "feed" for GET "/items" that the application answered And that failure reads "pluginkit: feed route panicked: boom" with the stack of the panic - @wip Scenario: Headers the route set before it panicked stay off the application's answer Given the plugin "feed" serves "/items" with a handler that sets the header "X-Feed" and a cookie, then panics with "boom" When a client asks "feed" for "/items" @@ -33,14 +30,12 @@ Feature: Serving each plugin's routes and reading them at boot And the answer holds no header "X-Feed" and no cookie And the answer holds the header the middleware set - @wip Scenario: An informational answer does not begin the answer Given the plugin "feed" serves "/items" with a handler that sends 103 Early Hints, then panics with "boom" When a client asks "feed" for "/items" Then the client gets status 500 and "internal error" And the report holds one failure of "feed" for GET "/items" that the application answered - @wip Scenario Outline: A route that panics after its answer began is cut Given the plugin "feed" serves "/items" with a handler that , then panics with "boom" When a client asks "feed" for "/items" @@ -55,7 +50,6 @@ Feature: Serving each plugin's routes and reading them at boot | hijacks the connection | | hijacks the connection through http.Hijacker | - @wip Scenario Outline: A route that aborts on purpose is left to the server Given the plugin "feed" serves "/items" with a handler that When a client asks "feed" for "/items" @@ -68,7 +62,6 @@ Feature: Serving each plugin's routes and reading them at boot | flushes the body "partial", then aborts | | hijacks the connection, then aborts | - @wip Scenario Outline: A route reaches the server's writer through the recover Given the plugin "feed" serves "/items" with a handler that , then writes the body "done" When a client asks "feed" for "/items" @@ -82,7 +75,6 @@ Feature: Serving each plugin's routes and reading them at boot | sets a read deadline through a response controller | | enables full duplex through a response controller | - @wip Scenario Outline: A route that takes over the connection answers on its own Given the plugin "feed" serves "/items" with a handler that , then answers status 200 and "done" on the connection When a client asks "feed" for "/items" @@ -95,7 +87,6 @@ Feature: Serving each plugin's routes and reading them at boot | hijacks the connection | | hijacks the connection through http.Hijacker | - @wip Scenario: A panic in one plugin's route leaves the other plugins answering Given the plugin "feed" serves "/items" with a handler that panics with "boom" And the plugin "events" serves "/list" with a handler that answers status 200 and "listed" @@ -104,14 +95,12 @@ Feature: Serving each plugin's routes and reading them at boot And a client asking "events" for "/list" gets status 200 and "listed" And the report holds one failure of "feed" for GET "/items" that the application answered - @wip Scenario: A plugin that gave no handler gets the application's answer Given the plugin "feed" serves no handler When a client asks "feed" for "/items" Then the client gets status 500 and "internal error" And the report holds one failure of "feed" for GET "/items" that the application answered - @wip Scenario: A public route that panics gets the application's answer without the middleware Given the plugin "feed" serves the public path "/rss.xml" with a handler that panics with "boom" When a client asks "feed" for "/rss.xml" @@ -119,7 +108,6 @@ Feature: Serving each plugin's routes and reading them at boot And the middleware did not run And the report holds one failure of "feed" for GET "/rss.xml" that the application answered - @wip Scenario: A panic in the application's middleware is never blamed on the plugin Given the plugin "feed" serves "/items" with a handler that answers status 200 and "listed" And the middleware panics with "session store down" @@ -127,7 +115,6 @@ Feature: Serving each plugin's routes and reading them at boot Then the client's answer is cut And the report holds nothing - @wip Scenario: Over HTTP/2, a route that panics before it answers gets the application's answer Given the server speaks HTTP/2 And the plugin "feed" serves "/items" with a handler that panics with "boom" @@ -136,7 +123,6 @@ Feature: Serving each plugin's routes and reading them at boot And the answer came over HTTP/2 And the report holds one failure of "feed" for GET "/items" that the application answered - @wip Scenario: Over HTTP/2, a route that panics after it flushed is cut Given the server speaks HTTP/2 And the plugin "feed" serves "/items" with a handler that flushes the body "partial", then panics with "boom" @@ -144,7 +130,6 @@ Feature: Serving each plugin's routes and reading them at boot Then the client's answer is cut And the report holds one failure of "feed" for GET "/items" that the application did not answer - @wip Scenario Outline: Protect refuses route settings it cannot serve with When the application protects a plugin's routes with Then the protect panics with "" @@ -156,7 +141,6 @@ Feature: Serving each plugin's routes and reading them at boot | no answer | pluginkit: Protect: RouteSettings.Answer is nil | | no report | pluginkit: Protect: RouteSettings.Report is nil | - @wip Scenario: Each plugin's routes and public paths are read on their own Given the registered plugins "events", "feed" and "theme" And "events" has routes and the public path "/rss.xml" @@ -166,7 +150,6 @@ Feature: Serving each plugin's routes and reading them at boot And the public paths hold "/rss.xml" for "events" only And no plugin failed to load its routes - @wip Scenario Outline: A plugin whose routes fail to load fails at the step routes Given the registered plugins "events" and "feed" And "events" has routes and the public path "/rss.xml" diff --git a/pluginkit/features/steps_routes_test.go b/pluginkit/features/steps_routes_test.go new file mode 100644 index 0000000..f7aec7e --- /dev/null +++ b/pluginkit/features/steps_routes_test.go @@ -0,0 +1,706 @@ +// SPDX-License-Identifier: Apache-2.0 + +package features_test + +import ( + "context" + "errors" + "fmt" + "io" + "log" + "maps" + "net" + "net/http" + "net/http/httptest" + "regexp" + "slices" + "strconv" + "strings" + "sync" + "sync/atomic" + "time" + + "github.com/cucumber/godog" + + "github.com/gopherium/framework/pluginkit" +) + +// clientTimeout bounds how long a scenario waits for a client or a handler. +const clientTimeout = 10 * time.Second + +// action is one thing a scenario's handler does while it serves a request. +type action func(w *routeWorld, h *handling) + +// handling is what one request's handler holds between the things it does. +type handling struct { + writer http.ResponseWriter + conn net.Conn +} + +// servedPlugin is one plugin a routes scenario mounts and what its handler does. +type servedPlugin struct { + id string + path string + public bool + noHandler bool + actions []action +} + +// answered is what a client got for one request. +type answered struct { + status int + proto int + header http.Header + body string + err error + bodyErr error +} + +// namedPlugin is a plugin whose routes a scenario reads. +type namedPlugin interface{ ID() string } + +// plainPlugin is a registered plugin that offers neither routes nor public paths. +type plainPlugin struct{ id string } + +// ID returns the plugin's id. +func (p plainPlugin) ID() string { + return p.id +} + +// routedPlugin is a registered plugin that offers routes. +type routedPlugin struct { + plainPlugin + routes func() http.Handler +} + +// Routes returns what the scenario has the plugin answer. +func (p routedPlugin) Routes() http.Handler { + return p.routes() +} + +// publicPlugin is a registered plugin that offers routes and public paths. +type publicPlugin struct { + routedPlugin + paths func() []string +} + +// PublicPaths returns what the scenario has the plugin answer. +func (p publicPlugin) PublicPaths() []string { + return p.paths() +} + +// routeWorld is what one scenario of the routes feature serves, reads and finds. +type routeWorld struct { + http2 bool + plugins []servedPlugin + middlewarePanic string + middlewareRan atomic.Bool + server *httptest.Server + client *http.Client + served sync.WaitGroup + mu sync.Mutex + failures []pluginkit.RouteFailure + calls []error + last answered + failure pluginkit.RouteFailure + protectPanic any + registered []string + hasRoutes map[string]bool + publicPath map[string]string + routesPanic map[string]string + pathsPanic map[string]string + noHandler map[string]bool + outcome pluginkit.RoutesOutcome +} + +// panicWith panics with value, so the stack of a scenario's panic names it. +func panicWith(value any) { + panic(value) +} + +// handlerActions lists each thing a scenario's handler can do and builds it from the words that name it. +var handlerActions = []struct { + pattern *regexp.Regexp + build func(match []string) action +}{ + {regexp.MustCompile(`^answers status (\d+) and "([^"]*)"$`), answersStatus}, + {regexp.MustCompile(`^answers status (\d+) and "([^"]*)" on the connection$`), answersOnConnection}, + {regexp.MustCompile(`^panics with "([^"]*)"$`), panics}, + {regexp.MustCompile(`^aborts$`), aborts}, + {regexp.MustCompile(`^writes the status (\d+)$`), writesStatus}, + {regexp.MustCompile(`^writes the body "([^"]*)"$`), writesBody}, + {regexp.MustCompile(`^flushes the body "([^"]*)"$`), flushesBody}, + {regexp.MustCompile(`^hijacks the connection( through http\.Hijacker)?$`), hijacks}, + {regexp.MustCompile(`^sends 103 Early Hints$`), sendsEarlyHints}, + {regexp.MustCompile(`^sets the header "([^"]*)" and a cookie$`), setsHeaderAndCookie}, + {regexp.MustCompile(`^flushes through http\.Flusher$`), flushesThroughFlusher}, + {regexp.MustCompile(`^(flushes|sets a read deadline|enables full duplex) through a response controller$`), controls}, +} + +// actionOf returns the action the words name. +func actionOf(words string) (action, error) { + for _, known := range handlerActions { + if match := known.pattern.FindStringSubmatch(words); match != nil { + return known.build(match), nil + } + } + return nil, fmt.Errorf("no handler action reads %q", words) +} + +// answersStatus writes the status and the body. +func answersStatus(match []string) action { + status, _ := strconv.Atoi(match[1]) + return func(_ *routeWorld, h *handling) { + h.writer.WriteHeader(status) + _, _ = io.WriteString(h.writer, match[2]) + } +} + +// answersOnConnection writes a whole answer with the status and the body on the hijacked connection, then closes it. +func answersOnConnection(match []string) action { + status, _ := strconv.Atoi(match[1]) + return func(w *routeWorld, h *handling) { + if h.conn == nil { + w.called(errors.New("no connection was hijacked")) + return + } + answer := fmt.Sprintf("HTTP/1.1 %d %s\r\nContent-Length: %d\r\nConnection: close\r\n\r\n%s", + status, http.StatusText(status), len(match[2]), match[2]) + _, err := io.WriteString(h.conn, answer) + w.called(errors.Join(err, h.conn.Close())) + } +} + +// panics panics with the text. +func panics(match []string) action { + return func(*routeWorld, *handling) { panicWith(match[1]) } +} + +// aborts panics with the value that tells the server to cut the answer. +func aborts([]string) action { + return func(*routeWorld, *handling) { panicWith(http.ErrAbortHandler) } +} + +// writesStatus writes the status alone. +func writesStatus(match []string) action { + status, _ := strconv.Atoi(match[1]) + return func(_ *routeWorld, h *handling) { h.writer.WriteHeader(status) } +} + +// writesBody writes the body without flushing it. +func writesBody(match []string) action { + return func(_ *routeWorld, h *handling) { _, _ = io.WriteString(h.writer, match[1]) } +} + +// flushesBody writes the body and flushes it to the client. +func flushesBody(match []string) action { + return func(w *routeWorld, h *handling) { + _, _ = io.WriteString(h.writer, match[1]) + w.called(http.NewResponseController(h.writer).Flush()) + } +} + +// hijacks takes over the connection through a response controller, or through http.Hijacker when the words say so. +func hijacks(match []string) action { + return func(w *routeWorld, h *handling) { + take := http.NewResponseController(h.writer).Hijack + if match[1] != "" { + hijacker, ok := h.writer.(http.Hijacker) + if !ok { + w.called(errors.New("the writer is not an http.Hijacker")) + return + } + take = hijacker.Hijack + } + conn, _, err := take() + h.conn = conn + w.called(err) + } +} + +// sendsEarlyHints sends an informational 103 with a Link header. +func sendsEarlyHints([]string) action { + return func(_ *routeWorld, h *handling) { + h.writer.Header().Set("Link", "; rel=preload; as=style") + h.writer.WriteHeader(http.StatusEarlyHints) + } +} + +// setsHeaderAndCookie sets the header and a cookie without writing them. +func setsHeaderAndCookie(match []string) action { + return func(_ *routeWorld, h *handling) { + h.writer.Header().Set(match[1], "plugin") + http.SetCookie(h.writer, &http.Cookie{Name: "feed", Value: "plugin"}) + } +} + +// flushesThroughFlusher flushes through the writer's own http.Flusher. +func flushesThroughFlusher([]string) action { + return func(w *routeWorld, h *handling) { + flusher, ok := h.writer.(http.Flusher) + if !ok { + w.called(errors.New("the writer is not an http.Flusher")) + return + } + flusher.Flush() + w.called(nil) + } +} + +// controls flushes, sets a read deadline or enables full duplex through a response controller. +func controls(match []string) action { + return func(w *routeWorld, h *handling) { + controller := http.NewResponseController(h.writer) + switch match[1] { + case "flushes": + w.called(controller.Flush()) + case "sets a read deadline": + w.called(controller.SetReadDeadline(time.Now().Add(time.Minute))) + default: + w.called(controller.EnableFullDuplex()) + } + } +} + +// called keeps the error a call the handler made on its writer answered. +func (w *routeWorld) called(err error) { + w.mu.Lock() + defer w.mu.Unlock() + w.calls = append(w.calls, err) +} + +// report is the scenario's report, keeping each failure. +func (w *routeWorld) report(failure pluginkit.RouteFailure) { + w.mu.Lock() + defer w.mu.Unlock() + w.failures = append(w.failures, failure) +} + +// applicationAnswer is the scenarios' answer to a request whose plugin handler panicked. +func applicationAnswer(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = io.WriteString(w, "internal error") +} + +// middleware is the scenario's middleware, which notes that it ran and sets a header of its own. +func (w *routeWorld) middleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(writer http.ResponseWriter, r *http.Request) { + w.middlewareRan.Store(true) + writer.Header().Set("X-Middleware", "ran") + if w.middlewarePanic != "" { + panicWith(w.middlewarePanic) + } + next.ServeHTTP(writer, r) + }) +} + +// settings are the route settings every plugin of the scenario is protected under. +func (w *routeWorld) settings() pluginkit.RouteSettings { + return pluginkit.RouteSettings{Wrap: w.middleware, Answer: http.HandlerFunc(applicationAnswer), Report: w.report} +} + +// serves mounts the plugin id on path with a handler that does what the words say, in order. +func (w *routeWorld) serves(id, public, path, words string) error { + plugin := servedPlugin{id: id, path: path, public: public != ""} + for _, part := range strings.Split(words, ", then ") { + act, err := actionOf(part) + if err != nil { + return err + } + plugin.actions = append(plugin.actions, act) + } + w.plugins = append(w.plugins, plugin) + return nil +} + +// handlerOf returns the handler of the plugin, nil when it serves none. +func (w *routeWorld) handlerOf(plugin servedPlugin) http.Handler { + if plugin.noHandler { + return nil + } + return http.HandlerFunc(func(writer http.ResponseWriter, _ *http.Request) { + h := &handling{writer: writer} + for _, act := range plugin.actions { + act(w, h) + } + }) +} + +// counted serves next, counting the request until next returns or panics. +func (w *routeWorld) counted(next http.Handler) http.Handler { + return http.HandlerFunc(func(writer http.ResponseWriter, r *http.Request) { + w.served.Add(1) + defer w.served.Done() + next.ServeHTTP(writer, r) + }) +} + +// start mounts each plugin under its id and starts the server, over HTTP/2 when the scenario asks. +func (w *routeWorld) start() { + mux := http.NewServeMux() + for _, plugin := range w.plugins { + var public []string + if plugin.public { + public = []string{plugin.path} + } + protected := pluginkit.Protect(plugin.id, w.handlerOf(plugin), public, w.settings()) + mux.Handle("/"+plugin.id+"/", http.StripPrefix("/"+plugin.id, w.counted(protected))) + } + w.server = httptest.NewUnstartedServer(mux) + w.server.Config.ErrorLog = log.New(io.Discard, "", 0) + if w.http2 { + w.server.EnableHTTP2 = true + w.server.StartTLS() + w.client = w.server.Client() + w.client.Timeout = clientTimeout + return + } + w.server.Start() + w.client = &http.Client{Transport: &http.Transport{DisableKeepAlives: true}, Timeout: clientTimeout} +} + +// ask has a client ask the plugin id for path, on a connection of its own. +func (w *routeWorld) ask(id, path string) answered { + if w.server == nil { + w.start() + } + response, err := w.client.Get(w.server.URL + "/" + id + path) + if err != nil { + return answered{err: err} + } + defer func() { _ = response.Body.Close() }() + body, err := io.ReadAll(response.Body) + return answered{ + status: response.StatusCode, proto: response.ProtoMajor, header: response.Header, body: string(body), bodyErr: err, + } +} + +// stop stops the scenario's server once the scenario ends. +func (w *routeWorld) stop(ctx context.Context, _ *godog.Scenario, _ error) (context.Context, error) { + if w.server != nil { + w.client.CloseIdleConnections() + w.server.Close() + } + return ctx, nil +} + +// settled waits until every request the server took has left its handler. +func (w *routeWorld) settled() error { + done := make(chan struct{}) + go func() { + w.served.Wait() + close(done) + }() + select { + case <-done: + return nil + case <-time.After(clientTimeout): + return errors.New("a handler is still running") + } +} + +// timedOut reports whether err is a timeout, which a cut answer never is. +func timedOut(err error) bool { + var netErr net.Error + return errors.As(err, &netErr) && netErr.Timeout() +} + +// is checks that the answer came whole with the status and the body. +func (a answered) is(status int, body string) error { + if err := errors.Join(a.err, a.bodyErr); err != nil { + return fmt.Errorf("the client got no whole answer: %w", err) + } + if a.status != status || a.body != body { + return fmt.Errorf("the client got %d %q, want %d %q", a.status, a.body, status, body) + } + return nil +} + +// clientGets checks the last answer. +func (w *routeWorld) clientGets(status int, body string) error { + return w.last.is(status, body) +} + +// askingGets has a client ask the plugin id for path and checks its answer. +func (w *routeWorld) askingGets(id, path string, status int, body string) error { + return w.ask(id, path).is(status, body) +} + +// answerCut checks that the last request failed, or that its 200 answer broke off, and neither for a timeout. +func (w *routeWorld) answerCut() error { + a := w.last + if a.err != nil && !timedOut(a.err) { + return nil + } + if a.err == nil && a.status == http.StatusOK && a.bodyErr != nil && !timedOut(a.bodyErr) { + return nil + } + return fmt.Errorf("the client got %d %q with %v, want the answer cut", a.status, a.body, errors.Join(a.err, a.bodyErr)) +} + +// reportHoldsOne checks that the report holds exactly one failure, of the plugin id for the request, and keeps it. +func (w *routeWorld) reportHoldsOne(id, method, path, how string) error { + if err := w.settled(); err != nil { + return err + } + w.mu.Lock() + defer w.mu.Unlock() + if len(w.failures) != 1 { + return fmt.Errorf("the report holds %d failures, want 1: %+v", len(w.failures), w.failures) + } + got, answeredWanted := w.failures[0], how == "answered" + if got.ID != id || got.Method != method || got.Path != path || got.Answered != answeredWanted { + return fmt.Errorf("the report holds %s %s %q answered %t, want %s %s %q answered %t", + got.ID, got.Method, got.Path, got.Answered, id, method, path, answeredWanted) + } + w.failure = got + return nil +} + +// failureReads checks the text of the kept failure and that its stack holds the panic. +func (w *routeWorld) failureReads(text string) error { + if w.failure.Err == nil || w.failure.Err.Error() != text { + return fmt.Errorf("the failure reads %v, want %q", w.failure.Err, text) + } + if !strings.Contains(w.failure.Stack, "panicWith") { + return fmt.Errorf("the stack does not hold the panic: %s", w.failure.Stack) + } + return nil +} + +// reportHoldsNothing checks that no failure was reported. +func (w *routeWorld) reportHoldsNothing() error { + if err := w.settled(); err != nil { + return err + } + w.mu.Lock() + defer w.mu.Unlock() + if len(w.failures) != 0 { + return fmt.Errorf("the report holds %+v, want nothing", w.failures) + } + return nil +} + +// middlewareRanAs checks whether the middleware ran. +func (w *routeWorld) middlewareRanAs(how string) error { + if ran := w.middlewareRan.Load(); ran != (how == "ran") { + return fmt.Errorf("the middleware ran: %t", ran) + } + return nil +} + +// noHeaderNorCookie checks that the last answer holds neither the header nor a cookie. +func (w *routeWorld) noHeaderNorCookie(name string) error { + if got, cookies := w.last.header.Get(name), w.last.header.Values("Set-Cookie"); got != "" || len(cookies) > 0 { + return fmt.Errorf("the answer holds %s %q and the cookies %q", name, got, cookies) + } + return nil +} + +// middlewareHeader checks that the last answer holds the header the middleware set. +func (w *routeWorld) middlewareHeader() error { + if got := w.last.header.Get("X-Middleware"); got != "ran" { + return fmt.Errorf("the answer holds X-Middleware %q, want %q", got, "ran") + } + return nil +} + +// overHTTP2 checks that the last answer came over HTTP/2. +func (w *routeWorld) overHTTP2() error { + if w.last.proto != 2 { + return fmt.Errorf("the answer came over HTTP/%d", w.last.proto) + } + return nil +} + +// callsSucceeded checks that the handler made a call on its writer and that every one succeeded. +func (w *routeWorld) callsSucceeded() error { + w.mu.Lock() + defer w.mu.Unlock() + if len(w.calls) == 0 { + return errors.New("the handler made no call on its writer") + } + return errors.Join(w.calls...) +} + +// protectWith protects a plugin's routes with the setting the words name left out, keeping what Protect panicked with. +func (w *routeWorld) protectWith(missing string) { + id, settings := "feed", w.settings() + switch missing { + case "no plugin id": + id = "" + case "no middleware": + settings.Wrap = nil + case "no answer": + settings.Answer = nil + default: + settings.Report = nil + } + defer func() { w.protectPanic = recover() }() + pluginkit.Protect(id, http.NotFoundHandler(), nil, settings) +} + +// protectPanicked checks what Protect panicked with. +func (w *routeWorld) protectPanicked(text string) error { + if w.protectPanic == nil || fmt.Sprint(w.protectPanic) != text { + return fmt.Errorf("Protect panicked with %v, want %q", w.protectPanic, text) + } + return nil +} + +// hasRoutesWith gives the plugin id routes, and the public path when the words name one. +func (w *routeWorld) hasRoutesWith(id, path string) { + w.hasRoutes[id] = true + if path != "" { + w.publicPath[id] = path + } +} + +// panicsOnRead makes the plugin id's Routes or PublicPaths panic with the text. +func (w *routeWorld) panicsOnRead(method, id, text string) { + if method == "Routes" { + w.routesPanic[id] = text + return + } + w.pathsPanic[id] = text +} + +// registeredPlugin returns the plugin id offering what the scenario gave it. +func (w *routeWorld) registeredPlugin(id string) namedPlugin { + plain := plainPlugin{id: id} + if !w.hasRoutes[id] { + return plain + } + routed := routedPlugin{plainPlugin: plain, routes: func() http.Handler { + if text := w.routesPanic[id]; text != "" { + panicWith(text) + } + if w.noHandler[id] { + return nil + } + return http.NotFoundHandler() + }} + path, public := w.publicPath[id] + if !public { + return routed + } + return publicPlugin{routedPlugin: routed, paths: func() []string { + if text := w.pathsPanic[id]; text != "" { + panicWith(text) + } + return []string{path} + }} +} + +// readRoutes reads the routes of each registered plugin. +func (w *routeWorld) readRoutes() { + plugins := make([]namedPlugin, 0, len(w.registered)) + for _, id := range w.registered { + plugins = append(plugins, w.registeredPlugin(id)) + } + w.outcome = pluginkit.RoutesEach(plugins) +} + +// routesHold checks that the outcome holds a handler for exactly the listed plugins. +func (w *routeWorld) routesHold(list string) error { + got := slices.Sorted(maps.Keys(w.outcome.Routes)) + if want := slices.Sorted(slices.Values(names(list))); !slices.Equal(got, want) { + return fmt.Errorf("the routes hold %q, want %q", got, want) + } + for id, handler := range w.outcome.Routes { + if handler == nil { + return fmt.Errorf("the routes hold no handler for %q", id) + } + } + return nil +} + +// publicPathsHold checks that the outcome holds the public path for the plugin id and nothing else. +func (w *routeWorld) publicPathsHold(path, id string) error { + if want := map[string][]string{id: {path}}; !maps.EqualFunc(w.outcome.PublicPaths, want, slices.Equal) { + return fmt.Errorf("the public paths hold %q, want %q", w.outcome.PublicPaths, want) + } + return nil +} + +// noneFailed checks that no plugin failed to load its routes. +func (w *routeWorld) noneFailed() error { + if len(w.outcome.Failed) != 0 { + return fmt.Errorf("failed %+v, want none", w.outcome.Failed) + } + return nil +} + +// failedAt checks that the plugin id is the one failure, at the step, with the text, and whether it panicked. +func (w *routeWorld) failedAt(id, step, text, how string) error { + if len(w.outcome.Failed) != 1 { + return fmt.Errorf("failed %+v, want one failure", w.outcome.Failed) + } + got := w.outcome.Failed[0] + if got.ID != id || string(got.Step) != step || got.Err == nil || got.Err.Error() != text || + got.Panicked != (how == "as a panic") { + return fmt.Errorf("failed %+v, want %s at %s with %q %s", got, id, step, text, how) + } + return nil +} + +// givenRoutes registers the steps that set up what a scenario serves or reads. +func givenRoutes(sc *godog.ScenarioContext, w *routeWorld) { + sc.Step(`^the plugin "([^"]*)" serves (the public path )?"([^"]*)" with a handler that (.+)$`, w.serves) + sc.Step(`^the plugin "([^"]*)" serves no handler$`, + func(id string) { w.plugins = append(w.plugins, servedPlugin{id: id, noHandler: true}) }) + sc.Step(`^the middleware panics with "([^"]*)"$`, func(text string) { w.middlewarePanic = text }) + sc.Step(`^the server speaks HTTP/2$`, func() { w.http2 = true }) + sc.Step(`^the registered plugins (.+)$`, func(list string) { w.registered = names(list) }) + sc.Step(`^"([^"]*)" has routes(?: and the public path "([^"]*)")?$`, w.hasRoutesWith) + sc.Step(`^the (Routes|PublicPaths) of "([^"]*)" panics with "([^"]*)"$`, w.panicsOnRead) + sc.Step(`^the Routes of "([^"]*)" answers no handler$`, func(id string) { w.noHandler[id] = true }) +} + +// whenRoutes registers the steps that serve or read a scenario's routes. +func whenRoutes(sc *godog.ScenarioContext, w *routeWorld) { + sc.Step(`^a client asks "([^"]*)" for "([^"]*)"$`, func(id, path string) { w.last = w.ask(id, path) }) + sc.Step(`^the application protects a plugin's routes with (no plugin id|no middleware|no answer|no report)$`, + w.protectWith) + sc.Step(`^the application reads the routes of each plugin$`, w.readRoutes) +} + +// thenServing registers the steps that check what a scenario's server answered and reported. +func thenServing(sc *godog.ScenarioContext, w *routeWorld) { + sc.Step(`^the client gets status (\d+) and "([^"]*)"$`, w.clientGets) + sc.Step(`^a client asking "([^"]*)" for "([^"]*)" gets status (\d+) and "([^"]*)"$`, w.askingGets) + sc.Step(`^the client's answer is cut$`, w.answerCut) + sc.Step(`^the report holds one failure of "([^"]*)" for (\S+) "([^"]*)" `+ + `that the application (answered|did not answer)$`, w.reportHoldsOne) + sc.Step(`^that failure reads "([^"]*)" with the stack of the panic$`, w.failureReads) + sc.Step(`^the report holds nothing$`, w.reportHoldsNothing) + sc.Step(`^the middleware (ran|did not run)$`, w.middlewareRanAs) + sc.Step(`^the answer holds no header "([^"]*)" and no cookie$`, w.noHeaderNorCookie) + sc.Step(`^the answer holds the header the middleware set$`, w.middlewareHeader) + sc.Step(`^the answer came over HTTP/2$`, w.overHTTP2) + sc.Step(`^every call the handler made on its writer succeeded$`, w.callsSucceeded) + sc.Step(`^the protect panics with "([^"]*)"$`, w.protectPanicked) +} + +// thenReading registers the steps that check the routes a scenario read. +func thenReading(sc *godog.ScenarioContext, w *routeWorld) { + sc.Step(`^the routes hold ((?:"[^"]*"(?:, | and )?)+)(?: only)?$`, w.routesHold) + sc.Step(`^the public paths hold "([^"]*)" for "([^"]*)" only$`, w.publicPathsHold) + sc.Step(`^no plugin failed to load its routes$`, w.noneFailed) + sc.Step(`^"([^"]*)" failed at the step "([^"]*)" with "([^"]*)" (as a panic|not as a panic)$`, w.failedAt) +} + +// initializeRoutes gives each routes scenario a world of its own and its steps. +func initializeRoutes() func(*godog.ScenarioContext) { + return func(sc *godog.ScenarioContext) { + w := &routeWorld{ + hasRoutes: map[string]bool{}, publicPath: map[string]string{}, routesPanic: map[string]string{}, + pathsPanic: map[string]string{}, noHandler: map[string]bool{}, + } + sc.After(w.stop) + givenRoutes(sc, w) + whenRoutes(sc, w) + thenServing(sc, w) + thenReading(sc, w) + } +} From 45e3851032178383861f9f971b976a000a7e13ce Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sun, 8 Nov 2026 23:19:39 +0100 Subject: [PATCH 06/10] fix(pluginkit): cut a panic after the middleware began, restore edited headers and refuse a nil Wrap --- pluginkit/protect.go | 5 +- pluginkit/protect_test.go | 76 ++++++++++++++++++ pluginkit/recover.go | 30 ++++++- pluginkit/recover_test.go | 164 ++++++++++++++++++++++---------------- 4 files changed, 203 insertions(+), 72 deletions(-) diff --git a/pluginkit/protect.go b/pluginkit/protect.go index af06576..f456b75 100644 --- a/pluginkit/protect.go +++ b/pluginkit/protect.go @@ -23,12 +23,15 @@ func Protect(id string, handler http.Handler, publicPaths []string, settings Rou } plugin := &recovered{id: id, handler: handler, settings: settings} protected := settings.Wrap(plugin) + if isNil(protected) { + panic("pluginkit: Protect: RouteSettings.Wrap answered no handler") + } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if _, ok := public[r.URL.Path]; ok { plugin.ServeHTTP(w, r) return } - protected.ServeHTTP(w, r) + protected.ServeHTTP(&routeWriter{ResponseWriter: w, http1: r.ProtoMajor == 1}, r) }) } diff --git a/pluginkit/protect_test.go b/pluginkit/protect_test.go index 03d4dc5..374687f 100644 --- a/pluginkit/protect_test.go +++ b/pluginkit/protect_test.go @@ -162,6 +162,78 @@ func TestProtectLetsAPanicOfTheMiddlewareReachTheServer(t *testing.T) { } } +// unwrapping is a writer of the tests' middleware that the server's writer stands under. +type unwrapping struct { + http.ResponseWriter +} + +// Unwrap returns the writer under it. +func (w unwrapping) Unwrap() http.ResponseWriter { + return w.ResponseWriter +} + +// opaque is a writer of the tests' middleware that hides the writer under it. +type opaque struct { + http.ResponseWriter +} + +// beganFirst returns middleware that writes the status 200 through what writerOf builds, then calls next on it. +func beganFirst(writerOf func(http.ResponseWriter) http.ResponseWriter) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + own := writerOf(w) + own.WriteHeader(http.StatusOK) + next.ServeHTTP(own, r) + }) + } +} + +func TestProtectCutsAPanicAfterTheMiddlewareBeganTheAnswer(t *testing.T) { + t.Parallel() + + tests := map[string]func(http.ResponseWriter) http.ResponseWriter{ + "on the writer it was given": func(w http.ResponseWriter) http.ResponseWriter { return w }, + "on a writer of its own": func(w http.ResponseWriter) http.ResponseWriter { return unwrapping{w} }, + } + + for name, writerOf := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + report := &routeReport{} + settings := routeSettings(report) + settings.Wrap = beganFirst(writerOf) + plugin := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { panicWith("boom") }) + + escaped := serveCatching(pluginkit.Protect("feed", plugin, nil, settings), httptest.NewRecorder(), getItems()) + + wantCut(t, escaped) + if got := onlyRouteFailure(t, report); got.Answered { + t.Errorf("reported %+v, want it not answered", got) + } + }) + } +} + +func TestProtectAnswersAPanicUnderAWriterItCannotSeeThrough(t *testing.T) { + t.Parallel() + + report := &routeReport{} + settings := routeSettings(report) + settings.Wrap = func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { next.ServeHTTP(opaque{w}, r) }) + } + plugin := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { panicWith("boom") }) + recorder := httptest.NewRecorder() + + escaped := serveCatching(pluginkit.Protect("feed", plugin, nil, settings), recorder, getItems()) + + wantApplicationAnswer(t, recorder, escaped) + if got := onlyRouteFailure(t, report); !got.Answered { + t.Errorf("reported %+v, want it answered", got) + } +} + func TestProtectRefusesRouteSettingsItCannotServeWith(t *testing.T) { t.Parallel() @@ -190,6 +262,10 @@ func TestProtectRefusesRouteSettingsItCannotServeWith(t *testing.T) { id: "feed", change: func(s *pluginkit.RouteSettings) { s.Report = nil }, want: "pluginkit: Protect: RouteSettings.Report is nil", }, + "Wrap answering no handler": { + id: "feed", change: func(s *pluginkit.RouteSettings) { s.Wrap = func(http.Handler) http.Handler { return nil } }, + want: "pluginkit: Protect: RouteSettings.Wrap answered no handler", + }, } for name, test := range tests { diff --git a/pluginkit/recover.go b/pluginkit/recover.go index caf3d50..6a52cd1 100644 --- a/pluginkit/recover.go +++ b/pluginkit/recover.go @@ -31,13 +31,27 @@ type recovered struct { settings RouteSettings } -// ServeHTTP serves the plugin's handler through a writer that notes whether the plugin's answer began. +// ServeHTTP serves the plugin's handler through a writer that notes whether the answer began, before or in it. func (h *recovered) ServeHTTP(w http.ResponseWriter, r *http.Request) { - writer := &routeWriter{ResponseWriter: w, http1: r.ProtoMajor == 1} - defer h.finish(writer, maps.Clone(w.Header()), r) + writer := &routeWriter{ResponseWriter: w, http1: r.ProtoMajor == 1, begun: begunUnder(w)} + defer h.finish(writer, w.Header().Clone(), r) h.handler.ServeHTTP(writer, r) } +// begunUnder reports whether the answer under w already began, as seen by the route writer w unwraps to. +func begunUnder(w http.ResponseWriter) bool { + for { + switch under := w.(type) { + case *routeWriter: + return under.begun + case interface{ Unwrap() http.ResponseWriter }: + w = under.Unwrap() + default: + return false + } + } +} + // finish recovers a panic of the plugin's handler, reports it, then answers it or cuts the answer the plugin began. func (h *recovered) finish(writer *routeWriter, header http.Header, r *http.Request) { caught := recover() @@ -74,11 +88,19 @@ type routeWriter struct { // WriteHeader writes the status, noting that the answer began unless the status is informational. func (w *routeWriter) WriteHeader(status int) { w.ResponseWriter.WriteHeader(status) - if status >= http.StatusOK || status == http.StatusSwitchingProtocols && w.http1 { + if w.beginsAnswer(status) { w.begun = true } } +// beginsAnswer reports whether status begins the answer: a final status, or a 101 over HTTP/1. +func (w *routeWriter) beginsAnswer(status int) bool { + if status >= http.StatusOK { + return true + } + return status == http.StatusSwitchingProtocols && w.http1 +} + // Write writes part of the body, noting that the answer began. func (w *routeWriter) Write(body []byte) (int, error) { w.begun = true diff --git a/pluginkit/recover_test.go b/pluginkit/recover_test.go index a1efca7..f063fb1 100644 --- a/pluginkit/recover_test.go +++ b/pluginkit/recover_test.go @@ -5,7 +5,9 @@ package pluginkit_test import ( "bufio" "errors" + "fmt" "io" + "maps" "net" "net/http" "net/http/httptest" @@ -39,6 +41,63 @@ func getItems() *http.Request { return httptest.NewRequest(http.MethodGet, "/items", nil) } +// routeShape returns failure without its error and stack, which the tests check on their own. +func routeShape(failure pluginkit.RouteFailure) pluginkit.RouteFailure { + failure.Err, failure.Stack = nil, "" + return failure +} + +// onlyRouteFailure returns the one failure report holds, ending the test when it holds another count. +func onlyRouteFailure(t *testing.T, report *routeReport) pluginkit.RouteFailure { + t.Helper() + failures := report.held() + if len(failures) != 1 { + t.Fatalf("reported %+v, want one failure", failures) + } + return failures[0] +} + +// wantNoFailure checks that report holds no failure. +func wantNoFailure(t *testing.T, report *routeReport) { + t.Helper() + if failures := report.held(); len(failures) != 0 { + t.Errorf("reported %+v, want nothing", failures) + } +} + +// wantRecorded checks the status and the body the recorder holds. +func wantRecorded(t *testing.T, recorder *httptest.ResponseRecorder, status int, body string) { + t.Helper() + if recorder.Code != status || recorder.Body.String() != body { + t.Errorf("answered %d %q, want %d %q", recorder.Code, recorder.Body.String(), status, body) + } +} + +// wantApplicationAnswer checks that the recorder holds the tests' answer to a panic and that no panic reached it. +func wantApplicationAnswer(t *testing.T, recorder *httptest.ResponseRecorder, escaped any) { + t.Helper() + if escaped != nil { + t.Errorf("the server got the panic %v, want none", escaped) + } + wantRecorded(t, recorder, http.StatusInternalServerError, "internal error") +} + +// wantCut checks that the server got http.ErrAbortHandler, which cuts the answer. +func wantCut(t *testing.T, escaped any) { + t.Helper() + if escaped != http.ErrAbortHandler { + t.Errorf("the server got the panic %v, want http.ErrAbortHandler", escaped) + } +} + +// wantFailureText checks the text of the failure's error. +func wantFailureText(t *testing.T, failure pluginkit.RouteFailure, want string) { + t.Helper() + if got := fmt.Sprint(failure.Err); got != want { + t.Errorf("the failure reads %q, want %q", got, want) + } +} + // informedWriter is a recorder that sends a status below 200 aside, as a server sends an informational answer. type informedWriter struct { *httptest.ResponseRecorder @@ -107,8 +166,9 @@ func TestRecoverAnswersAPanicBeforeTheAnswerBegan(t *testing.T) { report := &routeReport{} plugin := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("X-Feed", "plugin") + w.Header()["X-Wrapped"][0] = "edited" w.Header().Add("X-Wrapped", "plugin") + w.Header().Set("X-Feed", "plugin") http.SetCookie(w, &http.Cookie{Name: "feed", Value: "plugin"}) panicWith("boom") }) @@ -116,25 +176,17 @@ func TestRecoverAnswersAPanicBeforeTheAnswerBegan(t *testing.T) { escaped := serveCatching(protectedFeed(plugin, report), recorder, httptest.NewRequest(http.MethodPost, "/items", nil)) - answer := recorder.Result() - if escaped != nil || answer.StatusCode != http.StatusInternalServerError || - recorder.Body.String() != "internal error" { - t.Fatalf("answered %d %q with the panic %v, want the application's answer", answer.StatusCode, - recorder.Body.String(), escaped) + wantApplicationAnswer(t, recorder, escaped) + middlewareOnly := http.Header{"X-Wrapped": {"yes"}} + if got := recorder.Result().Header; !maps.EqualFunc(got, middlewareOnly, slices.Equal) { + t.Errorf("the answer holds %v, want only the middleware's header %v", got, middlewareOnly) } - if answer.Header.Get("X-Feed") != "" || len(answer.Cookies()) != 0 || - !slices.Equal(answer.Header.Values("X-Wrapped"), []string{"yes"}) { - t.Errorf("the answer holds %v, want only the middleware's header", answer.Header) - } - failures := report.held() - if len(failures) != 1 { - t.Fatalf("reported %+v, want one failure", failures) - } - got := failures[0] - if got.ID != "feed" || got.Method != http.MethodPost || got.Path != "/items" || !got.Answered || - got.Err == nil || got.Err.Error() != "pluginkit: feed route panicked: boom" { - t.Errorf("reported %+v, want the answered panic of feed for POST /items", got) + got := onlyRouteFailure(t, report) + want := pluginkit.RouteFailure{ID: "feed", Method: http.MethodPost, Path: "/items", Answered: true} + if routeShape(got) != want { + t.Errorf("reported %+v, want %+v", got, want) } + wantFailureText(t, got, "pluginkit: feed route panicked: boom") if !strings.Contains(got.Stack, "pluginkit_test.panicWith") { t.Errorf("the stack does not hold the panic:\n%s", got.Stack) } @@ -165,11 +217,9 @@ func TestRecoverCutsAnAnswerThatBegan(t *testing.T) { escaped := serveCatching(protectedFeed(plugin, report), writer, getItems()) - if escaped != http.ErrAbortHandler { - t.Errorf("the server got the panic %v, want http.ErrAbortHandler", escaped) - } - if failures := report.held(); len(failures) != 1 || failures[0].Answered { - t.Errorf("reported %+v, want one failure not answered", failures) + wantCut(t, escaped) + if got := onlyRouteFailure(t, report); got.Answered { + t.Errorf("reported %+v, want it not answered", got) } }) } @@ -199,14 +249,12 @@ func TestRecoverAnswersAfterAnInformationalStatus(t *testing.T) { escaped := serveCatching(protectedFeed(plugin, report), writer, request) - if escaped != nil || writer.Code != http.StatusInternalServerError { - t.Errorf("answered %d with the panic %v, want the application's answer", writer.Code, escaped) - } + wantApplicationAnswer(t, writer.ResponseRecorder, escaped) if !slices.Equal(writer.informed, []int{test.status}) { t.Errorf("informed %v, want %d", writer.informed, test.status) } - if failures := report.held(); len(failures) != 1 || !failures[0].Answered { - t.Errorf("reported %+v, want one answered failure", failures) + if got := onlyRouteFailure(t, report); !got.Answered { + t.Errorf("reported %+v, want it answered", got) } }) } @@ -221,13 +269,8 @@ func TestRecoverAnswersAStatusTheServerRefused(t *testing.T) { escaped := serveCatching(protectedFeed(plugin, report), recorder, getItems()) - if escaped != nil || recorder.Code != http.StatusInternalServerError { - t.Errorf("answered %d with the panic %v, want the application's answer", recorder.Code, escaped) - } - want := "pluginkit: feed route panicked: invalid WriteHeader code 42" - if failures := report.held(); len(failures) != 1 || failures[0].Err.Error() != want { - t.Errorf("reported %+v, want %q", failures, want) - } + wantApplicationAnswer(t, recorder, escaped) + wantFailureText(t, onlyRouteFailure(t, report), "pluginkit: feed route panicked: invalid WriteHeader code 42") } func TestRecoverClosesAConnectionTheHandlerHijacked(t *testing.T) { @@ -258,15 +301,14 @@ func TestRecoverClosesAConnectionTheHandlerHijacked(t *testing.T) { escaped := serveCatching(protectedFeed(plugin, report), writer, getItems()) - if escaped != http.ErrAbortHandler { - t.Errorf("the server got the panic %v, want http.ErrAbortHandler", escaped) - } + wantCut(t, escaped) _ = client.SetReadDeadline(time.Now().Add(time.Second)) if _, err := client.Read(make([]byte, 1)); !errors.Is(err, io.EOF) { t.Errorf("reading the hijacked connection answered %v, want io.EOF", err) } - if failures := report.held(); len(failures) != test.reported || slices.ContainsFunc(failures, - func(failure pluginkit.RouteFailure) bool { return failure.Answered }) { + failures := report.held() + answered := slices.ContainsFunc(failures, func(failure pluginkit.RouteFailure) bool { return failure.Answered }) + if len(failures) != test.reported || answered { t.Errorf("reported %+v, want %d failures not answered", failures, test.reported) } }) @@ -287,11 +329,9 @@ func TestRecoverAnswersAPanicAfterAHijackThatFailed(t *testing.T) { escaped := serveCatching(protectedFeed(plugin, report), writer, getItems()) - if escaped != nil || writer.Code != http.StatusInternalServerError { - t.Errorf("answered %d with the panic %v, want the application's answer", writer.Code, escaped) - } - if failures := report.held(); len(failures) != 1 || !failures[0].Answered { - t.Errorf("reported %+v, want one answered failure", failures) + wantApplicationAnswer(t, writer.ResponseRecorder, escaped) + if got := onlyRouteFailure(t, report); !got.Answered { + t.Errorf("reported %+v, want it answered", got) } } @@ -304,15 +344,9 @@ func TestRecoverLeavesAnAbortToTheServer(t *testing.T) { escaped := serveCatching(protectedFeed(plugin, report), recorder, getItems()) - if escaped != http.ErrAbortHandler { - t.Errorf("the server got the panic %v, want http.ErrAbortHandler", escaped) - } - if recorder.Body.Len() != 0 || recorder.Result().StatusCode != http.StatusOK { - t.Errorf("answered %d %q, want nothing written", recorder.Code, recorder.Body.String()) - } - if failures := report.held(); len(failures) != 0 { - t.Errorf("reported %+v, want nothing for an abort", failures) - } + wantCut(t, escaped) + wantRecorded(t, recorder, http.StatusOK, "") + wantNoFailure(t, report) } func TestRecoverAnswersAPluginWithNoHandler(t *testing.T) { @@ -323,13 +357,9 @@ func TestRecoverAnswersAPluginWithNoHandler(t *testing.T) { escaped := serveCatching(protectedFeed(nil, report), recorder, getItems()) - if escaped != nil || recorder.Code != http.StatusInternalServerError { - t.Errorf("answered %d with the panic %v, want the application's answer", recorder.Code, escaped) - } - want := "pluginkit: feed route panicked: runtime error: invalid memory address or nil pointer dereference" - if failures := report.held(); len(failures) != 1 || failures[0].Err.Error() != want { - t.Errorf("reported %+v, want %q", failures, want) - } + wantApplicationAnswer(t, recorder, escaped) + wantFailureText(t, onlyRouteFailure(t, report), + "pluginkit: feed route panicked: runtime error: invalid memory address or nil pointer dereference") } func TestRecoverLeavesAnAnswerThatDidNotPanicAlone(t *testing.T) { @@ -345,14 +375,14 @@ func TestRecoverLeavesAnAnswerThatDidNotPanicAlone(t *testing.T) { escaped := serveCatching(protectedFeed(plugin, report), recorder, getItems()) - if escaped != nil || recorder.Code != http.StatusCreated || recorder.Body.String() != "created" || - recorder.Header().Get("X-Feed") != "plugin" { - t.Errorf("answered %d %q %v with the panic %v, want the plugin's answer", recorder.Code, - recorder.Body.String(), recorder.Header(), escaped) + if escaped != nil { + t.Errorf("the server got the panic %v, want none", escaped) } - if failures := report.held(); len(failures) != 0 { - t.Errorf("reported %+v, want nothing", failures) + wantRecorded(t, recorder, http.StatusCreated, "created") + if got := recorder.Header().Get("X-Feed"); got != "plugin" { + t.Errorf("the answer holds X-Feed %q, want the plugin's", got) } + wantNoFailure(t, report) } func TestRecoverSetsAReadDeadlineOnTheServersWriter(t *testing.T) { From c9491b4ad6d8d938729657dd93232401734f8f6f Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sun, 8 Nov 2026 23:19:40 +0100 Subject: [PATCH 07/10] test(pluginkit): compare the RoutesEach failures as whole values --- pluginkit/routes_test.go | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/pluginkit/routes_test.go b/pluginkit/routes_test.go index dd036dd..e329414 100644 --- a/pluginkit/routes_test.go +++ b/pluginkit/routes_test.go @@ -3,6 +3,7 @@ package pluginkit_test import ( + "fmt" "maps" "net/http" "slices" @@ -97,19 +98,30 @@ func TestRoutesEachFailsAPluginWhoseRoutesFailToLoad(t *testing.T) { if len(outcome.PublicPaths) != 0 { t.Errorf("PublicPaths = %v, want none", outcome.PublicPaths) } - if len(outcome.Failed) != 1 { - t.Fatalf("Failed = %+v, want one failure", outcome.Failed) + shapes, texts := splitFailures(outcome.Failed) + want := []pluginkit.Failure{{ID: "feed", Step: pluginkit.StepRoutes, Panicked: test.panicked}} + if !slices.Equal(shapes, want) { + t.Errorf("Failed = %+v, want %+v", outcome.Failed, want) } - got := outcome.Failed[0] - if got.ID != "feed" || got.Step != pluginkit.StepRoutes || got.Panicked != test.panicked || - got.Err.Error() != test.want { - t.Errorf("Failed = %+v, want feed at %s with %q panicked %t", got, pluginkit.StepRoutes, test.want, - test.panicked) + if !slices.Equal(texts, []string{test.want}) { + t.Errorf("Failed reads %q, want %q", texts, test.want) } }) } } +// splitFailures returns each failure without its error, then the text of each error, in the order of failures. +func splitFailures(failures []pluginkit.Failure) ([]pluginkit.Failure, []string) { + shapes := make([]pluginkit.Failure, 0, len(failures)) + texts := make([]string, 0, len(failures)) + for _, failure := range failures { + texts = append(texts, fmt.Sprint(failure.Err)) + failure.Err = nil + shapes = append(shapes, failure) + } + return shapes, texts +} + func TestRoutesEachListsTheFailuresInTheOrderOfThePlugins(t *testing.T) { t.Parallel() @@ -129,10 +141,10 @@ func TestRoutesEachAnswersEmptyMapsForNoPlugins(t *testing.T) { outcome := pluginkit.RoutesEach[pluginkit.Plugin](nil) - if outcome.Routes == nil || outcome.PublicPaths == nil || len(outcome.Routes)+len(outcome.PublicPaths) != 0 { - t.Errorf("RoutesEach(nil) = %+v, want two empty maps", outcome) + if outcome.Routes == nil || outcome.PublicPaths == nil { + t.Errorf("RoutesEach(nil) = %+v, want two maps", outcome) } - if outcome.Failed != nil { - t.Errorf("Failed = %+v, want none", outcome.Failed) + if len(outcome.Routes)+len(outcome.PublicPaths) != 0 || outcome.Failed != nil { + t.Errorf("RoutesEach(nil) = %+v, want nothing in it", outcome) } } From b794d549ad78c1e3b074b1c5a642967d6af7e983 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sun, 8 Nov 2026 23:19:40 +0100 Subject: [PATCH 08/10] test(pluginkit): pin the cut after the middleware began the answer and compare whole failures --- pluginkit/features/routes.feature | 7 +++ pluginkit/features/steps_routes_test.go | 70 ++++++++++++++----------- 2 files changed, 47 insertions(+), 30 deletions(-) diff --git a/pluginkit/features/routes.feature b/pluginkit/features/routes.feature index e453d62..31baa9e 100644 --- a/pluginkit/features/routes.feature +++ b/pluginkit/features/routes.feature @@ -108,6 +108,13 @@ Feature: Serving each plugin's routes and reading them at boot And the middleware did not run And the report holds one failure of "feed" for GET "/rss.xml" that the application answered + Scenario: A route that panics after the middleware began the answer is cut + Given the plugin "feed" serves "/items" with a handler that panics with "boom" + And the middleware writes the status 200 before it calls the plugin + When a client asks "feed" for "/items" + Then the client's answer is cut + And the report holds one failure of "feed" for GET "/items" that the application did not answer + Scenario: A panic in the application's middleware is never blamed on the plugin Given the plugin "feed" serves "/items" with a handler that answers status 200 and "listed" And the middleware panics with "session store down" diff --git a/pluginkit/features/steps_routes_test.go b/pluginkit/features/steps_routes_test.go index f7aec7e..f06f6ea 100644 --- a/pluginkit/features/steps_routes_test.go +++ b/pluginkit/features/steps_routes_test.go @@ -91,26 +91,27 @@ func (p publicPlugin) PublicPaths() []string { // routeWorld is what one scenario of the routes feature serves, reads and finds. type routeWorld struct { - http2 bool - plugins []servedPlugin - middlewarePanic string - middlewareRan atomic.Bool - server *httptest.Server - client *http.Client - served sync.WaitGroup - mu sync.Mutex - failures []pluginkit.RouteFailure - calls []error - last answered - failure pluginkit.RouteFailure - protectPanic any - registered []string - hasRoutes map[string]bool - publicPath map[string]string - routesPanic map[string]string - pathsPanic map[string]string - noHandler map[string]bool - outcome pluginkit.RoutesOutcome + http2 bool + plugins []servedPlugin + middlewarePanic string + middlewareBegins bool + middlewareRan atomic.Bool + server *httptest.Server + client *http.Client + served sync.WaitGroup + mu sync.Mutex + failures []pluginkit.RouteFailure + calls []error + last answered + failure pluginkit.RouteFailure + protectPanic any + registered []string + hasRoutes map[string]bool + publicPath map[string]string + routesPanic map[string]string + pathsPanic map[string]string + noHandler map[string]bool + outcome pluginkit.RoutesOutcome } // panicWith panics with value, so the stack of a scenario's panic names it. @@ -290,6 +291,9 @@ func (w *routeWorld) middleware(next http.Handler) http.Handler { if w.middlewarePanic != "" { panicWith(w.middlewarePanic) } + if w.middlewareBegins { + writer.WriteHeader(http.StatusOK) + } next.ServeHTTP(writer, r) }) } @@ -429,10 +433,11 @@ func (w *routeWorld) askingGets(id, path string, status int, body string) error // answerCut checks that the last request failed, or that its 200 answer broke off, and neither for a timeout. func (w *routeWorld) answerCut() error { a := w.last - if a.err != nil && !timedOut(a.err) { - return nil + cause := a.err + if cause == nil && a.status == http.StatusOK { + cause = a.bodyErr } - if a.err == nil && a.status == http.StatusOK && a.bodyErr != nil && !timedOut(a.bodyErr) { + if cause != nil && !timedOut(cause) { return nil } return fmt.Errorf("the client got %d %q with %v, want the answer cut", a.status, a.body, errors.Join(a.err, a.bodyErr)) @@ -448,10 +453,11 @@ func (w *routeWorld) reportHoldsOne(id, method, path, how string) error { if len(w.failures) != 1 { return fmt.Errorf("the report holds %d failures, want 1: %+v", len(w.failures), w.failures) } - got, answeredWanted := w.failures[0], how == "answered" - if got.ID != id || got.Method != method || got.Path != path || got.Answered != answeredWanted { - return fmt.Errorf("the report holds %s %s %q answered %t, want %s %s %q answered %t", - got.ID, got.Method, got.Path, got.Answered, id, method, path, answeredWanted) + got := w.failures[0] + shape := pluginkit.RouteFailure{ID: got.ID, Method: got.Method, Path: got.Path, Answered: got.Answered} + want := pluginkit.RouteFailure{ID: id, Method: method, Path: path, Answered: how == "answered"} + if shape != want { + return fmt.Errorf("the report holds %+v, want %+v", shape, want) } w.failure = got return nil @@ -637,9 +643,12 @@ func (w *routeWorld) failedAt(id, step, text, how string) error { return fmt.Errorf("failed %+v, want one failure", w.outcome.Failed) } got := w.outcome.Failed[0] - if got.ID != id || string(got.Step) != step || got.Err == nil || got.Err.Error() != text || - got.Panicked != (how == "as a panic") { - return fmt.Errorf("failed %+v, want %s at %s with %q %s", got, id, step, text, how) + shape := pluginkit.Failure{ID: got.ID, Step: got.Step, Panicked: got.Panicked} + if want := (pluginkit.Failure{ID: id, Step: pluginkit.Step(step), Panicked: how == "as a panic"}); shape != want { + return fmt.Errorf("failed %+v, want %+v", shape, want) + } + if read := fmt.Sprint(got.Err); read != text { + return fmt.Errorf("the failure reads %q, want %q", read, text) } return nil } @@ -650,6 +659,7 @@ func givenRoutes(sc *godog.ScenarioContext, w *routeWorld) { sc.Step(`^the plugin "([^"]*)" serves no handler$`, func(id string) { w.plugins = append(w.plugins, servedPlugin{id: id, noHandler: true}) }) sc.Step(`^the middleware panics with "([^"]*)"$`, func(text string) { w.middlewarePanic = text }) + sc.Step(`^the middleware writes the status 200 before it calls the plugin$`, func() { w.middlewareBegins = true }) sc.Step(`^the server speaks HTTP/2$`, func() { w.http2 = true }) sc.Step(`^the registered plugins (.+)$`, func(list string) { w.registered = names(list) }) sc.Step(`^"([^"]*)" has routes(?: and the public path "([^"]*)")?$`, w.hasRoutesWith) From 15ce25c702e693f459444e777ccd4acce9334a28 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sun, 8 Nov 2026 23:35:47 +0100 Subject: [PATCH 09/10] fix(pluginkit): see the middleware begin the answer through any writer it wraps --- pluginkit/protect.go | 8 ++++++-- pluginkit/protect_test.go | 5 +++-- pluginkit/recover.go | 28 ++++++++++++---------------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/pluginkit/protect.go b/pluginkit/protect.go index f456b75..e8ff471 100644 --- a/pluginkit/protect.go +++ b/pluginkit/protect.go @@ -2,7 +2,10 @@ package pluginkit -import "net/http" +import ( + "context" + "net/http" +) // RouteSettings are what every plugin's routes are protected under, one value an application reuses for each plugin. type RouteSettings struct { @@ -31,7 +34,8 @@ func Protect(id string, handler http.Handler, publicPaths []string, settings Rou plugin.ServeHTTP(w, r) return } - protected.ServeHTTP(&routeWriter{ResponseWriter: w, http1: r.ProtoMajor == 1}, r) + outer := &routeWriter{ResponseWriter: w, http1: r.ProtoMajor == 1} + protected.ServeHTTP(outer, r.WithContext(context.WithValue(r.Context(), middlewareWriter{}, outer))) }) } diff --git a/pluginkit/protect_test.go b/pluginkit/protect_test.go index 374687f..fee46f8 100644 --- a/pluginkit/protect_test.go +++ b/pluginkit/protect_test.go @@ -192,8 +192,9 @@ func TestProtectCutsAPanicAfterTheMiddlewareBeganTheAnswer(t *testing.T) { t.Parallel() tests := map[string]func(http.ResponseWriter) http.ResponseWriter{ - "on the writer it was given": func(w http.ResponseWriter) http.ResponseWriter { return w }, - "on a writer of its own": func(w http.ResponseWriter) http.ResponseWriter { return unwrapping{w} }, + "on the writer it was given": func(w http.ResponseWriter) http.ResponseWriter { return w }, + "on a writer of its own": func(w http.ResponseWriter) http.ResponseWriter { return unwrapping{w} }, + "on a writer that hides another": func(w http.ResponseWriter) http.ResponseWriter { return opaque{w} }, } for name, writerOf := range tests { diff --git a/pluginkit/recover.go b/pluginkit/recover.go index 6a52cd1..e99a6fe 100644 --- a/pluginkit/recover.go +++ b/pluginkit/recover.go @@ -31,25 +31,20 @@ type recovered struct { settings RouteSettings } -// ServeHTTP serves the plugin's handler through a writer that notes whether the answer began, before or in it. +// middlewareWriter is the context key of the writer Protect hands the application's middleware. +type middlewareWriter struct{} + +// ServeHTTP serves the plugin's handler through a writer that notes whether the plugin's answer began. func (h *recovered) ServeHTTP(w http.ResponseWriter, r *http.Request) { - writer := &routeWriter{ResponseWriter: w, http1: r.ProtoMajor == 1, begun: begunUnder(w)} + writer := &routeWriter{ResponseWriter: w, http1: r.ProtoMajor == 1} defer h.finish(writer, w.Header().Clone(), r) h.handler.ServeHTTP(writer, r) } -// begunUnder reports whether the answer under w already began, as seen by the route writer w unwraps to. -func begunUnder(w http.ResponseWriter) bool { - for { - switch under := w.(type) { - case *routeWriter: - return under.begun - case interface{ Unwrap() http.ResponseWriter }: - w = under.Unwrap() - default: - return false - } - } +// begunBefore reports whether the writer Protect handed the middleware serving r saw the answer begin. +func begunBefore(r *http.Request) bool { + under, ok := r.Context().Value(middlewareWriter{}).(*routeWriter) + return ok && under.begun } // finish recovers a panic of the plugin's handler, reports it, then answers it or cuts the answer the plugin began. @@ -64,11 +59,12 @@ func (h *recovered) finish(writer *routeWriter, header http.Header, r *http.Requ if caught == http.ErrAbortHandler { panic(caught) } + begun := writer.begun || begunBefore(r) h.settings.Report(RouteFailure{ - ID: h.id, Method: r.Method, Path: r.URL.Path, Answered: !writer.begun, + ID: h.id, Method: r.Method, Path: r.URL.Path, Answered: !begun, Err: panicError("pluginkit: " + h.id + " route panicked: " + describe(caught)), Stack: string(debug.Stack()), }) - if writer.begun { + if begun { panic(http.ErrAbortHandler) } clear(writer.Header()) From e42ac5d5c5ce3475945ef19dc05a3d253357b302 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sun, 8 Nov 2026 23:49:12 +0100 Subject: [PATCH 10/10] fix(pluginkit): read whether the answer began safely from a handler the middleware runs apart --- pluginkit/protect_test.go | 28 ++++++++++++++++++++++++++++ pluginkit/recover.go | 18 ++++++++++-------- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/pluginkit/protect_test.go b/pluginkit/protect_test.go index fee46f8..438c49a 100644 --- a/pluginkit/protect_test.go +++ b/pluginkit/protect_test.go @@ -10,6 +10,7 @@ import ( "slices" "sync" "testing" + "time" "github.com/gopherium/framework/pluginkit" ) @@ -216,6 +217,33 @@ func TestProtectCutsAPanicAfterTheMiddlewareBeganTheAnswer(t *testing.T) { } } +func TestProtectReportsAPanicAfterTheMiddlewareTimedOut(t *testing.T) { + t.Parallel() + + reported := make(chan pluginkit.RouteFailure, 1) + settings := routeSettings(&routeReport{}) + settings.Report = func(failure pluginkit.RouteFailure) { reported <- failure } + settings.Wrap = func(next http.Handler) http.Handler { return http.TimeoutHandler(next, time.Millisecond, "too slow") } + plugin := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + <-r.Context().Done() + panicWith("boom") + }) + recorder := httptest.NewRecorder() + + escaped := serveCatching(pluginkit.Protect("feed", plugin, nil, settings), recorder, getItems()) + + if escaped != nil { + t.Errorf("the server got the panic %v, want none", escaped) + } + wantRecorded(t, recorder, http.StatusServiceUnavailable, "too slow") + select { + case failure := <-reported: + wantFailureText(t, failure, "pluginkit: feed route panicked: boom") + case <-time.After(10 * time.Second): + t.Fatal("the panic after the timeout was never reported") + } +} + func TestProtectAnswersAPanicUnderAWriterItCannotSeeThrough(t *testing.T) { t.Parallel() diff --git a/pluginkit/recover.go b/pluginkit/recover.go index e99a6fe..757e16e 100644 --- a/pluginkit/recover.go +++ b/pluginkit/recover.go @@ -8,6 +8,7 @@ import ( "net" "net/http" "runtime/debug" + "sync/atomic" ) // RouteFailure is one panic of one plugin's handler while it served one request. @@ -44,7 +45,7 @@ func (h *recovered) ServeHTTP(w http.ResponseWriter, r *http.Request) { // begunBefore reports whether the writer Protect handed the middleware serving r saw the answer begin. func begunBefore(r *http.Request) bool { under, ok := r.Context().Value(middlewareWriter{}).(*routeWriter) - return ok && under.begun + return ok && under.begun.Load() } // finish recovers a panic of the plugin's handler, reports it, then answers it or cuts the answer the plugin began. @@ -59,7 +60,7 @@ func (h *recovered) finish(writer *routeWriter, header http.Header, r *http.Requ if caught == http.ErrAbortHandler { panic(caught) } - begun := writer.begun || begunBefore(r) + begun := writer.begun.Load() || begunBefore(r) h.settings.Report(RouteFailure{ ID: h.id, Method: r.Method, Path: r.URL.Path, Answered: !begun, Err: panicError("pluginkit: " + h.id + " route panicked: " + describe(caught)), Stack: string(debug.Stack()), @@ -72,12 +73,12 @@ func (h *recovered) finish(writer *routeWriter, header http.Header, r *http.Requ h.settings.Answer.ServeHTTP(writer.ResponseWriter, r) } -// routeWriter is the writer a plugin's handler answers through, noting whether its answer began. +// routeWriter is the writer a plugin's handler, or the middleware before it, answers through, noting when it began. type routeWriter struct { http.ResponseWriter // http1 makes a 101 status begin the answer, as only HTTP/1 switches protocols with it. http1 bool - begun bool + begun atomic.Bool hijacked net.Conn } @@ -85,7 +86,7 @@ type routeWriter struct { func (w *routeWriter) WriteHeader(status int) { w.ResponseWriter.WriteHeader(status) if w.beginsAnswer(status) { - w.begun = true + w.begun.Store(true) } } @@ -99,7 +100,7 @@ func (w *routeWriter) beginsAnswer(status int) bool { // Write writes part of the body, noting that the answer began. func (w *routeWriter) Write(body []byte) (int, error) { - w.begun = true + w.begun.Store(true) return w.ResponseWriter.Write(body) } @@ -110,7 +111,7 @@ func (w *routeWriter) Flush() { // FlushError sends what the handler wrote so far, writing the status 200 first when nothing began the answer. func (w *routeWriter) FlushError() error { - if !w.begun { + if !w.begun.Load() { w.WriteHeader(http.StatusOK) } return http.NewResponseController(w.ResponseWriter).Flush() @@ -120,7 +121,8 @@ func (w *routeWriter) FlushError() error { func (w *routeWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { conn, buffered, err := http.NewResponseController(w.ResponseWriter).Hijack() if err == nil { - w.begun, w.hijacked = true, conn + w.begun.Store(true) + w.hijacked = conn } return conn, buffered, err }