From 75a4590f18cac5f38a42e06f54d3a27cbec4cb12 Mon Sep 17 00:00:00 2001 From: juancwu Date: Sat, 25 Apr 2026 20:41:52 +0000 Subject: [PATCH] use errx and splinter packages --- go.mod | 5 +++++ go.sum | 4 ++++ pkg/middleware/logger.go | 10 +++++++-- pkg/middleware/logger_test.go | 36 ++++++++++++++++++++------------ pkg/middleware/recoverer.go | 18 +++++++++++++--- pkg/middleware/recoverer_test.go | 34 ++++++++++++++++++++++++++---- pkg/router/pattern.go | 14 +++++++++---- 7 files changed, 95 insertions(+), 26 deletions(-) create mode 100644 go.sum diff --git a/go.mod b/go.mod index 329bb20..b5e1214 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,8 @@ module git.juancwu.dev/juancwu/lightmux go 1.26.2 + +require ( + git.juancwu.dev/juancwu/errx v0.1.0 + git.juancwu.dev/juancwu/splinter v0.1.0 +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..207ed5b --- /dev/null +++ b/go.sum @@ -0,0 +1,4 @@ +git.juancwu.dev/juancwu/errx v0.1.0 h1:92yA0O1BkKGXcoEiWtxwH/ztXCjoV1KSTMtKpm3gd2w= +git.juancwu.dev/juancwu/errx v0.1.0/go.mod h1:7jNhBOwcZ/q7zDD6mln3QCJBYZ8T6h+dAdxVfykprTk= +git.juancwu.dev/juancwu/splinter v0.1.0 h1:ZGvvzyi24hZw/yFAwpUsHtj+q+fh9I2KIGmOAILWD5Q= +git.juancwu.dev/juancwu/splinter v0.1.0/go.mod h1:dAYsRQfS6tqWynEGz8xMCtIJUN7+KIp3jLE7kgO3yKE= diff --git a/pkg/middleware/logger.go b/pkg/middleware/logger.go index a535b43..ced4a20 100644 --- a/pkg/middleware/logger.go +++ b/pkg/middleware/logger.go @@ -1,9 +1,10 @@ package middleware import ( - "log" "net/http" "time" + + "git.juancwu.dev/juancwu/splinter" ) type statusRecorder struct { @@ -33,6 +34,11 @@ func Logger(next http.Handler) http.Handler { start := time.Now() rec := &statusRecorder{ResponseWriter: w, status: http.StatusOK} next.ServeHTTP(rec, r) - log.Printf("%s %s %d %s", r.Method, r.URL.Path, rec.status, time.Since(start)) + splinter.Info("http.request", + "method", r.Method, + "path", r.URL.Path, + "status", rec.status, + "duration", time.Since(start), + ) }) } diff --git a/pkg/middleware/logger_test.go b/pkg/middleware/logger_test.go index 3c886bf..37f251b 100644 --- a/pkg/middleware/logger_test.go +++ b/pkg/middleware/logger_test.go @@ -2,18 +2,29 @@ package middleware import ( "bytes" - "log" "net/http" "net/http/httptest" "strings" "testing" + + "git.juancwu.dev/juancwu/splinter" ) -func TestLogger(t *testing.T) { +func captureSplinter(t *testing.T) *bytes.Buffer { + t.Helper() var buf bytes.Buffer - orig := log.Default().Writer() - log.Default().SetOutput(&buf) - defer log.Default().SetOutput(orig) + logger := splinter.New(splinter.WithStream(splinter.NewConsoleStream( + splinter.ConsoleJSON, + splinter.LevelDebug, + splinter.ConsoleWriter(&buf), + ))) + prev := splinter.SetDefault(logger) + t.Cleanup(func() { splinter.SetDefault(prev) }) + return &buf +} + +func TestLogger(t *testing.T) { + buf := captureSplinter(t) h := Logger(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusTeapot) @@ -26,23 +37,22 @@ func TestLogger(t *testing.T) { t.Errorf("status code = %d, want 418", rr.Code) } out := buf.String() - if !strings.Contains(out, "GET /foo 418") { - t.Errorf("log output missing expected fields: %q", out) + for _, want := range []string{`"method":"GET"`, `"path":"/foo"`, `"status":418`} { + if !strings.Contains(out, want) { + t.Errorf("log output missing %s\nfull output: %s", want, out) + } } } func TestLoggerDefaultStatusOK(t *testing.T) { - var buf bytes.Buffer - orig := log.Default().Writer() - log.Default().SetOutput(&buf) - defer log.Default().SetOutput(orig) + buf := captureSplinter(t) h := Logger(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write([]byte("hi")) })) h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/", nil)) - if !strings.Contains(buf.String(), "200") { - t.Errorf("expected default 200 in log, got %q", buf.String()) + if !strings.Contains(buf.String(), `"status":200`) { + t.Errorf("expected default status 200 in log, got %q", buf.String()) } } diff --git a/pkg/middleware/recoverer.go b/pkg/middleware/recoverer.go index b7a8258..37a3262 100644 --- a/pkg/middleware/recoverer.go +++ b/pkg/middleware/recoverer.go @@ -4,15 +4,27 @@ import ( "log" "net/http" "runtime/debug" + + "git.juancwu.dev/juancwu/errx" ) +const recovererOp = "middleware.Recoverer" + func Recoverer(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { defer func() { - if rec := recover(); rec != nil { - log.Printf("panic: %v\n%s", rec, debug.Stack()) - http.Error(w, "Internal Server Error", http.StatusInternalServerError) + rec := recover() + if rec == nil { + return } + var err error + if e, ok := rec.(error); ok { + err = errx.Wrap(recovererOp, e) + } else { + err = errx.Newf(recovererOp, "panic: %v", rec) + } + log.Printf("%v\n%s", err, debug.Stack()) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) }() next.ServeHTTP(w, r) }) diff --git a/pkg/middleware/recoverer_test.go b/pkg/middleware/recoverer_test.go index a5a07a9..471bf5c 100644 --- a/pkg/middleware/recoverer_test.go +++ b/pkg/middleware/recoverer_test.go @@ -2,6 +2,7 @@ package middleware import ( "bytes" + "errors" "log" "net/http" "net/http/httptest" @@ -9,11 +10,17 @@ import ( "testing" ) -func TestRecovererCatchesPanic(t *testing.T) { +func captureLog(t *testing.T) *bytes.Buffer { + t.Helper() var buf bytes.Buffer orig := log.Default().Writer() log.Default().SetOutput(&buf) - defer log.Default().SetOutput(orig) + t.Cleanup(func() { log.Default().SetOutput(orig) }) + return &buf +} + +func TestRecovererCatchesStringPanic(t *testing.T) { + buf := captureLog(t) h := Recoverer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { panic("boom") @@ -25,8 +32,27 @@ func TestRecovererCatchesPanic(t *testing.T) { if rr.Code != http.StatusInternalServerError { t.Errorf("status = %d, want 500", rr.Code) } - if !strings.Contains(buf.String(), "panic: boom") { - t.Errorf("expected panic log, got %q", buf.String()) + out := buf.String() + for _, want := range []string{"middleware.Recoverer", "panic: boom"} { + if !strings.Contains(out, want) { + t.Errorf("log missing %q\nfull: %s", want, out) + } + } +} + +func TestRecovererWrapsErrorPanic(t *testing.T) { + buf := captureLog(t) + + cause := errors.New("db down") + h := Recoverer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + panic(cause) + })) + + h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/", nil)) + + out := buf.String() + if !strings.Contains(out, "middleware.Recoverer: db down") { + t.Errorf("expected errx-wrapped breadcrumb, got: %s", out) } } diff --git a/pkg/router/pattern.go b/pkg/router/pattern.go index 3d79c19..75592cb 100644 --- a/pkg/router/pattern.go +++ b/pkg/router/pattern.go @@ -1,6 +1,12 @@ package router -import "strings" +import ( + "strings" + + "git.juancwu.dev/juancwu/errx" +) + +const groupOp = "router.Group" func splitPattern(pattern string) (method, host, path string) { rest := pattern @@ -22,10 +28,10 @@ func validateGroupPrefix(p string) { return } if strings.ContainsAny(p, " \t") { - panic("lightmux: group prefix must not contain whitespace (no method or host allowed): " + p) + panic(errx.Newf(groupOp, "prefix must not contain whitespace (no method or host allowed): %q", p)) } if !strings.HasPrefix(p, "/") { - panic("lightmux: group prefix must start with '/': " + p) + panic(errx.Newf(groupOp, "prefix must start with '/': %q", p)) } } @@ -53,7 +59,7 @@ func joinPath(prefix, sub string) string { return prefix + "/" } if !strings.HasPrefix(sub, "/") { - panic("lightmux: route path must start with '/': " + sub) + panic(errx.Newf(groupOp, "route path must start with '/': %q", sub)) } return prefix + sub }