From 390ffa19a4305c94b72c47a66de7287fc40603e9 Mon Sep 17 00:00:00 2001 From: djmil Date: Tue, 5 May 2026 00:06:23 +0000 Subject: [PATCH] result.Errf --- .vscode/settings.json | 5 ++++- CLAUDE.md | 2 ++ cmd/app/main.go | 2 +- internal/greeter/greeter.go | 3 +-- internal/logger/logger.go | 24 +++++++++++---------- internal/logger/logger_test.go | 33 ++++++++++++++++++++++++++++ internal/testutil/testutil.go | 31 +++++++++++++++++++++++++++ pkg/result/bench_test.go | 12 +++++------ pkg/result/doc.go | 16 +++++++++++++- pkg/result/example_test.go | 5 ++--- pkg/result/expect_goexit.go | 7 +++--- pkg/result/expect_panic.go | 5 ++--- pkg/result/goexit_test.go | 39 ++++++++++++++++++++++++++++++++++ pkg/result/panic_test.go | 3 +-- pkg/result/result.go | 10 +++++++-- 15 files changed, 161 insertions(+), 36 deletions(-) create mode 100644 internal/logger/logger_test.go create mode 100644 pkg/result/goexit_test.go diff --git a/.vscode/settings.json b/.vscode/settings.json index 03d9e20..438e263 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -30,8 +30,11 @@ "cSpell.words": [ "djmil", "Expectf", + "Failf", "gitea", "golangci", - "testutil" + "nolint", + "testutil", + "Errf" ] } diff --git a/CLAUDE.md b/CLAUDE.md index 1a77bc0..bec81f9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -43,6 +43,7 @@ tools.versions Pinned tool versions (sourced by Makefile and pre-push - `pkg/` libraries **only return** `result.Expect[T]` — never call `.Expect()`, `.Must()`, or `.Expectf()` inside library code; those methods exit the goroutine via `runtime.Goexit` and are only safe in application-layer code protected by a boundary - application code (`cmd/`, HTTP handlers, etc.) chains `.Expect("context")` freely — each call exits the goroutine on failure and is caught at the entry point - top-level entry points defer `result.Catch(&err)` (or use `result.Run(...)`) to convert any result exit into a normal Go error; genuine runtime panics (nil-deref, etc.) are re-panicked + - **`result.Catch` is incompatible with `-tags result_goexit`**: it relies on `recover()` which cannot intercept `runtime.Goexit`; prefer `result.Run`/`result.Go` which work in both builds - bridge existing `(T, error)` stdlib/third-party calls with `result.Of(...)`: `result.Of(os.ReadFile("cfg.json")).Expect("read config")` - use `result.StackTrace(err)` to retrieve the capture-site stack from a caught error - still use `fmt.Errorf("context: %w", err)` when wrapping errors *before* constructing a `result.Fail` @@ -72,6 +73,7 @@ tools.versions Pinned tool versions (sourced by Makefile and pre-push - Table-driven tests with `t.Run("description", ...)` for multiple cases - The race detector is enabled in CI (`make test-race`); don't introduce data races - Never use `time.Sleep` in tests; use channels or `t.Cleanup` +- Use `internal/testutil` helpers instead of manual checks — `ResultOk`, `ResultOkNotNil`, `ResultErr` for `result.Expect[T]`; `NoError`, `Error`, `ErrorContains`, `Equal` for plain values --- diff --git a/cmd/app/main.go b/cmd/app/main.go index a3d8edd..851d490 100644 --- a/cmd/app/main.go +++ b/cmd/app/main.go @@ -33,7 +33,7 @@ func showGreeting() { if cfg.App.Env == "dev" { log = logger.NewDevelopment() } else { - log = result.Of(logger.New(cfg.Logger.Level)).Expect("create logger") + log = logger.New(cfg.Logger.Level).Expect("create logger") } log.WithFields(map[string]any{ diff --git a/internal/greeter/greeter.go b/internal/greeter/greeter.go index 1bcd602..d0b771b 100644 --- a/internal/greeter/greeter.go +++ b/internal/greeter/greeter.go @@ -8,7 +8,6 @@ package greeter import ( - "errors" "fmt" "gitea.djmil.dev/go/template/internal/logger" @@ -34,7 +33,7 @@ func New(log *logger.Logger) *Service { // Greet returns a personalized greeting and logs the interaction. func (s *Service) Greet(name string) result.Expect[string] { if name == "" { - return result.Fail[string](errors.New("Greet: name must not be empty")) + return result.Errf[string]("Greet: name must not be empty") } msg := fmt.Sprintf("Hello, %s!", name) diff --git a/internal/logger/logger.go b/internal/logger/logger.go index c6ec9ec..866e7cf 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -6,7 +6,7 @@ // // Usage: // -// log, _ := logger.New("info") +// log := logger.New("info").Expect("create logger") // log.Info("server started") // // req := log.WithField("request_id", rid).WithField("user_id", uid) @@ -14,10 +14,11 @@ package logger import ( - "fmt" "io" "log/slog" "os" + + "gitea.djmil.dev/go/template/pkg/result" ) // Logger is a thin wrapper around *slog.Logger. @@ -28,15 +29,16 @@ type Logger struct { // New creates a JSON logger writing to stderr for the given level string. // Valid levels: debug, info, warn, error. -func New(level string) (*Logger, error) { - lvl, err := parseLevel(level) - if err != nil { - return nil, err +func New(level string) result.Expect[*Logger] { + lvl := parseLevel(level) + if lvl.Err() != nil { + return result.Errf[*Logger]("parseLevel: %w", lvl.Err()) } - h := slog.NewJSONHandler(os.Stderr, &slog.HandlerOptions{Level: lvl}) + handler := slog.NewJSONHandler(os.Stderr, &slog.HandlerOptions{Level: lvl.Value()}) + logger := &Logger{slog.New(handler)} - return &Logger{slog.New(h)}, nil + return result.Ok(logger) } // NewDevelopment creates a human-friendly text logger writing to stderr. @@ -71,11 +73,11 @@ func (l *Logger) WithFields(fields map[string]any) *Logger { // ── helpers ─────────────────────────────────────────────────────────────────── -func parseLevel(level string) (slog.Level, error) { +func parseLevel(level string) result.Expect[slog.Level] { var lvl slog.Level if err := lvl.UnmarshalText([]byte(level)); err != nil { - return lvl, fmt.Errorf("logger: unknown level %q (use debug|info|warn|error)", level) + return result.Errf[slog.Level]("unknown level %q (use debug|info|warn|error)", level) } - return lvl, nil + return result.Ok(lvl) } diff --git a/internal/logger/logger_test.go b/internal/logger/logger_test.go new file mode 100644 index 0000000..b219e31 --- /dev/null +++ b/internal/logger/logger_test.go @@ -0,0 +1,33 @@ +package logger_test + +import ( + "testing" + + "gitea.djmil.dev/go/template/internal/logger" + "gitea.djmil.dev/go/template/internal/testutil" +) + +func TestNew(t *testing.T) { + tests := []struct { + level string + wantErr bool + }{ + {level: "debug"}, + {level: "info"}, + {level: "warn"}, + {level: "error"}, + {level: "invalid", wantErr: true}, + {level: "", wantErr: true}, + } + + for _, tc := range tests { + t.Run(tc.level, func(t *testing.T) { + r := logger.New(tc.level) + if tc.wantErr { + testutil.ResultErr(t, r) + return + } + testutil.ResultOkNotNil(t, r) + }) + } +} diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go index 2b77066..6da8eed 100644 --- a/internal/testutil/testutil.go +++ b/internal/testutil/testutil.go @@ -8,6 +8,8 @@ package testutil import ( "strings" "testing" + + "gitea.djmil.dev/go/template/pkg/result" ) // NoError fails the test immediately if err is not nil. @@ -42,3 +44,32 @@ func Equal[T comparable](t *testing.T, got, want T) { t.Errorf("got %v, want %v", got, want) } } + +// ResultOk fails the test if r holds an error, then returns the value. +func ResultOk[T any](t *testing.T, r result.Expect[T]) T { + t.Helper() + if r.Err() != nil { + t.Fatalf("unexpected error: %v", r.Err()) + } + return r.Value() +} + +// ResultOkNotNil fails the test if r holds an error or its value is nil. +// T must be a pointer type. +func ResultOkNotNil[T comparable](t *testing.T, r result.Expect[T]) T { + t.Helper() + v := ResultOk(t, r) + var zero T + if v == zero { + t.Fatal("expected non-nil value, got nil") + } + return v +} + +// ResultErr fails the test if r does not hold an error. +func ResultErr[T any](t *testing.T, r result.Expect[T]) { + t.Helper() + if r.Err() == nil { + t.Fatal("expected error, got nil") + } +} diff --git a/pkg/result/bench_test.go b/pkg/result/bench_test.go index cb2afb0..1dd42f5 100644 --- a/pkg/result/bench_test.go +++ b/pkg/result/bench_test.go @@ -248,11 +248,11 @@ func r_parseHeader(raw string) result.Expect[bHeader] { return r_parseHeader2(ra func r_parseHeader2(raw string) result.Expect[bHeader] { return r_parseHeader3(raw) } func r_parseHeader3(raw string) result.Expect[bHeader] { if raw == "" { - return result.Fail[bHeader](errEmpty) + return result.Err[bHeader](errEmpty) } parts := strings.SplitN(raw, "|", 3) if len(parts) != 3 { - return result.Fail[bHeader](fmt.Errorf("malformed record: %q", raw)) + return result.Errf[bHeader]("malformed record: %q", raw) } return result.Ok(bHeader{raw: raw, id: parts[0], name: parts[1], val: parts[2]}) } @@ -273,10 +273,10 @@ func r_validate4(h bHeader) result.Expect[bFields] { return r_validate5(h) } func r_validate5(h bHeader) result.Expect[bFields] { id, err := strconv.Atoi(h.id) if err != nil { - return result.Fail[bFields](fmt.Errorf("parse id %q: %w", h.id, err)) + return result.Errf[bFields]("parse id %q: %w", h.id, err) } if id <= 0 { - return result.Fail[bFields](fmt.Errorf("id %d: must be > 0", id)) + return result.Errf[bFields]("id %d: must be > 0", id) } return result.Ok(bFields{id: id, name: h.name, val: h.val}) } @@ -297,7 +297,7 @@ func r_transform4(f bFields) result.Expect[bRecord] { return r_transform5(f) } func r_transform5(f bFields) result.Expect[bRecord] { v, err := strconv.ParseFloat(f.val, 64) if err != nil { - return result.Fail[bRecord](fmt.Errorf("parse value %q: %w", f.val, err)) + return result.Errf[bRecord]("parse value %q: %w", f.val, err) } return result.Ok(bRecord{id: f.id, name: f.name, score: v * 1.5}) } @@ -336,7 +336,7 @@ func r_enrich8(r bRecord) result.Expect[bRecord] { return r_enrich9(r) } func r_enrich9(r bRecord) result.Expect[bRecord] { return r_enrich10(r) } func r_enrich10(r bRecord) result.Expect[bRecord] { if r.score < 0 { - return result.Fail[bRecord](errNegScore) + return result.Err[bRecord](errNegScore) } return result.Ok(bRecord{id: r.id, name: r.name, score: r.score + 10.0}) } diff --git a/pkg/result/doc.go b/pkg/result/doc.go index 01a1bcb..0ab5dbf 100644 --- a/pkg/result/doc.go +++ b/pkg/result/doc.go @@ -34,7 +34,7 @@ // // # Constructors // -// Use [Ok] to wrap a success value, [Fail] to wrap an error, and [Of] to +// Use [Ok] to wrap a success value, [Err] to wrap an error, and [Of] to // bridge existing (value, error) return signatures: // // data := result.Of(os.ReadFile("cfg.json")).Expect("read config") @@ -51,6 +51,20 @@ // [Go] is the typed variant — it returns Expect[T] when the closure produces // a value. [Run] is a convenience wrapper for closures that return nothing. // +// [Catch] is an alternative boundary for use with named error returns: +// +// func load() (err error) { +// defer result.Catch(&err) +// port := parsePort(cfg.Port).Expect("load config port") +// _ = port +// return +// } +// +// Important: [Catch] relies on recover() and only works with the default +// (panic) build. With -tags result_goexit, Expect and Expectf exit via +// runtime.Goexit which recover() cannot intercept — use [Run] or [Go] instead, +// as they work correctly in both builds. +// // Genuine runtime panics (nil-pointer dereferences, index out of bounds, etc.) // are not recovered — they still crash the program, as they should. package result diff --git a/pkg/result/example_test.go b/pkg/result/example_test.go index 74cf444..500bf4a 100644 --- a/pkg/result/example_test.go +++ b/pkg/result/example_test.go @@ -1,7 +1,6 @@ package result_test import ( - "errors" "fmt" "strconv" @@ -11,7 +10,7 @@ import ( // parseHost is an example of a simple utility function, that validates a hostname. func parseHost(s string) result.Expect[string] { if s == "" { - return result.Fail[string](errors.New("host must not be empty")) + return result.Errf[string]("host must not be empty") } return result.Ok(s) } @@ -23,7 +22,7 @@ func parsePort(s string) result.Expect[int] { return port } if port.Value() < 1 || port.Value() > 65535 { - return result.Fail[int](fmt.Errorf("%d out of range", port.Value())) + return result.Errf[int]("%d out of range", port.Value()) } return port } diff --git a/pkg/result/expect_goexit.go b/pkg/result/expect_goexit.go index a280229..19e856a 100644 --- a/pkg/result/expect_goexit.go +++ b/pkg/result/expect_goexit.go @@ -15,7 +15,6 @@ package result import ( - "errors" "fmt" "runtime" "sync" @@ -97,7 +96,7 @@ func Async[T any](fn func() T) <-chan Expect[T] { if v := recover(); v != nil { if err, ok := v.(*stackError); ok { // Must() panic — treat as a collected failure. - ch <- Fail[T](err) + ch <- Err[T](err) return } panic(v) // genuine runtime panic — crash the program @@ -108,9 +107,9 @@ func Async[T any](fn func() T) <-chan Expect[T] { } // goroutineID is looked up here, on the error path only. if stored, ok := gErrors.LoadAndDelete(goroutineID()); ok { - ch <- Fail[T](stored.(error)) + ch <- Err[T](stored.(error)) } else { - ch <- Fail[T](errors.New("goroutine exited unexpectedly")) + ch <- Errf[T]("goroutine exited unexpectedly") } }() val = fn() diff --git a/pkg/result/expect_panic.go b/pkg/result/expect_panic.go index a2ce57d..f769b92 100644 --- a/pkg/result/expect_panic.go +++ b/pkg/result/expect_panic.go @@ -29,7 +29,6 @@ package result import ( - "errors" "fmt" ) @@ -87,7 +86,7 @@ func Async[T any](fn func() T) <-chan Expect[T] { if v := recover(); v != nil { if se, ok := v.(*stackError); ok { // Expect/Must panic — treat as a collected failure. - ch <- Fail[T](se) + ch <- Err[T](se) return } panic(v) // genuine runtime panic — crash the program @@ -96,7 +95,7 @@ func Async[T any](fn func() T) <-chan Expect[T] { ch <- Ok(val) return } - ch <- Fail[T](errors.New("goroutine exited unexpectedly")) + ch <- Errf[T]("goroutine exited unexpectedly") }() val = fn() finished = true diff --git a/pkg/result/goexit_test.go b/pkg/result/goexit_test.go new file mode 100644 index 0000000..2f46ce3 --- /dev/null +++ b/pkg/result/goexit_test.go @@ -0,0 +1,39 @@ +//go:build result_goexit + +// Run with: go test -tags result_goexit -run Test ./pkg/result/... +// +// The -run Test flag is required: Example_catch uses defer result.Catch inside +// a single goroutine, which cannot intercept runtime.Goexit — examples that +// rely on Catch are incompatible with this build tag. + +package result_test + +import ( + "errors" + "testing" + + "gitea.djmil.dev/go/template/pkg/result" +) + +// TestExpectNotRecoverable verifies the core safety property of the goexit +// build: an Expect failure exits via runtime.Goexit, which is invisible to +// recover() — user code inside a Run/Go closure cannot accidentally swallow it. +func TestExpectNotRecoverable(t *testing.T) { + swallowed := false + + err := result.Run(func() { + defer func() { + if recover() != nil { + swallowed = true + } + }() + result.Err[int](errors.New("oops")).Expect("test") + }) + + if swallowed { + t.Fatal("Expect failure was caught by recover() — goexit build should prevent this") + } + if err == nil { + t.Fatal("expected Run to collect the error, got nil") + } +} diff --git a/pkg/result/panic_test.go b/pkg/result/panic_test.go index 7d7362c..914d9f9 100644 --- a/pkg/result/panic_test.go +++ b/pkg/result/panic_test.go @@ -1,7 +1,6 @@ package result_test import ( - "errors" "os" "os/exec" "strings" @@ -17,7 +16,7 @@ import ( // without a subprocess, so it is only documented here. func TestMustCollected(t *testing.T) { err := result.Run(func() { - result.Fail[int](errors.New("unrecoverable")).Must() + result.Errf[int]("unrecoverable").Must() }) if err == nil { diff --git a/pkg/result/result.go b/pkg/result/result.go index ccdcef0..788bc3f 100644 --- a/pkg/result/result.go +++ b/pkg/result/result.go @@ -43,11 +43,17 @@ func Ok[T any](v T) Expect[T] { return Expect[T]{value: v} } -// Fail wraps an error in an Expect. -func Fail[T any](err error) Expect[T] { +// Err wraps an error in an Expect. +func Err[T any](err error) Expect[T] { return Expect[T]{err: err} } +// Errf wraps a formatted error in an Expect. It is a convenience shorthand +// for [Err][fmt.Errorf(format, args...)]. +func Errf[T any](format string, args ...any) Expect[T] { + return Expect[T]{err: fmt.Errorf(format, args...)} +} + // Of is a convenience constructor that bridges standard Go (value, error) // return signatures: //