g

komuw · Aug 16, 2024 · 0460c9c · 0460c9c
1 parent 2cf73be
commit 0460c9c
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 28 deletions.
diff --git a/middleware/log_test.go b/middleware/log_test.go
@@ -5,7 +5,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"log/slog"
 	mathRand "math/rand/v2"
 	"net/http"
 	"net/http/httptest"
@@ -24,7 +23,7 @@ const (
 	someLatencyMS        = 3
 )
 
-func toLog(t *testing.T, l *slog.Logger) func(w http.ResponseWriter, r http.Request, statusCode int, fields []any) {
+func toLog(t *testing.T, buf *bytes.Buffer) func(w http.ResponseWriter, r http.Request, statusCode int, fields []any) {
 	t.Helper()
 
 	const (
@@ -33,6 +32,8 @@ func toLog(t *testing.T, l *slog.Logger) func(w http.ResponseWriter, r http.Requ
 		rateShedSamplePercent = 10
 	)
 
+	l := log.New(context.Background(), buf, 500)
+
 	return func(w http.ResponseWriter, r http.Request, statusCode int, fields []any) {
 		// Each request should get its own context. That's why we call `log.WithID` for every request.
 		reqL := log.WithID(r.Context(), l)
@@ -84,16 +85,12 @@ func someLogHandler(successMsg string) http.HandlerFunc {
 func TestLogMiddleware(t *testing.T) {
 	t.Parallel()
 
-	getLogger := func(w io.Writer) *slog.Logger {
-		return log.New(context.Background(), w, 500)
-	}
-
 	t.Run("success", func(t *testing.T) {
 		t.Parallel()
 
 		logOutput := &bytes.Buffer{}
 		successMsg := "hello"
-		wrappedHandler := logger(someLogHandler(successMsg), getLogger(logOutput))
+		wrappedHandler := logger(someLogHandler(successMsg), toLog(t, logOutput))
 
 		rec := httptest.NewRecorder()
 		req := httptest.NewRequest(http.MethodHead, "/someUri", nil)
@@ -116,7 +113,7 @@ func TestLogMiddleware(t *testing.T) {
 		logOutput := &bytes.Buffer{}
 		errorMsg := "someLogHandler failed"
 		successMsg := "hello"
-		wrappedHandler := logger(someLogHandler(successMsg), getLogger(logOutput))
+		wrappedHandler := logger(someLogHandler(successMsg), toLog(t, logOutput))
 
 		rec := httptest.NewRecorder()
 		req := httptest.NewRequest(http.MethodHead, "/someUri", nil)
@@ -152,7 +149,7 @@ func TestLogMiddleware(t *testing.T) {
 		logOutput := &bytes.Buffer{}
 		successMsg := "hello"
 		errorMsg := "someLogHandler failed"
-		wrappedHandler := logger(someLogHandler(successMsg), getLogger(logOutput))
+		wrappedHandler := logger(someLogHandler(successMsg), toLog(t, logOutput))
 
 		{
 			// first request that succeds
@@ -231,7 +228,7 @@ func TestLogMiddleware(t *testing.T) {
 
 		logOutput := &bytes.Buffer{}
 		successMsg := "hello"
-		wrappedHandler := logger(someLogHandler(successMsg), getLogger(logOutput))
+		wrappedHandler := logger(someLogHandler(successMsg), toLog(t, logOutput))
 
 		someLogID := "hey-some-log-id:" + id.New()
 
@@ -261,7 +258,7 @@ func TestLogMiddleware(t *testing.T) {
 		successMsg := "hello"
 		// for this concurrency test, we have to re-use the same wrappedHandler
 		// so that state is shared and thus we can see if there is any state which is not handled correctly.
-		wrappedHandler := logger(someLogHandler(successMsg), getLogger(logOutput))
+		wrappedHandler := logger(someLogHandler(successMsg), toLog(t, logOutput))
 
 		runhandler := func() {
 			rec := httptest.NewRecorder()

diff --git a/middleware/middleware.go b/middleware/middleware.go
@@ -41,6 +41,11 @@ const (
 	allowHeader = "Allow"
 )
 
+// TODO: remove
+func todoRemove(w http.ResponseWriter, r http.Request, statusCode int, fields []any) {
+	return
+}
+
 // allDefaultMiddlewares is a middleware that bundles all the default/core middlewares into one.
 //
 // example usage:
@@ -168,7 +173,9 @@ func allDefaultMiddlewares(
 								rateLimit,
 							),
 						),
+						todoRemove,
 					),
+					todoRemove,
 				),
 			),
 			strategy,

diff --git a/middleware/recoverer_test.go b/middleware/recoverer_test.go
@@ -2,19 +2,14 @@ package middleware
 
 import (
 	"bytes"
-	"context"
 	"fmt"
-	"io"
-	"log/slog"
 	"net/http"
 	"net/http/httptest"
-	"os"
 	"strings"
 	"sync"
 	"testing"
 
 	"github.com/komuw/ong/errors"
-	"github.com/komuw/ong/log"
 
 	"go.akshayshah.org/attest"
 )
@@ -48,16 +43,12 @@ func anotherHandlerThatPanics() http.HandlerFunc {
 func TestPanic(t *testing.T) {
 	t.Parallel()
 
-	getLogger := func(w io.Writer) *slog.Logger {
-		return log.New(context.Background(), w, 500)
-	}
-
 	t.Run("ok if no panic", func(t *testing.T) {
 		t.Parallel()
 
 		logOutput := &bytes.Buffer{}
 		msg := "hello"
-		wrappedHandler := recoverer(handlerThatPanics(msg, false, nil), getLogger(logOutput))
+		wrappedHandler := recoverer(handlerThatPanics(msg, false, nil), toLog(t, logOutput))
 
 		rec := httptest.NewRecorder()
 		req := httptest.NewRequest(http.MethodGet, "/someUri", nil)
@@ -74,7 +65,7 @@ func TestPanic(t *testing.T) {
 
 		logOutput := &bytes.Buffer{}
 		msg := "hello"
-		wrappedHandler := recoverer(handlerThatPanics(msg, true, nil), getLogger(logOutput))
+		wrappedHandler := recoverer(handlerThatPanics(msg, true, nil), toLog(t, logOutput))
 
 		rec := httptest.NewRecorder()
 		req := httptest.NewRequest(http.MethodGet, "/someUri", nil)
@@ -104,7 +95,7 @@ func TestPanic(t *testing.T) {
 		msg := "hello"
 		errMsg := "99 problems"
 		err := errors.New(errMsg)
-		wrappedHandler := recoverer(handlerThatPanics(msg, false, err), getLogger(logOutput))
+		wrappedHandler := recoverer(handlerThatPanics(msg, false, err), toLog(t, logOutput))
 
 		rec := httptest.NewRecorder()
 		req := httptest.NewRequest(http.MethodGet, "/someUri", nil)
@@ -132,7 +123,7 @@ func TestPanic(t *testing.T) {
 		t.Parallel()
 
 		logOutput := &bytes.Buffer{}
-		wrappedHandler := recoverer(anotherHandlerThatPanics(), getLogger(logOutput))
+		wrappedHandler := recoverer(anotherHandlerThatPanics(), toLog(t, logOutput))
 
 		rec := httptest.NewRecorder()
 		req := httptest.NewRequest(http.MethodGet, "/someUri", nil)
@@ -141,19 +132,19 @@ func TestPanic(t *testing.T) {
 		res := rec.Result()
 		defer res.Body.Close()
 		attest.Equal(t, res.StatusCode, http.StatusInternalServerError)
-		attest.Subsequence(t, logOutput.String(), "middleware/recoverer_test.go:42") // line where panic happened.
+		attest.Subsequence(t, logOutput.String(), "middleware/recoverer_test.go:37") // line where panic happened.
 	})
 
 	t.Run("concurrency safe", func(t *testing.T) {
 		t.Parallel()
 
-		// &bytes.Buffer{} is not concurrency safe, so we use os.Stderr instead.
-		logOutput := os.Stderr
+		// If &bytes.Buffer{} is not concurrency safe, we can use os.Stderr instead.
+		logOutput := &bytes.Buffer{}
 		msg := "hey"
 		err := errors.New(msg)
 		// for this concurrency test, we have to re-use the same wrappedHandler
 		// so that state is shared and thus we can see if there is any state which is not handled correctly.
-		wrappedHandler := recoverer(handlerThatPanics(msg, false, err), getLogger(logOutput))
+		wrappedHandler := recoverer(handlerThatPanics(msg, false, err), toLog(t, logOutput))
 
 		runhandler := func() {
 			rec := httptest.NewRecorder()