From 22b5746ef96c233554aa905b1a6de6bc420b5d88 Mon Sep 17 00:00:00 2001 From: Andres Bott Date: Sun, 19 Apr 2026 16:20:11 +0200 Subject: [PATCH 1/3] refactor some stuff --- .claude/skills/verify/SKILL.md | 63 ++++++++++ .github/workflows/golangci-lint.yml | 23 ++++ .github/workflows/license-check.yml | 22 ++++ .github/workflows/test.yml | 22 ++++ CLAUDE.md | 34 ++++++ Makefile | 23 +++- README.md | 68 ++++++++--- lib/limitio/limitbuf.go | 23 +++- lib/limitio/limitbuf_test.go | 101 +++++++++++++++- middleware/delay.go | 2 +- middleware/errors.go | 87 ++++++++++++++ middleware/middleware.go | 140 ++++++++++++---------- middleware/panicrecover.go | 31 +++++ middleware/panicrecover_test.go | 174 ++++++++++++++++++++++++++++ middleware/prometheus.go | 31 ++++- middleware/prometheus_test.go | 5 +- middleware/respwriter.go | 52 ++++++--- middleware/respwriter_test.go | 160 +++++++++++++++++++++++++ middleware/slog.go | 68 ++++++----- server/server.go | 127 -------------------- 20 files changed, 993 insertions(+), 263 deletions(-) create mode 100644 .claude/skills/verify/SKILL.md create mode 100644 .github/workflows/golangci-lint.yml create mode 100644 .github/workflows/license-check.yml create mode 100644 .github/workflows/test.yml create mode 100644 CLAUDE.md create mode 100644 middleware/errors.go create mode 100644 middleware/panicrecover_test.go create mode 100644 middleware/respwriter_test.go delete mode 100644 server/server.go diff --git a/.claude/skills/verify/SKILL.md b/.claude/skills/verify/SKILL.md new file mode 100644 index 0000000..9b6de29 --- /dev/null +++ b/.claude/skills/verify/SKILL.md @@ -0,0 +1,63 @@ +--- +name: verify +description: Use when the user runs /verify or asks to run make verify. Runs the full verification suite (tests, lint, coverage, benchmarks, license) and fixes every issue found. +--- + +# verify + +Run `make verify` and fix all issues until it passes clean. + +`make verify` runs in order: `test` → `ui-test` → `license-check` → `lint` → `benchmark` → `coverage` + +## How to run + +```bash +make verify +``` + +Read ALL output carefully. Don't stop at the first failure — run through to the end to collect all issues, then fix them together. + +## Fixing issues — The Iron Law + +**Fix the code. Never silence the tool.** + +| Forbidden | Why | +|-----------|-----| +| Adding `//nolint:...` directives | Hides the problem, ships broken code | +| Removing or skipping tests | Destroys the safety net | +| Lowering the coverage threshold | Treats the symptom | +| Commenting out failing assertions | Same as deleting the test | +| `//nolint` without a real reason | `nolintlint` requires specific linter + explanation anyway | + +The only valid `//nolint` is when the linter is provably wrong for that exact line and you include a clear explanation. This should be rare. + +## Linter quick reference + +Config: `.golangci.yaml` — standard linters + `nolintlint`, `gocyclo` (≥20), `nestif` (≥5), `gosec`, `dupl` + +| Linter | Common fix | +|--------|-----------| +| `errcheck` | Handle or explicitly discard the error: `_ = f()` only if truly safe | +| `staticcheck` | Follow the message — usually dead code, deprecated API, or impossible condition | +| `unused` | Delete the unused symbol, don't keep it for "future use" | +| `govet` | Fix the suspicious construct (printf verbs, mutex copies, etc.) | +| `ineffassign` | Remove the assignment or actually use the value | +| `gocyclo` / `nestif` | Refactor: extract helper functions, invert conditions, reduce nesting | +| `gosec` | Fix the security issue (weak random, unhandled error on Close, etc.) | +| `dupl` | Extract the duplicated block into a shared function | +| `nolintlint` | Remove invalid nolint or add specific linter name + explanation | + +## Coverage + +Threshold: **70%** for `./internal/...` and `./libs/...` + +If coverage drops below 70%: write the missing tests. Do not lower the threshold. + +## Step-by-step + +1. Run `make verify`, capture full output +2. Group failures by type (test failures, lint issues, coverage gaps) +3. Fix all test failures first (they may affect coverage numbers) +4. Fix all lint issues by refactoring code +5. Add missing tests if coverage is below threshold +6. Run `make verify` again — repeat until it passes with zero errors diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 0000000..094d300 --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,23 @@ +name: golangci-lint +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: golangci-lint + uses: golangci/golangci-lint-action@v6 + with: + version: v2 diff --git a/.github/workflows/license-check.yml b/.github/workflows/license-check.yml new file mode 100644 index 0000000..732eb6e --- /dev/null +++ b/.github/workflows/license-check.yml @@ -0,0 +1,22 @@ +name: license-check +on: + push: + branches: [main] + pull_request: + +jobs: + license-check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Install go-licence-detector + run: go install go.elastic.co/go-licence-detector@v0.10.0 + + - name: License check + run: make license-check diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..fd0155e --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,22 @@ +name: test +on: + push: + branches: [main] + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Run tests + run: make test + + - name: Check coverage + run: make coverage diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..746d903 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,34 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Commands + +```bash +make test # run all tests with coverage +make lint # run golangci-lint (must be installed externally) +make benchmark # run benchmarks +make verify # run tests + lint + benchmarks + coverage check +go test ./... # run all tests +go test ./middleware/ # run tests for a single package +go test ./middleware/ -run TestMiddleware # run a single test +``` + +## Architecture + +This is a Go library (`github.com/go-bumbu/http`) providing reusable HTTP components for backend services. It is not an application — it's imported by other projects. + +### Packages + +- **middleware/** — Composable middleware chain using standard `func(next http.Handler) http.Handler` pattern. Includes: structured logging (slog), Prometheus metrics, JSON error wrapping, generic error messages, and development delay. +- **handlers/spa/** — Single Page Application handler serving files from an `fs.FS` (typically embedded). +- **lib/limitio/** — Internal IO utilities: bounded buffer (2000 byte cap) and limited writer. + +### Key Design Decisions + +- **StatWriter** (`middleware/respwriter.go`) wraps `http.ResponseWriter` to intercept status codes and error bodies. The `teeOnErr` flag simultaneously buffers the body for logging while forwarding it to the client — this prevents reverse-proxy hangs when the upstream writes an error body. +- **Error classification**: `IsStatusError()` (< 200 or >= 400) vs `IsServerErr()` (>= 500) drives log levels — server errors log at ERROR, client errors at INFO. + +## Linting + +Uses golangci-lint v2 with: nolintlint, gocyclo (max 20), nestif (max 5), gosec, dupl. All `//nolint` directives require an explanation and specific linter name. diff --git a/Makefile b/Makefile index 1f12d8f..05d8f6b 100644 --- a/Makefile +++ b/Makefile @@ -20,8 +20,29 @@ license-check: ## check for invalid licenses # depends on : https://github.com/elastic/go-licence-detector @go list -m -mod=readonly -json all | go-licence-detector -includeIndirect -validate -rules allowedLicenses.json + +COVERAGE_THRESHOLD ?= 70 +.PHONY: coverage +coverage: + @fail=0; \ + for pkg in $$(go list ./lib/... ./middleware/... ./handlers/...); do \ + go test -coverprofile=coverage.out -covermode=atomic $$pkg > /dev/null 2>&1; \ + if [ -f coverage.out ]; then \ + coverage=$$(go tool cover -func=coverage.out | grep total: | awk '{print $$3}' | sed 's/%//'); \ + if [ $$(echo "$$coverage < $(COVERAGE_THRESHOLD)" | bc -l) -eq 1 ]; then \ + echo "❌ Coverage in $$pkg is below $(COVERAGE_THRESHOLD)%: $${coverage}%"; \ + fail=1; \ + fi; \ + rm -f coverage.out; \ + else \ + echo "⚠️ No coverage data for $$pkg"; \ + fail=1; \ + fi; \ + done; \ + exit $$fail + .PHONY: verify -verify: test license-check lint benchmark ## run all tests +verify: test license-check lint benchmark coverage ## run all tests cover-report: ## generate a coverage report go test -covermode=count -coverpkg=./... -coverprofile cover.out ./... diff --git a/README.md b/README.md index 0a5abdb..334bcbf 100644 --- a/README.md +++ b/README.md @@ -1,33 +1,63 @@ # Http -The Http module contains a set uf useful http packages that can be used in any Http backend project. +Reusable HTTP packages for Go backend services. -## Import +## Install -Import the module ``` go get github.com/go-bumbu/http ``` -## Server +## Packages -The server package contains boilerplate to create an http server. -Features: -* you can specify a main handler and an optional observability handler - * the observability handler is intended to expose details like metrics, runtime controls or hprof endpoint -* the server can safely shut down with os kill signals -* it exposes a Stop() method to safely shut down both servers. +### middleware +Composable HTTP middleware using the standard `func(next http.Handler) http.Handler` pattern. +Middleware can be used individually or combined via the `Middleware` struct which orchestrates all features in a single handler. +**Standalone middleware:** -## Middleware +| Middleware | Import | Description | +|---|---|---| +| `Logging` | `middleware.Logging(logger)` | Structured request logging via `log/slog`. Logs at INFO for client errors, ERROR for server errors. Captures error response bodies. | +| `Metrics` | `middleware.Metrics(hist)` | Prometheus histogram recording request duration, method, path, status code, and error flag. | +| `JSONErrors` | `middleware.JSONErrors(generic)` | Intercepts error responses (>= 400) and wraps the body in `{"error":"...","code":N}`. Optionally replaces messages with generic status text. | +| `GenericErrors` | `middleware.GenericErrors()` | Replaces error response bodies with the standard status text (e.g. "Internal Server Error"). | +| `PanicRecover` | `middleware.PanicRecover(logger)` | Recovers from panics, logs a stack trace, and returns 500 to the client. | +| `ReqDelay` | `middleware.ReqDelay{...}.Delay` | Adds a random delay between min/max duration. Useful during development to simulate slow backends. | -Middleware contains several middleware handlers to facilitate writing backends -* delay: useful during development for AJAX calls, adds delay to a response -* jsonErr: wraps all http errors into a json response, also allows to generalize errors - * setting the flag _genericMessage_ to true, will not return the error string but the generic error string matching the response code instead -* prometheus: adds an _http_duration_seconds_ bucket to measure volume and duration of requests per response code -* zerolog: middleware that will write a log message to a zerolog logger capturing every request +**Combined middleware:** -## Handlers -* spa: simple Single page application handler to serve SPAs embedded into go code \ No newline at end of file +```go +m := middleware.New(middleware.Cfg{ + JsonErrors: true, + GenericErrs: true, + PanicRecover: true, + Logger: slog.Default(), + PromHisto: hist, +}) +mux.Handle("/", m.Middleware(handler)) +``` + +The combined `Middleware` struct runs logging, metrics, error wrapping, and panic recovery in a single pass. + +### handlers/spa + +Single Page Application handler that serves files from an `fs.FS` (typically `embed.FS`). +Requests for unknown paths fall back to `index.html`, allowing client-side routing. + +```go +spaHandler, err := handlers.NewSpaHAndler(embeddedFS, "dist", "/ui") +``` + +Parameters: +- `inputFs` — the filesystem containing the SPA assets +- `fsSubDir` — subdirectory within the FS to serve from (empty string for root) +- `pathPrefix` — URL path prefix where the SPA is mounted + +### lib/limitio + +Internal IO utilities for bounded writes. + +- **`LimitedBuf`** — A `bytes.Buffer` that stops accepting data after a configured byte limit (default 2000 in the middleware). Returns `ErrBufferLimit` when the cap is reached. Used to safely buffer error response bodies for logging without unbounded memory growth. +- **`LimitWriter`** — Wraps any `io.Writer` and caps total bytes written, returning `io.EOF` at the limit. diff --git a/lib/limitio/limitbuf.go b/lib/limitio/limitbuf.go index 0586282..b9f141d 100644 --- a/lib/limitio/limitbuf.go +++ b/lib/limitio/limitbuf.go @@ -9,22 +9,39 @@ import ( // it implements the io.ReadWriter interface type LimitedBuf struct { bytes.Buffer - MaxBytes int - curByte int + MaxBytes int + curByte int + truncated bool } func (b *LimitedBuf) Reset() { b.Buffer.Reset() b.curByte = 0 + b.truncated = false } func (b *LimitedBuf) Write(p []byte) (n int, err error) { - if len(p)+b.curByte > b.MaxBytes { + remaining := b.MaxBytes - b.curByte + if remaining <= 0 { return 0, ErrBufferLimit } + if len(p) > remaining { + p = p[:remaining] + n, err = b.Buffer.Write(p) + b.curByte += n + b.truncated = true + if err != nil { + return n, err + } + return n, ErrBufferLimit + } n, err = b.Buffer.Write(p) b.curByte += n return n, err } +func (b *LimitedBuf) Truncated() bool { + return b.truncated +} + var ErrBufferLimit = fmt.Errorf("buffer write limit reached") diff --git a/lib/limitio/limitbuf_test.go b/lib/limitio/limitbuf_test.go index 71d9142..5893a6c 100644 --- a/lib/limitio/limitbuf_test.go +++ b/lib/limitio/limitbuf_test.go @@ -1,8 +1,11 @@ package limitio_test import ( - "github.com/go-bumbu/http/lib/limitio" + "bytes" + "io" "testing" + + "github.com/go-bumbu/http/lib/limitio" ) func TestBuffer_Write(t *testing.T) { @@ -28,8 +31,8 @@ func TestBuffer_Write(t *testing.T) { buffer: &limitio.LimitedBuf{MaxBytes: 5}, data: []byte("Hello, World!"), expectedErr: limitio.ErrBufferLimit, - expectedN: 0, - expectedBuf: "", + expectedN: 5, + expectedBuf: "Hello", }, { name: "Write exactly at limit", @@ -72,3 +75,95 @@ func TestBuffer_Write(t *testing.T) { }) } } + +func TestBuffer_Truncated(t *testing.T) { + buf := &limitio.LimitedBuf{MaxBytes: 5} + if buf.Truncated() { + t.Error("expected Truncated() to be false initially") + } + _, _ = buf.Write([]byte("Hello, World!")) + if !buf.Truncated() { + t.Error("expected Truncated() to be true after exceeding limit") + } + buf.Reset() + if buf.Truncated() { + t.Error("expected Truncated() to be false after Reset()") + } +} + +func TestLimitWriter_Write(t *testing.T) { + tests := []struct { + name string + limit int64 + data []byte + expectedN int + expectedErr error + expectedBuf string + }{ + { + name: "write within limit", + limit: 10, + data: []byte("Hello"), + expectedN: 5, + expectedErr: nil, + expectedBuf: "Hello", + }, + { + name: "write exceeds limit", + limit: 3, + data: []byte("Hello"), + expectedN: 3, + expectedErr: nil, + expectedBuf: "Hel", + }, + { + name: "write at zero limit", + limit: 0, + data: []byte("Hello"), + expectedN: 0, + expectedErr: io.EOF, + expectedBuf: "", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var buf bytes.Buffer + lw := &limitio.LimitWriter{R: &buf, N: tc.limit} + n, err := lw.Write(tc.data) + if err != tc.expectedErr { + t.Errorf("expected error %v, got %v", tc.expectedErr, err) + } + if n != tc.expectedN { + t.Errorf("expected n = %d, got n = %d", tc.expectedN, n) + } + if buf.String() != tc.expectedBuf { + t.Errorf("expected buffer content %q, got %q", tc.expectedBuf, buf.String()) + } + }) + } +} + +func TestLimitWriter_MultipleWrites(t *testing.T) { + var buf bytes.Buffer + lw := &limitio.LimitWriter{R: &buf, N: 8} + + n, err := lw.Write([]byte("Hello")) + if err != nil || n != 5 { + t.Fatalf("first write: n=%d, err=%v", n, err) + } + + n, err = lw.Write([]byte("World")) + if err != nil || n != 3 { + t.Fatalf("second write: expected n=3, got n=%d, err=%v", n, err) + } + + n, err = lw.Write([]byte("!")) + if err != io.EOF || n != 0 { + t.Fatalf("third write: expected EOF, got n=%d, err=%v", n, err) + } + + if buf.String() != "HelloWor" { + t.Errorf("expected %q, got %q", "HelloWor", buf.String()) + } +} diff --git a/middleware/delay.go b/middleware/delay.go index a476be7..9486519 100644 --- a/middleware/delay.go +++ b/middleware/delay.go @@ -18,7 +18,7 @@ func (t ReqDelay) Delay(next http.Handler) http.Handler { if t.On && t.MinDelay.Milliseconds() != 0 && t.MaxDelay.Milliseconds() != 0 { size := t.MaxDelay - t.MinDelay randDur := rand.IntN(int(size)) //nolint:gosec // non-crypto randomness is sufficient for delay jitter - time.Sleep(time.Duration(randDur)) + time.Sleep(t.MinDelay + time.Duration(randDur)) } next.ServeHTTP(w, r) }) diff --git a/middleware/errors.go b/middleware/errors.go new file mode 100644 index 0000000..c236ac9 --- /dev/null +++ b/middleware/errors.go @@ -0,0 +1,87 @@ +package middleware + +import ( + "encoding/json" + "fmt" + "net/http" +) + +// JSONErrors returns a standalone middleware that intercepts error responses (>= 400) +// and wraps the body in a JSON envelope: {"error": "...", "code": N}. +func JSONErrors(genericErrs bool) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + respWriter := NewWriter(w, true, false) + + next.ServeHTTP(respWriter, r) + + if IsStatusError(respWriter.statusCode) && !respWriter.BodyForwarded() { + errMsg := readErrMsg(respWriter) + if genericErrs { + errMsg = http.StatusText(respWriter.StatusCode()) + } + b := jsonErrBytes(errMsg, respWriter.StatusCode()) + w.Header().Set("Content-Type", "application/json") + respWriter.flushHeader() + _, _ = w.Write(b) + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() + } + } else { + respWriter.flushHeader() + } + }) + } +} + +// GenericErrors returns a standalone middleware that intercepts error responses (>= 400) +// and replaces the body with a generic status text (e.g., "Internal Server Error"). +func GenericErrors() func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + respWriter := NewWriter(w, true, false) + + next.ServeHTTP(respWriter, r) + + if IsStatusError(respWriter.statusCode) && !respWriter.BodyForwarded() { + errMsg := http.StatusText(respWriter.StatusCode()) + w.Header().Set("Content-Type", "text/plain") + respWriter.flushHeader() + _, _ = fmt.Fprint(w, errMsg) + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() + } + } else { + respWriter.flushHeader() + } + }) + } +} + +func readErrMsg(respWriter *StatWriter) string { + msg := respWriter.buf.String() + if respWriter.buf.Truncated() { + msg += " [truncated]" + } + return msg +} + +type jsonErr struct { + Error string `json:"error"` + Code int `json:"code"` +} + +func jsonErrBytes(errMsg string, code int) []byte { + if code == 0 { + code = http.StatusInternalServerError + } + payload := jsonErr{ + Error: errMsg, + Code: code, + } + byteErr, err := json.Marshal(payload) + if err != nil { + return []byte(fmt.Sprintf(`{"error":"internal error","code":%d}`, code)) + } + return byteErr +} diff --git a/middleware/middleware.go b/middleware/middleware.go index 5c04e7e..3a63e55 100644 --- a/middleware/middleware.go +++ b/middleware/middleware.go @@ -1,48 +1,53 @@ package middleware import ( - "encoding/json" "fmt" "io" "log/slog" "net/http" + "runtime/debug" "strings" "time" + + "github.com/go-bumbu/http/lib/limitio" ) type Cfg struct { - JsonErrors bool - GenericErrs bool // print generic error messages instead of the actual one - Logger *slog.Logger - PromHisto Histogram + JsonErrors bool + GenericErrs bool // print generic error messages instead of the actual one + PanicRecover bool + Logger *slog.Logger + PromHisto Histogram } func New(cfg Cfg) *Middleware { m := Middleware{ - jsonErrors: cfg.JsonErrors, - genericErrs: cfg.GenericErrs, - hist: cfg.PromHisto, - logger: cfg.Logger, + jsonErrors: cfg.JsonErrors, + genericErrs: cfg.GenericErrs, + panicRecover: cfg.PanicRecover, + hist: cfg.PromHisto, + logger: cfg.Logger, } return &m } // Middleware is intended perform common actions done by a production http server, it has several configuration flags: -// - JsonErrors: if set to true it will intercept all responses != 200, read the response error handlerMsg and -// wrap it into a json file, this is useful for APIs +// - JsonErrors: if set to true it will intercept all error responses (status < 200 or >= 400), read the response +// error handlerMsg and wrap it into a json file, this is useful for APIs // - GenericErrs: if set to true the error handlerMsg responded to the en user is a generic handlerMsg based on the // response code instead of the original error handlerMsg, the original error will still be logged. // -// NOTE: both JsonErrors and GenericErrs assumes that only 200 response codes contain usable body, e.g. don't -// return html in case of errors +// NOTE: both JsonErrors and GenericErrs only intercept error responses (< 200 or >= 400). Success codes like +// 200, 204, 206 etc. pass through unmodified. // // - Histogram: use NewPromHistogram to create an histogram used to capture prometheus metrics about every request // if left empty, no prometheus metric will be captured type Middleware struct { - jsonErrors bool - genericErrs bool - hist Histogram - logger *slog.Logger + jsonErrors bool + genericErrs bool + panicRecover bool + hist Histogram + logger *slog.Logger } // Middleware is an HTTP middleware that checks the Config and applies logic based on it. @@ -54,64 +59,77 @@ func (c *Middleware) Middleware(next http.Handler) http.Handler { teeOnErr := !c.genericErrs && !c.jsonErrors respWriter := NewWriter(w, true, teeOnErr) - next.ServeHTTP(respWriter, r) - timeDiff := time.Since(timeStart) + if c.panicRecover { + defer func() { + if rec := recover(); rec != nil { + stack := debug.Stack() + if c.logger != nil { + c.logger.Error("panic recovered", + slog.String("method", r.Method), + slog.String("url", r.RequestURI), + slog.String("panic", fmt.Sprint(rec)), + slog.String("stack", string(stack)), + ) + } + respWriter.WriteHeader(http.StatusInternalServerError) + _, _ = respWriter.Write([]byte(http.StatusText(http.StatusInternalServerError))) + } + c.finalize(w, r, respWriter, timeStart) + }() + } - // get the generic or the specific error string - errMsg := c.getErrMsg(respWriter.statusCode, respWriter.buf) - c.log(r, respWriter.StatusCode(), errMsg, timeDiff) + next.ServeHTTP(respWriter, r) - if c.genericErrs { - errMsg = http.StatusText(respWriter.StatusCode()) + if !c.panicRecover { + c.finalize(w, r, respWriter, timeStart) } + }) +} + +func (c *Middleware) finalize(w http.ResponseWriter, r *http.Request, respWriter *StatWriter, timeStart time.Time) { + timeDiff := time.Since(timeStart) + + errMsg := c.getErrMsg(respWriter.statusCode, respWriter.buf) + c.log(r, respWriter.StatusCode(), errMsg, timeDiff) + + if c.genericErrs { + errMsg = http.StatusText(respWriter.StatusCode()) + } - if respWriter.statusCode != 200 && !respWriter.BodyForwarded() { - // StatWriter did not tee (e.g. direct http.Error from handler): body was buffered only, - // so we must write it here or the client hangs waiting for Content-Length bytes. - if c.jsonErrors { - b := jsonErrBytes(errMsg, respWriter.StatusCode()) - w.Header().Set("Content-Type", "application/json") - _, _ = fmt.Fprint(w, string(b)) - } else { - w.Header().Set("Content-Type", "text/plain") - _, _ = fmt.Fprint(w, errMsg) - } - if flusher, ok := w.(http.Flusher); ok { - flusher.Flush() - } + if IsStatusError(respWriter.statusCode) && !respWriter.BodyForwarded() { + if c.jsonErrors { + b := jsonErrBytes(errMsg, respWriter.StatusCode()) + w.Header().Set("Content-Type", "application/json") + respWriter.flushHeader() + _, _ = w.Write(b) + } else { + w.Header().Set("Content-Type", "text/plain") + respWriter.flushHeader() + _, _ = fmt.Fprint(w, errMsg) + } + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() } + } else { + respWriter.flushHeader() + } - c.observe(r, respWriter.StatusCode(), timeDiff) - }) + c.observe(r, respWriter.StatusCode(), timeDiff) } -// getErrMsg returns the error handlerMsg in case of a request != 200 or empty string -func (c *Middleware) getErrMsg(code int, buf io.Reader) string { - if code == 200 { +// getErrMsg returns the error handlerMsg in case of an error response or empty string +func (c *Middleware) getErrMsg(code int, buf *limitio.LimitedBuf) string { + if !IsStatusError(code) { return "" } msgB, err := io.ReadAll(buf) if err != nil && c.logger != nil { - // I wish I could estimate the conditions of this error c.logger.Error("error while reading buffer error handlerMsg:", slog.Any("err", err)) } - return strings.Trim(string(msgB), "\n") -} - -type jsonErr struct { - Error string `json:"error"` - Code int `json:"code"` -} - -func jsonErrBytes(error string, code int) []byte { - if code == 0 { - code = http.StatusInternalServerError - } - payload := jsonErr{ - Error: error, - Code: code, + msg := strings.Trim(string(msgB), "\n") + if buf.Truncated() { + msg += " [truncated]" } - byteErr, _ := json.Marshal(payload) - return byteErr + return msg } diff --git a/middleware/panicrecover.go b/middleware/panicrecover.go index c870d7c..108138f 100644 --- a/middleware/panicrecover.go +++ b/middleware/panicrecover.go @@ -1 +1,32 @@ package middleware + +import ( + "fmt" + "log/slog" + "net/http" + "runtime/debug" +) + +// PanicRecover returns a middleware that recovers from panics in downstream handlers, +// logs the panic with a stack trace, and returns a 500 response to the client. +func PanicRecover(logger *slog.Logger) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + defer func() { + if rec := recover(); rec != nil { + stack := debug.Stack() + if logger != nil { + logger.Error("panic recovered", + slog.String("method", r.Method), + slog.String("url", r.RequestURI), + slog.String("panic", fmt.Sprint(rec)), + slog.String("stack", string(stack)), + ) + } + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + } + }() + next.ServeHTTP(w, r) + }) + } +} diff --git a/middleware/panicrecover_test.go b/middleware/panicrecover_test.go new file mode 100644 index 0000000..993c63c --- /dev/null +++ b/middleware/panicrecover_test.go @@ -0,0 +1,174 @@ +package middleware_test + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-bumbu/http/middleware" +) + +func panicHandler() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + panic("something went terribly wrong") + }) +} + +func TestPanicRecover_Returns500(t *testing.T) { + _, logger := newMemSlog() + handler := middleware.PanicRecover(logger)(panicHandler()) + + srv := httptest.NewServer(handler) + defer srv.Close() + + resp, err := http.Get(srv.URL + "/test") + if err != nil { + t.Fatalf("expected a valid response, got error: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusInternalServerError { + t.Errorf("expected status 500, got %d", resp.StatusCode) + } + body, _ := io.ReadAll(resp.Body) + if !strings.Contains(string(body), "Internal Server Error") { + t.Errorf("expected generic error body, got %q", string(body)) + } +} + +func TestPanicRecover_LogsPanic(t *testing.T) { + buf, logger := newMemSlog() + handler := middleware.PanicRecover(logger)(panicHandler()) + + rec := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/boom", nil) + handler.ServeHTTP(rec, req) + + logOutput := buf.String() + if !strings.Contains(logOutput, "something went terribly wrong") { + t.Errorf("expected panic message in log, got %q", logOutput) + } +} + +func TestPanicRecover_ServerSurvives(t *testing.T) { + _, logger := newMemSlog() + mux := http.NewServeMux() + mux.Handle("/panic", panicHandler()) + mux.HandleFunc("/ok", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("alive")) + }) + + handler := middleware.PanicRecover(logger)(mux) + srv := httptest.NewServer(handler) + defer srv.Close() + + // First request panics + resp, err := http.Get(srv.URL + "/panic") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + _ = resp.Body.Close() + + // Second request should work fine + resp, err = http.Get(srv.URL + "/ok") + if err != nil { + t.Fatalf("server died after panic: %v", err) + } + defer func() { _ = resp.Body.Close() }() + body, _ := io.ReadAll(resp.Body) + if string(body) != "alive" { + t.Errorf("expected 'alive', got %q", string(body)) + } +} + +func TestPanicRecover_NilLogger(t *testing.T) { + handler := middleware.PanicRecover(nil)(panicHandler()) + + rec := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/boom", nil) + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusInternalServerError { + t.Errorf("expected 500, got %d", rec.Code) + } +} + +func TestPanicRecover_NoPanic(t *testing.T) { + _, logger := newMemSlog() + inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("ok")) + }) + handler := middleware.PanicRecover(logger)(inner) + + rec := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/fine", nil) + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Errorf("expected 200, got %d", rec.Code) + } + if rec.Body.String() != "ok" { + t.Errorf("expected 'ok', got %q", rec.Body.String()) + } +} + +func TestPanicRecover_BundledMiddlewareWithJSON(t *testing.T) { + buf, logger := newMemSlog() + m := middleware.New(middleware.Cfg{ + JsonErrors: true, + PanicRecover: true, + Logger: logger, + }) + + handler := m.Middleware(panicHandler()) + + rec := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/boom", nil) + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusInternalServerError { + t.Errorf("expected 500, got %d", rec.Code) + } + body := rec.Body.String() + if !strings.Contains(body, `"error"`) || !strings.Contains(body, `"code":500`) { + t.Errorf("expected JSON error envelope, got %q", body) + } + ct := rec.Header().Get("Content-Type") + if !strings.Contains(ct, "application/json") { + t.Errorf("expected application/json content-type, got %q", ct) + } + logOutput := buf.String() + if !strings.Contains(logOutput, "something went terribly wrong") { + t.Errorf("expected panic in log, got %q", logOutput) + } +} + +func TestPanicRecover_ComposesWithJSONErrors(t *testing.T) { + _, logger := newMemSlog() + // JSONErrors wraps PanicRecover so the 500 from recovery gets intercepted as JSON. + handler := middleware.JSONErrors(false)( + middleware.PanicRecover(logger)(panicHandler()), + ) + + rec := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/boom", nil) + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusInternalServerError { + t.Errorf("expected 500, got %d", rec.Code) + } + body := rec.Body.String() + if !strings.Contains(body, `"error"`) || !strings.Contains(body, `"code":500`) { + t.Errorf("expected JSON error envelope, got %q", body) + } + + ct := rec.Header().Get("Content-Type") + if !strings.Contains(ct, "application/json") { + t.Errorf("expected application/json content-type, got %q", ct) + } +} + diff --git a/middleware/prometheus.go b/middleware/prometheus.go index cfc54b5..9c29b27 100644 --- a/middleware/prometheus.go +++ b/middleware/prometheus.go @@ -1,12 +1,33 @@ package middleware import ( - "github.com/prometheus/client_golang/prometheus" + "fmt" "net/http" "strconv" "time" + + "github.com/prometheus/client_golang/prometheus" ) +// Metrics returns a standalone middleware that records Prometheus request duration metrics. +func Metrics(hist Histogram) func(http.Handler) http.Handler { + if hist.h == nil { + return func(next http.Handler) http.Handler { return next } + } + m := &Middleware{hist: hist} + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + timeStart := time.Now() + respWriter := NewWriter(w, false, false) + + next.ServeHTTP(respWriter, r) + timeDiff := time.Since(timeStart) + + m.observe(r, respWriter.StatusCode(), timeDiff) + }) + } +} + func (c *Middleware) observe(r *http.Request, statusCode int, dur time.Duration) { if c.hist.h != nil { isErrorStr := strconv.FormatBool(IsStatusError(statusCode)) @@ -27,7 +48,7 @@ type Histogram struct { h *prometheus.HistogramVec } -func NewPromHistogram(prefix string, buckets []float64, registry prometheus.Registerer) Histogram { +func NewPromHistogram(prefix string, buckets []float64, registry prometheus.Registerer) (Histogram, error) { if registry == nil { registry = prometheus.DefaultRegisterer } @@ -55,9 +76,11 @@ func NewPromHistogram(prefix string, buckets []float64, registry prometheus.Regi "isError", }, ) - registry.MustRegister(histogram) + if err := registry.Register(histogram); err != nil { + return Histogram{}, fmt.Errorf("registering prometheus histogram: %w", err) + } return Histogram{ h: histogram, - } + }, nil } diff --git a/middleware/prometheus_test.go b/middleware/prometheus_test.go index 0a08c44..ac905c4 100644 --- a/middleware/prometheus_test.go +++ b/middleware/prometheus_test.go @@ -57,7 +57,10 @@ func TestPromMiddleware(t *testing.T) { t.Run(tc.name, func(t *testing.T) { reg := prometheus.NewRegistry() - hist := middleware.NewPromHistogram(tc.metricPrefix, nil, reg) + hist, err := middleware.NewPromHistogram(tc.metricPrefix, nil, reg) + if err != nil { + t.Fatalf("failed to create histogram: %v", err) + } m := middleware.New(middleware.Cfg{ JsonErrors: false, diff --git a/middleware/respwriter.go b/middleware/respwriter.go index 0c44452..1d49f72 100644 --- a/middleware/respwriter.go +++ b/middleware/respwriter.go @@ -20,11 +20,11 @@ type StatWriter struct { bodyForwarded bool // true when body was written to client (via tee) } -// NewWriter returns a StatWriter. When interceptBody is true and status != 200, -// the body is buffered. If teeOnErr is also true, the body is also forwarded to -// the client immediately (avoids hang when e.g. a reverse proxy copies the response). -// When teeOnErr is false, only the buffer is written; the middleware must write -// the body (e.g. when it will replace it with jsonErrors or genericErrs). +// NewWriter returns a StatWriter. When interceptBody is true and status is an error +// (< 200 or >= 400), the body is buffered. If teeOnErr is also true, the body is also +// forwarded to the client immediately (avoids hang when e.g. a reverse proxy copies the +// response). When teeOnErr is false, only the buffer is written; the middleware must +// write the body (e.g. when it will replace it with jsonErrors or genericErrs). func NewWriter(w http.ResponseWriter, interceptBody bool, teeOnErr bool) *StatWriter { return &StatWriter{ ResponseWriter: w, @@ -47,14 +47,13 @@ func (r *StatWriter) StatusCodeStr() string { } // Write returns underlying Write result. -// For non-200 when interceptBody is true: always buffers for logging. +// For error responses when interceptBody is true: always buffers for logging. // When teeOnErr is true, also forwards to client (so proxy copy completes; avoids hang). // When teeOnErr is false, buffers only (middleware will write, possibly modified). func (r *StatWriter) Write(b []byte) (int, error) { - if r.interceptBody && r.statusCode != 200 { - if n, err := r.buf.Write(b); err != nil { - return n, err - } + if r.interceptBody && IsStatusError(r.statusCode) { + // Buffer for logging; ignore ErrBufferLimit since partial content is acceptable for logging + _, _ = r.buf.Write(b) if r.teeOnErr { n, err := r.ResponseWriter.Write(b) if n > 0 { @@ -73,18 +72,41 @@ func (r *StatWriter) BodyForwarded() bool { return r.bodyForwarded } -// WriteHeader writes the response status code and stores it internally +// WriteHeader stores the response status code. When body interception is active and the +// body will be replaced (teeOnErr is false), the actual header write is deferred so the +// middleware can set correct Content-Type/Content-Length before flushing. func (r *StatWriter) WriteHeader(code int) { + if r.headerWritten { + return + } + r.statusCode = code + if r.interceptBody && !r.teeOnErr && IsStatusError(code) { + // Defer: middleware will write headers after determining the final body. + return + } + r.ResponseWriter.WriteHeader(code) + r.headerWritten = true +} + +// flushHeader ensures the status code is written to the underlying ResponseWriter. +// Called by the middleware after it has set final headers. +func (r *StatWriter) flushHeader() { if !r.headerWritten { - r.ResponseWriter.WriteHeader(code) - r.statusCode = code + r.ResponseWriter.WriteHeader(r.statusCode) r.headerWritten = true } } +// Unwrap returns the underlying ResponseWriter, allowing http.ResponseController +// to access optional interfaces (Flusher, Hijacker) on the original writer. +func (r *StatWriter) Unwrap() http.ResponseWriter { + return r.ResponseWriter +} + func IsStatusError(statusCode int) bool { - return statusCode < 200 || statusCode >= 400 + return statusCode >= 400 } + func IsServerErr(statusCode int) bool { - return statusCode < 200 || statusCode >= 500 + return statusCode >= 500 } diff --git a/middleware/respwriter_test.go b/middleware/respwriter_test.go new file mode 100644 index 0000000..883c2fa --- /dev/null +++ b/middleware/respwriter_test.go @@ -0,0 +1,160 @@ +package middleware + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestStatWriter_WriteHeader_OnlyOnce(t *testing.T) { + rec := httptest.NewRecorder() + sw := NewWriter(rec, false, false) + + sw.WriteHeader(http.StatusNotFound) + sw.WriteHeader(http.StatusOK) + + if sw.StatusCode() != http.StatusNotFound { + t.Errorf("expected status %d, got %d", http.StatusNotFound, sw.StatusCode()) + } + if rec.Code != http.StatusNotFound { + t.Errorf("expected recorder status %d, got %d", http.StatusNotFound, rec.Code) + } +} + +func TestStatWriter_DefaultStatus(t *testing.T) { + rec := httptest.NewRecorder() + sw := NewWriter(rec, false, false) + + if sw.StatusCode() != http.StatusOK { + t.Errorf("expected default status %d, got %d", http.StatusOK, sw.StatusCode()) + } +} + +func TestStatWriter_Write_PassthroughOnSuccess(t *testing.T) { + rec := httptest.NewRecorder() + sw := NewWriter(rec, true, false) + + sw.WriteHeader(http.StatusOK) + n, err := sw.Write([]byte("hello")) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if n != 5 { + t.Errorf("expected n=5, got %d", n) + } + if rec.Body.String() != "hello" { + t.Errorf("expected body 'hello', got %q", rec.Body.String()) + } +} + +func TestStatWriter_Write_BuffersOnError(t *testing.T) { + rec := httptest.NewRecorder() + sw := NewWriter(rec, true, false) + + sw.WriteHeader(http.StatusInternalServerError) + n, err := sw.Write([]byte("db broke")) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if n != 8 { + t.Errorf("expected n=8, got %d", n) + } + // Body should NOT be written to recorder (buffered only) + if rec.Body.Len() != 0 { + t.Errorf("expected empty recorder body, got %q", rec.Body.String()) + } + // Buffer should have the content + if sw.buf.String() != "db broke" { + t.Errorf("expected buffer 'db broke', got %q", sw.buf.String()) + } +} + +func TestStatWriter_Write_TeeOnError(t *testing.T) { + rec := httptest.NewRecorder() + sw := NewWriter(rec, true, true) + + sw.WriteHeader(http.StatusBadGateway) + n, err := sw.Write([]byte("upstream error")) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if n != 14 { + t.Errorf("expected n=14, got %d", n) + } + // Body should be written to both recorder and buffer + if rec.Body.String() != "upstream error" { + t.Errorf("expected recorder body 'upstream error', got %q", rec.Body.String()) + } + if !sw.BodyForwarded() { + t.Error("expected BodyForwarded() to be true") + } +} + +func TestStatWriter_BufferOverflow(t *testing.T) { + rec := httptest.NewRecorder() + sw := NewWriter(rec, true, true) + sw.WriteHeader(http.StatusInternalServerError) + + // Write more than 2000 bytes + bigBody := strings.Repeat("x", 2500) + n, err := sw.Write([]byte(bigBody)) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Tee mode: full body should be forwarded to client regardless of buffer limit + if n != 2500 { + t.Errorf("expected n=2500, got %d", n) + } + if rec.Body.Len() != 2500 { + t.Errorf("expected recorder body len 2500, got %d", rec.Body.Len()) + } + // Buffer should be truncated + if !sw.buf.Truncated() { + t.Error("expected buffer to be truncated") + } + if sw.buf.Len() != 2000 { + t.Errorf("expected buffer len 2000, got %d", sw.buf.Len()) + } +} + +func TestStatWriter_DeferredHeader(t *testing.T) { + rec := httptest.NewRecorder() + sw := NewWriter(rec, true, false) + + sw.WriteHeader(http.StatusBadRequest) + + // Header should be deferred (not yet written to recorder) + // httptest.ResponseRecorder defaults Code to 200, only changes on explicit WriteHeader + if sw.headerWritten { + t.Error("expected header to be deferred") + } + + sw.flushHeader() + if !sw.headerWritten { + t.Error("expected header to be written after flushHeader") + } +} + +func TestStatWriter_Unwrap(t *testing.T) { + rec := httptest.NewRecorder() + sw := NewWriter(rec, false, false) + + if sw.Unwrap() != rec { + t.Error("Unwrap should return the underlying ResponseWriter") + } +} + +func TestStatWriter_StatusCodeStr(t *testing.T) { + rec := httptest.NewRecorder() + sw := NewWriter(rec, false, false) + sw.WriteHeader(http.StatusTeapot) + + if sw.StatusCodeStr() != "418" { + t.Errorf("expected '418', got %q", sw.StatusCodeStr()) + } +} diff --git a/middleware/slog.go b/middleware/slog.go index 47aa4ba..fbf0644 100644 --- a/middleware/slog.go +++ b/middleware/slog.go @@ -6,40 +6,52 @@ import ( "time" ) +// Logging returns a standalone middleware that logs requests using structured logging. +// Error responses (>= 400) include the response body in the log. +func Logging(logger *slog.Logger) func(http.Handler) http.Handler { + if logger == nil { + return func(next http.Handler) http.Handler { return next } + } + m := &Middleware{logger: logger} + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + timeStart := time.Now() + respWriter := NewWriter(w, true, true) + + next.ServeHTTP(respWriter, r) + timeDiff := time.Since(timeStart) + + errMsg := m.getErrMsg(respWriter.statusCode, respWriter.buf) + m.log(r, respWriter.StatusCode(), errMsg, timeDiff) + + respWriter.flushHeader() + }) + } +} + func (c *Middleware) log(r *http.Request, statusCode int, errmsg string, dur time.Duration) { if c.logger == nil { return } + + attrs := []slog.Attr{ + slog.String("method", r.Method), + slog.String("url", r.RequestURI), + slog.Duration("req-dur", dur), + slog.Int("response-code", statusCode), + slog.String("ip", userIp(r)), + slog.String("req-id", r.Header.Get("Request-Id")), + } + if IsStatusError(statusCode) { + attrs = append(attrs, slog.String("err-handlerMsg", errmsg)) + } + + level := slog.LevelInfo if IsServerErr(statusCode) { - c.logger.Error("", - slog.String("method", r.Method), - slog.String("url", r.RequestURI), - slog.Duration("req-dur", dur), - slog.Int("response-code", statusCode), - slog.String("ip", userIp(r)), - slog.String("req-id", r.Header.Get("Request-Id")), - slog.String("err-handlerMsg", errmsg), - ) - } else if IsStatusError(statusCode) { - c.logger.Info("", - slog.String("method", r.Method), - slog.String("url", r.RequestURI), - slog.Duration("req-dur", dur), - slog.Int("response-code", statusCode), - slog.String("ip", userIp(r)), - slog.String("req-id", r.Header.Get("Request-Id")), - slog.String("err-handlerMsg", errmsg), - ) - } else { - c.logger.Info("", - slog.String("method", r.Method), - slog.String("url", r.RequestURI), - slog.Duration("req-dur", dur), - slog.Int("response-code", statusCode), - slog.String("ip", userIp(r)), - slog.String("req-id", r.Header.Get("Request-Id")), - ) + level = slog.LevelError } + + c.logger.LogAttrs(r.Context(), level, "", attrs...) } func userIp(r *http.Request) string { diff --git a/server/server.go b/server/server.go deleted file mode 100644 index 199a041..0000000 --- a/server/server.go +++ /dev/null @@ -1,127 +0,0 @@ -package server - -import ( - "context" - "errors" - "fmt" - "net" - "net/http" - "os" - "os/signal" - "syscall" - "time" -) - -type Server struct { - server http.Server - obsServer *http.Server - logger func(msg string, isErr bool) -} - -type Cfg struct { - Addr string - Handler http.Handler - SkipObs bool - ObsAddr string - ObsHandler http.Handler - Logger func(msg string, isErr bool) - ReadHeaderTimeout time.Duration -} - -// New creates a new sever instance that can be started individually -func New(cfg Cfg) (*Server, error) { - - if cfg.Addr == "" { - return nil, fmt.Errorf("server address canot be empty") - - } - if !cfg.SkipObs && cfg.ObsAddr == "" { - return nil, fmt.Errorf("obserbavility server address canot be empty") - } - - s := Server{ - logger: cfg.Logger, - server: http.Server{ - Addr: cfg.Addr, - Handler: cfg.Handler, - ReadHeaderTimeout: cfg.ReadHeaderTimeout, - }, - } - - if !cfg.SkipObs { - s.obsServer = &http.Server{ - Addr: cfg.ObsAddr, - Handler: cfg.ObsHandler, - ReadHeaderTimeout: cfg.ReadHeaderTimeout, - } - } - return &s, nil -} - -func (srv *Server) logMsg(msg string, isErr bool) { - if srv.logger != nil { - srv.logger(msg, isErr) - } -} - -// Start to listen on the configured address -func (srv *Server) Start() error { - - stopDone := make(chan bool, 1) - signalCh := make(chan os.Signal, 1) - signal.Notify(signalCh, syscall.SIGINT, syscall.SIGTERM) - - // handle shutdown - go func() { - <-signalCh - srv.Stop() - stopDone <- true - }() - - // observability server - if srv.obsServer != nil { - go func() { - ln, err := net.Listen("tcp", srv.obsServer.Addr) - if err != nil { - panic(fmt.Sprintf("error starting obserbability server: %v", err)) - } - srv.logMsg(fmt.Sprintf("obserbability server started on: %s", srv.obsServer.Addr), false) - - if err := srv.obsServer.Serve(ln); !errors.Is(err, http.ErrServerClosed) { - panic(fmt.Sprintf("error in obserbability server: %v", err)) - } - - }() - } - - ln, err := net.Listen("tcp", srv.server.Addr) - if err != nil { - return err - } - srv.logMsg(fmt.Sprintf("server started on: %s", srv.server.Addr), false) - - if err = srv.server.Serve(ln); !errors.Is(err, http.ErrServerClosed) { - return err - } - <-stopDone - return nil -} - -// Stop shut down the server cleanly -func (srv *Server) Stop() { - - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - - if err := srv.server.Shutdown(ctx); err != nil { - srv.logMsg(fmt.Sprintf("server shutdown: %v", err), true) - } - srv.logMsg("server stopped", false) - - if srv.obsServer != nil { - if err := srv.obsServer.Shutdown(ctx); err != nil { - srv.logMsg(fmt.Sprintf("observability server shutdown: %v", err), true) - } - } - srv.logMsg("observability server stopped", false) -} From 1c28580b1a9f49185c9837cb5261c5673974b1af Mon Sep 17 00:00:00 2001 From: Andres Bott Date: Sun, 19 Apr 2026 16:21:52 +0200 Subject: [PATCH 2/3] ci --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 094d300..5d590c7 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -20,4 +20,4 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: v2 + version: v2.1 From c13c88f12201cf4525b7a85c07b46386958a61df Mon Sep 17 00:00:00 2001 From: Andres Bott Date: Sun, 19 Apr 2026 16:22:37 +0200 Subject: [PATCH 3/3] ci --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 5d590c7..7f4b7ca 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -18,6 +18,6 @@ jobs: go-version-file: go.mod - name: golangci-lint - uses: golangci/golangci-lint-action@v6 + uses: golangci/golangci-lint-action@v7 with: version: v2.1