diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index bfd826e..a00e777 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -16,17 +16,17 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set up Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v5 with: - go-version: 1.23 + go-version: "1.25" - name: Setup golangci-lint - uses: golangci/golangci-lint-action@v3.1.0 + uses: golangci/golangci-lint-action@v8 with: - version: v1.61.0 + version: v2.5.0 - name: Test run: make test coverage diff --git a/.golangci.yml b/.golangci.yml index d9246f8..65f400b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,81 +3,81 @@ # license that can be found in the LICENSE file. --- +version: "2" + run: timeout: 1m - skip-files: - - internal/* linters: - enable-all: true + default: all disable: - wrapcheck - testpackage - gochecknoglobals - - exhaustivestruct - exhaustruct - paralleltest - godox - cyclop - tagliatelle - - goerr113 + - err113 # renamed from goerr113 - varnamelen - errorlint - forcetypeassert - perfsprint - depguard - testifylint - fast: false - -# Settings for specific linters -linters-settings: - funlen: - lines: 150 - statements: 45 - -issues: - exclude-rules: - - path: cmd/ - linters: - - gochecknoinits - - gomnd - - forbidigo - - lll - - - path: example_test.go - linters: - - lll - - dupword - - - path: internal/http/ - linters: - - unparam - - nlreturn - - - path: _test\.go - linters: - - scopelint - - wsl - - nlreturn - - funlen - - dupl - - gocognit - - nosnakecase - - lll - - - path: doc.go - linters: - - lll - - - linters: - - lll - source: "json:" - - - linters: - - lll - source: "Err" + # Linters introduced in golangci-lint v2 that are not part of this repo's + # v1 baseline and conflict with its established idioms; kept off so the v2 + # migration preserves the previous effective rule set. + - noinlineerr # idiomatic `if err := f(); err != nil` is used throughout + - funcorder # repo groups code via "//////" section dividers, not ctor-first + - wsl_v5 # superseded ruleset; `wsl` (below) preserves the prior behavior + settings: + funlen: + lines: 150 + statements: 45 + exclusions: + paths: + - internal/.* + rules: + - path: cmd/ + linters: + - gochecknoinits + - mnd # renamed from gomnd + - forbidigo + - lll + - path: example_test.go + linters: + - lll + - dupword + - path: internal/http/ + linters: + - unparam + - nlreturn + - path: _test\.go + linters: + - wsl + - nlreturn + - funlen + - dupl + - gocognit + - lll + - gosmopolitan # i18n test fixtures legitimately contain non-Latin text + - path: doc.go + linters: + - lll + - source: "json:" + linters: + - lll + - source: "Err" + linters: + - lll + - source: "//////" + linters: + - gocritic + - godot - - linters: - - gocritic - - godot - source: "//////" +formatters: + enable: + - gofmt + - goimports diff --git a/CHANGELOG.md b/CHANGELOG.md index d638c67..56fa0a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,30 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.0.0] - 2026-05-30 +### Changed +- **BREAKING:** module path is now `github.com/thalesfsp/customerror/v2`. +- **BREAKING:** minimum Go is now 1.25 (`go.mod` `go 1.25.0`); CI runs Go 1.25. +- `From` returns a modified copy instead of mutating the original (safe for shared sentinels). +- `WithLanguage`/`WithTranslation`/`FormatError` no longer panic on invalid input; they degrade gracefully. +- `ErrorCode` validation is now strict/anchored: only letters, digits, and underscores are accepted. +- `Catalog.Get`/`MustGet` apply the provided options and return a copy (the stored entry is never handed out or mutated). +- `WithFields` merges into existing fields instead of replacing them. +- `Wrap` preserves the identity of every wrapped error (`errors.Is`/`errors.As` match all of them). +- Chinese language code corrected to ISO 639-1 `zh` (was `ch`); Italian "failed to" template spelling fixed. +- Dependencies updated to latest; linting migrated to golangci-lint v2. + +### Removed +- **BREAKING:** removed the unused exported `CustomError.LanguageErrorTypeMap` field. + +### Fixed +- `New` no longer terminates the process (`log.Fatalf`/`os.Exit`) on invalid input; it panics (recoverable). +- `NewHTTPError` (method) no longer mutates its receiver (no more "sticky" status code on reused factories). +- `MarshalJSON` no longer lets user fields clobber structural keys (`code`, `message`, `statusCode`, `tags`, `retryable`, `retried`). +- `Error()`/`APIError()` no longer emit dangling `". Tags:"`/`". Fields:"` for empty sets/maps. +- Instance factory methods honor ignore options without a nil type-assertion panic. +- Resolved the open Dependabot security advisories by upgrading dependencies. + ## [1.1.1] - 2023-03-29 ### Added - Added `NewNotFoundError`. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..157c020 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,77 @@ +# customerror — project guide for Claude + +Go library providing rich, structured custom errors (`*CustomError`) with a code, +HTTP status code, tags, fields, i18n/translations, retryability, and an error +`Catalog`. Module path: `github.com/thalesfsp/customerror/v2` (v2 — importers use +the `/v2` suffix; the package name is still `customerror`). + +## Layout +- `customerror.go` — core `CustomError` type; `Error()/APIError()/JustError()`; + `MarshalJSON`; `Is`/`Unwrap`; `Copy`; package funcs (`New`, `Factory`, `From`, + `Wrap`, `To`, `IsCustomError`, `IsHTTPStatus`, `IsErrorCode`, `IsRetryable`); + instance factory methods (`cE.NewFailedToError/NewInvalidError/NewMissingError/ + NewRequiredError/NewHTTPError/New`, `FormatError`, `NewChildError`). +- `builtin.go` — package-level constructors (`NewFailedToError`, `NewInvalidError`, + `NewMissingError`, `NewRequiredError`, `NewNotFoundError`, `NewHTTPError`). +- `options.go` — functional options + `prependOptions`. +- `language.go` — `Language` type, ISO 639-1/3166-1 regex, `NewLanguage`. +- `languageprefixtemplate.go` — built-in translation templates (singleton via + `sync.Once`), `GetTemplate`, `AddNewLanguage`, `NewErrorPrefixMap`. +- `catalog.go` — `Catalog` (error code → error), `ErrorCode` validation. +- Tests: `customerror_test.go`, `catalog_test.go`, + `languageprefixtemplate_test.go`, `example_test.go` (runnable Examples that + assert EXACT stdout), `bugfixes_test.go` (regression tests; happy/bad/edge). + +## Conventions +- Functional options (Rob Pike style). Options are `func(*CustomError)` and + cannot return errors, so invalid input is ignored gracefully (do NOT add + `panic` to options — `WithLanguage`/`WithTranslation` ignore bad codes). +- `//////` section dividers; doc comment on every exported symbol; comments end + with a period (godot is enabled). +- `Copy(src, target)` mutates and returns `target`, copying only NON-ZERO src + fields; `LanguageMessageMap`/`Fields`/`Tags` are merged into fresh containers. +- Factory methods intentionally apply options twice (in `FormatError`/method + body, then again in the inner package constructor) and depend on `Copy`'s + "non-zero overwrites" merge for option precedence (e.g. a user `WithStatusCode` + must win over a built-in default). Do NOT "dedupe" this without re-checking + precedence — it is load-bearing. + +## Gotchas +- `Error()` output is non-deterministic for >1 field (`sync.Map` range order); + assert with `strings.Contains`, not equality. +- `example_test.go` asserts EXACT stdout — any message-format change breaks it. +- The template singleton (`languageprefixtemplate.go`) is package-global and + persists across tests. In tests, register/use UNIQUE language codes (e.g. + "xy", "qq") so you don't pollute other tests. +- `New`/`Factory` PANIC (recoverable) on invalid attributes: message `gte=3`, + code `gte=2`, status `100..511`. They must NEVER call `os.Exit`/`log.Fatalf`. +- `From(*CustomError, …)` returns a modified COPY (never mutates the original); + so `errors.Is(From(x,…), x)` is false for a `*CustomError` input. + +## Build / test / lint +- `make test` / `make coverage` — `go test -race -cover` (passes; ~93%). +- `make lint` — golangci-lint with `.golangci.yml` (**v2 format**, `default: all`). + CI pins **golangci-lint v2.5.0** + **Go 1.25** (`.github/workflows/go.yml`, + `golangci/golangci-lint-action@v8`). A modern (v2.x) golangci-lint runs the + config directly — no version gymnastics needed. + - The v2 migration disabled three linters that are new in v2 and clash with + the repo's idioms, to preserve the prior baseline: `noinlineerr`, + `funcorder`, `wsl_v5` (the old `wsl` stays). Re-enable deliberately if you + want to adopt them (each needs codebase-wide changes). + - `default: all` is strict: `intrange`/`copyloopvar` are on (use + `for i := range n`); non-Latin template strings need a `gosmopolitan` + exception (test files are already excluded) or `//nolint:gosmopolitan`. + +## Dependency policy +- Tracks the **latest** releases (go.mod `go` directive is `1.25.0`, matching CI). + When bumping, confirm a candidate's required Go via + `curl -s https://proxy.golang.org//@v/.mod | grep '^go '` and keep + it ≤ the CI Go. Do NOT add a `toolchain` directive (a library shouldn't pin one). + +## CI & releases +- The workflow runs ONLY on push to `main` and PRs targeting `main`. Pushing a + feature branch does NOT trigger CI. +- Fresh clones may not fetch tags (`git fetch --tags`). Versioned via semver + tags. This module is **v2** (`/v2` path); the v1 line stopped at `v1.2.9`. + A breaking change now requires a `/v3` path; prefer additive/deprecation + within `v2.x`. diff --git a/README.md b/README.md index 965cc53..21e7078 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,13 @@ ## Install -`$ go get github.com/thalesfsp/customerror@vX.Y.Z` +`$ go get github.com/thalesfsp/customerror/v2@latest` + +Import it (the package name remains `customerror`): + +```go +import "github.com/thalesfsp/customerror/v2" +``` ## Usage @@ -12,7 +18,7 @@ See [`example_test.go`](example_test.go), and [`customerror_test.go`](customerro ## Documentation -Run `$ make doc` or check out [online](https://pkg.go.dev/github.com/thalesfsp/customerror). +Run `$ make doc` or check out [online](https://pkg.go.dev/github.com/thalesfsp/customerror/v2). ## Development diff --git a/bugfixes_test.go b/bugfixes_test.go new file mode 100644 index 0000000..72466b1 --- /dev/null +++ b/bugfixes_test.go @@ -0,0 +1,664 @@ +// Copyright 2021 The customerror Authors. All rights reserved. +// Use of this source code is governed by a MIT +// license that can be found in the LICENSE file. + +// Regression tests for the bug fixes. Each test block covers a happy path, a +// bad/buggy path (the behavior that was broken before the fix), and edge cases. + +package customerror + +import ( + "encoding/json" + "errors" + "fmt" + "net/http" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +////// +// #1 - New must NOT terminate the process (no log.Fatalf/os.Exit). Invalid +// input panics (recoverable) instead. +////// + +func TestFix_New_InvalidInputPanicsButDoesNotExit(t *testing.T) { + // Bad path: invalid attributes must panic (recoverable), NOT call os.Exit. + mustPanic := func(message string) { + defer func() { + r := recover() + require.NotNil(t, r, "expected a (recoverable) panic for message %q", message) + assert.Contains(t, fmt.Sprintf("%v", r), "Invalid custom error") + }() + + _ = New(message) + } + + mustPanic("") // empty message + mustPanic("ab") // 2 chars, below the gte=3 minimum + + // Reaching this point proves the process was NOT terminated by the panics + // above (a log.Fatalf/os.Exit would have killed the test binary). + + // Happy path + edge: exactly 3 chars (the boundary) is valid. + assert.NotPanics(t, func() { + assert.NotNil(t, New("abc")) + assert.NotNil(t, New("a valid message")) + }) +} + +////// +// #2 - The NewHTTPError method must not mutate its receiver. +////// + +func TestFix_NewHTTPError_DoesNotMutateReceiver(t *testing.T) { + base := Factory("something") + + // Happy path. + first, ok := To(base.NewHTTPError(http.StatusNotFound)) + require.True(t, ok) + assert.Equal(t, http.StatusNotFound, first.StatusCode) + assert.Equal(t, "not found", first.Message) + + // The receiver must remain untouched. + assert.Equal(t, 0, base.StatusCode, "receiver StatusCode must not be mutated") + + // Bad path (the bug): reusing the same factory must NOT make the first + // status code "stick". + second, ok := To(base.NewHTTPError(http.StatusInternalServerError)) + require.True(t, ok) + assert.Equal(t, http.StatusInternalServerError, second.StatusCode) + assert.Equal(t, 0, base.StatusCode) + + // Edge: a receiver with a preset status code keeps it (receiver wins), and + // is still not mutated. + preset := Factory("teapot", WithStatusCode(http.StatusTeapot)) + got, ok := To(preset.NewHTTPError(http.StatusNotFound)) + require.True(t, ok) + assert.Equal(t, http.StatusTeapot, got.StatusCode) + assert.Equal(t, http.StatusTeapot, preset.StatusCode) + + // Edge: nil receiver returns nil. + var nilCE *CustomError + assert.Nil(t, nilCE.NewHTTPError(http.StatusNotFound)) +} + +////// +// #3 - MarshalJSON: user fields must not clobber structural keys. +////// + +func TestFix_MarshalJSON_FieldsDoNotClobberReservedKeys(t *testing.T) { + // Happy path: ordinary fields are emitted at the top level. + okFields := New("real message", WithErrorCode("E1010"), WithField("custom", "value")).(*CustomError) + b, err := json.Marshal(okFields) + require.NoError(t, err) + + var m map[string]interface{} + require.NoError(t, json.Unmarshal(b, &m)) + assert.Equal(t, "value", m["custom"]) + assert.Equal(t, "E1010", m["code"]) + + // Bad path (the bug): a field named like a reserved key must NOT override + // the structural value. + hijack := New( + "real message", + WithErrorCode("E1010"), + WithStatusCode(http.StatusBadRequest), + WithField("code", "HIJACKED"), + WithField("message", "HIJACKED"), + WithField("statusCode", 999), + ).(*CustomError) + + b, err = json.Marshal(hijack) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(b, &m)) + + assert.Equal(t, "E1010", m["code"], "field must not clobber code") + assert.Equal(t, "real message", m["message"], "field must not clobber message") + assert.Equal(t, float64(http.StatusBadRequest), m["statusCode"], "field must not clobber statusCode") + + // Edge: a "statusCode" field with no real status code is dropped (not + // emitted as the field value either). + noStatus := New("msg here", WithField("statusCode", 12345)).(*CustomError) + b, err = json.Marshal(noStatus) + require.NoError(t, err) + assert.NotContains(t, string(b), "12345") + + // Edge: empty key and nil value are skipped. + weird := New("msg here", WithField("", "x"), WithField("nilval", nil), WithField("good", 1)).(*CustomError) + b, err = json.Marshal(weird) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(b, &m)) + _, hasEmpty := m[""] + assert.False(t, hasEmpty) + _, hasNil := m["nilval"] + assert.False(t, hasNil) + assert.Equal(t, float64(1), m["good"]) +} + +////// +// #4 - WithFields must merge (not wipe existing fields). +////// + +func TestFix_WithFields_Merges(t *testing.T) { + // Happy path: all keys present. + e := New("message", WithFields(map[string]interface{}{"a": 1, "b": 2})).(*CustomError) + assertField(t, e, "a", 1) + assertField(t, e, "b", 2) + + // Bad path (the bug): WithField then WithFields must keep both. + e = New("message", + WithField("a", 1), + WithFields(map[string]interface{}{"b": 2}), + ).(*CustomError) + assertField(t, e, "a", 1) + assertField(t, e, "b", 2) + + // Edge: an empty map preserves existing fields (and adds no dangling state). + e = New("message", + WithField("a", 1), + WithFields(map[string]interface{}{}), + ).(*CustomError) + assertField(t, e, "a", 1) + + // Edge: WithFields twice merges; same key is overwritten with the latest. + e = New("message", + WithFields(map[string]interface{}{"a": 1}), + WithFields(map[string]interface{}{"a": 99, "b": 2}), + ).(*CustomError) + assertField(t, e, "a", 99) + assertField(t, e, "b", 2) +} + +func assertField(t *testing.T, e *CustomError, key string, want interface{}) { + t.Helper() + + require.NotNil(t, e.Fields) + got, ok := e.Fields.Load(key) + assert.True(t, ok, "expected field %q to be present", key) + assert.Equal(t, want, got) +} + +////// +// #5 - Catalog.Get/MustGet: apply opts and return a copy (not the shared +// pointer). +////// + +func TestFix_CatalogGet_AppliesOptsAndReturnsCopy(t *testing.T) { + c := MustNewCatalog("myapp") + c.MustSet("E1010", "original message") + + // Happy path. + got, err := c.Get("E1010") + require.NoError(t, err) + assert.Equal(t, "original message", got.Message) + + // Bad path (the bug 1): opts passed to Get/MustGet must be applied. + withOpt := c.MustGet("E1010", WithStatusCode(http.StatusTeapot)) + assert.Equal(t, http.StatusTeapot, withOpt.StatusCode) + + // The stored entry must be unaffected by the applied opt. + stored, err := c.Get("E1010") + require.NoError(t, err) + assert.NotEqual(t, http.StatusTeapot, stored.StatusCode) + + // Bad path (the bug 2): mutating the returned error must not corrupt the + // catalog, and two Gets must return distinct pointers. + got1, _ := c.Get("E1010") + got2, _ := c.Get("E1010") + assert.True(t, got1 != got2, "Get must return distinct copies") + + got1.SetMessage("MUTATED") + fresh, _ := c.Get("E1010") + assert.Equal(t, "original message", fresh.Message, "catalog entry must not be mutated") + + // Bad path: not found / invalid code. + _, err = c.Get("DOES_NOT_EXIST") + assert.ErrorIs(t, err, ErrCatalogErrorNotFound) + + _, err = c.Get("invalid code!") + assert.Error(t, err) + + // Edge: MustGet panics on a missing code. + assert.Panics(t, func() { _ = c.MustGet("DOES_NOT_EXIST") }) +} + +////// +// #6 - ErrorCode regex must actually validate (anchored). +////// + +func TestFix_ErrorCodeRegex_Anchored(t *testing.T) { + // Happy path: legitimate codes are accepted. + for _, code := range []string{ + "E1010", "INVALID_REQUEST_BODY", "ERR_INVALID_HARD_DRIVE_PATH", + "CE_ERR_INVALID_LANG_CODE", "ERR_A1_B2", "E12345678", "AbCd123", "X", "_", + } { + _, err := NewErrorCode(code) + assert.NoError(t, err, "expected %q to be valid", code) + } + + // Bad path (the bug): junk with spaces/punctuation must be rejected. + for _, code := range []string{ + "hello world", "!!!abc!!!", "@@@", "ERR-123", "", "a b", "x.y", + } { + _, err := NewErrorCode(code) + assert.ErrorIs(t, err, ErrErrorCodeInvalidCode, "expected %q to be rejected", code) + } +} + +////// +// #7 - Language regex must be fully anchored ("default" alternative). +////// + +func TestFix_LanguageRegex_Anchored(t *testing.T) { + // Happy path. + for _, lang := range []string{"en", "en-US", "pt-BR", "default"} { + _, err := NewLanguage(lang) + assert.NoError(t, err, "expected %q to be valid", lang) + } + + // Bad path (the bug): partial "default" matches must be rejected, along + // with malformed codes. + for _, lang := range []string{"mydefault", "defaultx", "xdefault", "EN", "e", "eng", "en-us", "en-USA", ""} { + _, err := NewLanguage(lang) + assert.ErrorIs(t, err, ErrInvalidLanguageCode, "expected %q to be rejected", lang) + } + + // Edge: GetRoot keeps working with the (preserved) single capture group. + assert.Equal(t, "en", Language("en-US").GetRoot()) + assert.Equal(t, "pt", Language("pt-BR").GetRoot()) + assert.Equal(t, "en", Language("en").GetRoot()) + assert.Equal(t, "default", Language("default").GetRoot()) +} + +////// +// #8 - From must return a copy, never mutate the original. +////// + +func TestFix_From_DoesNotMutateOriginal(t *testing.T) { + sentinel := New("sentinel error", WithErrorCode("E1010")).(*CustomError) + + // Happy path: the returned error has the new options applied. + modified := From(sentinel, WithErrorCode("E9999")).(*CustomError) + assert.Equal(t, "E9999", modified.Code) + + // Bad path (the bug): the original sentinel must be unchanged. + assert.Equal(t, "E1010", sentinel.Code, "From must not mutate the original error") + + // Edge: From on a non-CustomError wraps it, applies opts, and stays + // chain-compatible. + plain := errors.New("plain error") + wrapped := From(plain, WithErrorCode("E1234")).(*CustomError) + assert.Equal(t, "E1234", wrapped.Code) + assert.ErrorIs(t, wrapped, plain) + + // Edge: From with no opts returns an independent copy. + cp := From(sentinel).(*CustomError) + assert.True(t, cp != sentinel, "From must return a distinct copy") + cp.SetMessage("changed") + assert.Equal(t, "sentinel error", sentinel.Message) +} + +////// +// #9 - No dangling ". Tags:" / ". Fields:" for empty (non-nil) sets/maps. +////// + +func TestFix_EmptyTagsFields_NoDanglingOutput(t *testing.T) { + // Bad path (the bug): empty (but non-nil) tags and fields must not render. + e := New("boom", WithTag(), WithFields(map[string]interface{}{})).(*CustomError) + assert.Equal(t, "boom", e.Error()) + assert.Equal(t, "boom", e.APIError()) + + // Edge: only-empty-tags and only-empty-fields independently produce nothing. + assert.Equal(t, "boom", New("boom", WithTag()).(*CustomError).Error()) + assert.Equal(t, "boom", New("boom", WithFields(map[string]interface{}{})).(*CustomError).Error()) + + // Edge: MarshalJSON must omit an empty "tags" key. + b, err := json.Marshal(e) + require.NoError(t, err) + assert.NotContains(t, string(b), "tags") + + // Happy path: non-empty tags and fields still render. + full := New("boom", WithTag("t1", "t2"), WithField("k", "v")).(*CustomError) + assert.Contains(t, full.Error(), ". Tags: t1, t2") + assert.Contains(t, full.Error(), ". Fields:") + assert.Contains(t, full.Error(), "k=v") +} + +////// +// #10 - Factory methods apply options correctly despite internal re-application +// (idempotency regression). +////// + +func TestFix_FactoryMethods_OptionPrecedenceAndNoDuplication(t *testing.T) { + base := Factory("disk") + + // A WithStatusCode override must win over the built-in default (500). + failed, ok := To(base.NewFailedToError(WithStatusCode(http.StatusTeapot))) + require.True(t, ok) + assert.Equal(t, http.StatusTeapot, failed.StatusCode) + assert.Equal(t, "failed to disk", failed.Message) + + // Default status code is still applied when not overridden. + failedDefault, ok := To(base.NewFailedToError()) + require.True(t, ok) + assert.Equal(t, http.StatusInternalServerError, failedDefault.StatusCode) + + // Tags/fields are not duplicated. + tagged, ok := To(base.NewInvalidError(WithTag("t1"), WithField("k", "v"))) + require.True(t, ok) + assert.Equal(t, 1, tagged.Tags.Size()) + assertField(t, tagged, "k", "v") +} + +////// +// #11 - Options and FormatError must not panic on bad input. +////// + +func TestFix_NoPanicOnInvalidLanguage(t *testing.T) { + // Bad path (the bug): an invalid language code must not panic. + assert.NotPanics(t, func() { + e := New("base message", WithLanguage("not-a-lang")).(*CustomError) + assert.Equal(t, "base message", e.Message) // default kept + }) + + assert.NotPanics(t, func() { + _ = New("base message", WithTranslation("BAD_CODE", "x")) + }) + + // Edge: a valid code that is not present in the map is ignored (default). + assert.NotPanics(t, func() { + e := New("base message", WithLanguage("fr")).(*CustomError) + assert.Equal(t, "base message", e.Message) + }) + + // Happy path: a valid, present translation is used. + e := New("base message", + WithTranslation("fr", "message français"), + WithLanguage("fr"), + ).(*CustomError) + assert.Equal(t, "message français", e.Message) +} + +func TestFix_FormatError_NoTemplateDegradesGracefully(t *testing.T) { + // Register a language whose template map is intentionally incomplete (only + // has the "failed to" template, not "invalid"). + partial := &sync.Map{} + partial.Store(FailedTo, "qq-failed %s") + require.NoError(t, AddNewLanguage("qq", partial)) + + cE := Factory("thing", WithTranslation("qq", "translated thing")) + + // Bad path (the bug): a missing template must NOT panic; it degrades to the + // unprefixed (translated) message. + var result error + assert.NotPanics(t, func() { + result = cE.NewInvalidError(WithLanguage("qq")) + }) + assert.Equal(t, "translated thing", result.Error()) +} + +////// +// #12 - Chinese must use the correct ISO 639-1 code ("zh"). +////// + +func TestFix_ChineseLanguageCode(t *testing.T) { + // Happy path / correctness. + assert.Equal(t, "zh", string(Chinese)) + assert.Contains(t, BuiltInLanguages, "zh") + assert.NotContains(t, BuiltInLanguages, "ch") + + tmpl, err := GetTemplate("zh", string(FailedTo)) + require.NoError(t, err) + assert.Equal(t, "无法 %s", tmpl) + + // Edge: the old (wrong) "ch" code is no longer a registered language. + _, err = GetTemplate("ch", string(FailedTo)) + assert.ErrorIs(t, err, ErrLanguageNotFound) +} + +////// +// #13 - Italian "failed to" template spelling. +////// + +func TestFix_ItalianFailedToSpelling(t *testing.T) { + tmpl, err := GetTemplate("it", string(FailedTo)) + require.NoError(t, err) + assert.Equal(t, "impossibile %s", tmpl) //nolint:misspell // correct Italian, not an English typo +} + +////// +// #14 - The dead LanguageErrorTypeMap field is gone (not serialized). +////// + +func TestFix_NoLanguageErrorTypeMapInJSON(t *testing.T) { + b, err := json.Marshal(New("a message", WithErrorCode("E1010")).(*CustomError)) + require.NoError(t, err) + assert.NotContains(t, string(b), "languageErrorTypeMap") +} + +////// +// #15 - The shared validator instance is safe under concurrent use. +////// + +func TestFix_ConcurrentNewIsRaceFree(t *testing.T) { + const n = 50 + + results := make(chan error, n) + + for i := range n { + go func(i int) { + results <- New(fmt.Sprintf("message %d", i), WithErrorCode("E1010"), WithStatusCode(http.StatusBadRequest)) + }(i) + } + + for range n { + assert.NotNil(t, <-results) + } +} + +////// +// #16 - Factory honors WithIgnoreFunc (and the removed dead nil-checks don't +// change behavior). +////// + +func TestFix_FactoryAndNewHappyAndIgnore(t *testing.T) { + // Happy path. + assert.NotNil(t, Factory("a valid message")) + + // Ignore path: Factory returns nil when ignored. + assert.Nil(t, Factory("ignore me", WithIgnoreString("ignore"))) + + // Ignore path: New returns nil when ignored. + assert.Nil(t, New("ignore me", WithIgnoreString("ignore"))) +} + +////// +// #17 - AddNewLanguage can update an existing language (Store, not LoadOrStore). +////// + +func TestFix_AddNewLanguage_UpdatesExisting(t *testing.T) { + // Happy path: register a brand-new language. + require.NoError(t, AddNewLanguage("xy", NewErrorPrefixMap( + "xy-failed %s", "xy-invalid %s", "xy-missing %s", "xy-required %s", "xy-notfound %s", + ))) + + tmpl, err := GetTemplate("xy", string(FailedTo)) + require.NoError(t, err) + assert.Equal(t, "xy-failed %s", tmpl) + + // Bad path: an invalid language code returns an error (and does not panic). + assert.Error(t, AddNewLanguage("INVALID", NewErrorPrefixMap("a", "b", "c", "d", "e"))) + + // Edge (the bug): re-adding the same language must overwrite, not no-op. + require.NoError(t, AddNewLanguage("xy", NewErrorPrefixMap( + "xy2-failed %s", "xy2-invalid %s", "xy2-missing %s", "xy2-required %s", "xy2-notfound %s", + ))) + + tmpl, err = GetTemplate("xy", string(FailedTo)) + require.NoError(t, err) + assert.Equal(t, "xy2-failed %s", tmpl) +} + +////// +// Catalog name validation (gte=3, consistent with the rest). +////// + +func TestFix_CatalogName_Validation(t *testing.T) { + // Happy path. + _, err := NewCatalog("myapp") + assert.NoError(t, err) + + // Edge: exactly 3 chars is now valid (boundary). + _, err = NewCatalog("abc") + assert.NoError(t, err) + + // Bad path: empty and too-short names are rejected. + _, err = NewCatalog("") + assert.ErrorIs(t, err, ErrCatalogInvalidName) + + _, err = NewCatalog("ab") + assert.ErrorIs(t, err, ErrCatalogInvalidName) +} + +////// +// prependOptions must not mutate the caller's slice (no append aliasing). +////// + +func TestFix_PrependOptions_NoAliasing(t *testing.T) { + var order []string + + mk := func(name string) Option { + return func(*CustomError) { order = append(order, name) } + } + + // Happy path: item is first, originals follow in order. + source := []Option{mk("b"), mk("c")} + result := prependOptions(source, mk("a")) + + ce := &CustomError{} + for _, o := range result { + o(ce) + } + assert.Equal(t, []string{"a", "b", "c"}, order) + + // Edge (the bug): even with spare capacity, the caller's slice is intact. + order = nil + src := make([]Option, 2, 8) + src[0] = mk("b") + src[1] = mk("c") + _ = prependOptions(src, mk("a")) + + for _, o := range src { + o(ce) + } + assert.Equal(t, []string{"b", "c"}, order, "caller's slice must be unchanged") + + // Edge: empty source. + order = nil + for _, o := range prependOptions(nil, mk("a")) { + o(ce) + } + assert.Equal(t, []string{"a"}, order) +} + +////// +// Is must not panic when comparing non-comparable wrapped errors. +////// + +type uncomparableError struct{ _ []int } + +func (uncomparableError) Error() string { return "uncomparable" } + +func TestFix_Is_NonComparableNoPanic(t *testing.T) { + // Bad path (the bug): comparing non-comparable error values must not panic. + cE := New("wrapper", WithError(uncomparableError{})).(*CustomError) + + assert.NotPanics(t, func() { + assert.False(t, errors.Is(cE, uncomparableError{})) + }) + + // Happy path: comparable wrapped errors still match correctly. + target := errors.New("target") + wrapper := New("wrapper", WithError(target)).(*CustomError) + assert.True(t, errors.Is(wrapper, target)) + assert.False(t, errors.Is(wrapper, errors.New("other"))) +} + +////// +// #20 - Wrap preserves the identity of every wrapped error. +////// + +func TestFix_Wrap_PreservesIdentity(t *testing.T) { + primary := New("primary", WithErrorCode("E1010")) + extra1 := errors.New("extra one") + extra2 := errors.New("extra two") + + wrapped := Wrap(primary, extra1, extra2) + + // Happy path: the message format is preserved. + assert.Equal(t, "E1010: primary. Wrapped Error(s): extra one. extra two", wrapped.Error()) + + // The fix: errors.Is matches the primary AND every extra error. + assert.ErrorIs(t, wrapped, primary) + assert.ErrorIs(t, wrapped, extra1) + assert.ErrorIs(t, wrapped, extra2) + + // errors.As can still reach the wrapped CustomError. + var cE *CustomError + assert.True(t, errors.As(wrapped, &cE)) + assert.Equal(t, "E1010", cE.Code) + + // Edge: no extra errors. + only := Wrap(primary) + assert.ErrorIs(t, only, primary) + + // Edge: nil errors are skipped (message and identity). + withNil := Wrap(primary, nil, extra1, nil) + assert.Equal(t, "E1010: primary. Wrapped Error(s): extra one", withNil.Error()) + assert.ErrorIs(t, withNil, extra1) +} + +////// +// #21 - Instance factory methods must not panic (nil type assertion) when an +// ignore option fires; they return nil instead. +////// + +func TestFix_FactoryMethods_IgnoreReturnsNilNoPanic(t *testing.T) { + base := Factory("create file") + + cases := []struct { + name string + call func() error + }{ + {"NewFailedToError", func() error { return base.NewFailedToError(WithIgnoreString("failed")) }}, + {"NewInvalidError", func() error { return base.NewInvalidError(WithIgnoreString("invalid")) }}, + {"NewMissingError", func() error { return base.NewMissingError(WithIgnoreString("missing")) }}, + {"NewRequiredError", func() error { return base.NewRequiredError(WithIgnoreString("required")) }}, + {"NewHTTPError", func() error { return base.NewHTTPError(http.StatusNotFound, WithIgnoreString("not found")) }}, + {"New", func() error { return base.New(WithIgnoreString("create")) }}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var got error + assert.NotPanics(t, func() { got = tc.call() }) + assert.Nil(t, got, "ignored error must be nil") + }) + } + + // Happy path: without an ignore match, a proper error is returned. + failed := base.NewFailedToError() + require.NotNil(t, failed) + assert.Contains(t, failed.Error(), "failed to create file") +} + +// Guard against accidental cross-test pollution of the shared comparator: the +// Set stringer must stay deterministic. +func TestFix_Sanity_SetStringDeterministic(t *testing.T) { + e := New("msg", WithTag("b", "a", "c")).(*CustomError) + assert.Equal(t, "a, b, c", strings.TrimSpace(e.Tags.String())) +} diff --git a/catalog.go b/catalog.go index ac8f03f..bee1388 100644 --- a/catalog.go +++ b/catalog.go @@ -5,8 +5,6 @@ import ( "regexp" "strings" "sync" - - "github.com/go-playground/validator/v10" ) ////// @@ -24,30 +22,12 @@ var ( // ErrErrorCodeInvalidCode is returned when an error code is invalid. ErrErrorCodeInvalidCode = NewInvalidError("error code. It requires typeOf, and subject", WithErrorCode("CE_ERR_INVALID_ERROR_CODE")) - // ErrorCodeRegex is a regular expression to validate error codes. It's - // designed to match four distinct patterns: - // - // 1. An optional number and letter followed by an underscore, "ERR_", a - // letter and a number, another underscore, another letter and a number, and - // an optional underscore followed by a number and a letter: - // Format: {optional_number_letter}_ERR_{number_letter}_{number_letter}_{optional_number_letter} - // Example: 1A_ERR_A1_B2 or 1A_ERR_A1_B2_3C - // - // 2. "ERR_" followed by a letter and a number, another underscore, another - // letter and a number, and an optional underscore followed by a number and - // a letter: - // Format: ERR_{number_letter}_{number_letter}_{optional_number_letter} - // Example: ERR_A1_B2 or ERR_A1_B2_3C - // - // 3. "E" followed by 1 to 8 digits: - // Format: E{1 to 8 digits} - // Example: E12345678 - // - // 4. At least one letter or number (any combination of uppercase and - // lowercase letters and digits): - // Format: {letters or digits, at least one character} - // Example: AbCd123. - ErrorCodeRegex = regexp.MustCompile(`^(\d?[A-Za-z]_)?ERR_[A-Za-z]\d_[A-Za-z]\d(_\d[A-Za-z])?$|^ERR_[A-Za-z]\d_[A-Za-z]\d(_\d[A-Za-z])?$|^E\d{1,8}$|[A-Za-z\d]+`) + // ErrorCodeRegex validates error codes. A valid error code is a non-empty + // string consisting solely of ASCII letters, digits, and underscores, for + // example: "E1010", "INVALID_REQUEST", "ERR_A1_B2", "AbCd123". Spaces and + // punctuation are rejected. The pattern is fully anchored so the entire + // string must match (no partial matches). + ErrorCodeRegex = regexp.MustCompile(`^[A-Za-z0-9_]+$`) ) type ( @@ -65,7 +45,7 @@ type ( ErrorCodeErrorMap ErrorCodeErrorMap `json:"custom_errors"` // Name of the catalog, usually, the name of the application. - Name string `json:"name" validate:"required,gt=3"` + Name string `json:"name" validate:"required,gte=3"` } ) @@ -111,18 +91,30 @@ func (c *Catalog) MustSet(errorCode string, defaultMessage string, opts ...Optio } // Get returns a custom error from the catalog, if not found, returns an error. -func (c *Catalog) Get(errorCode string, _ ...Option) (*CustomError, error) { +// +// The returned `*CustomError` is a copy of the catalog entry, so callers can +// safely mutate it (or apply `opts`) without corrupting the shared catalog. +// Any `opts` provided are applied to the returned copy. +func (c *Catalog) Get(errorCode string, opts ...Option) (*CustomError, error) { errCode, err := NewErrorCode(errorCode) if err != nil { return nil, err } customErr, ok := c.ErrorCodeErrorMap.Load(errCode) - if ok { - return customErr.(*CustomError), nil + if !ok { + return nil, fmt.Errorf("%w. Code: %s", ErrCatalogErrorNotFound, errCode) + } + + // Return a copy so the caller cannot mutate the catalog's stored entry. + cE := Copy(customErr.(*CustomError), &CustomError{}) + + // Apply the caller-provided options to the copy. + for _, opt := range opts { + opt(cE) } - return nil, fmt.Errorf("%w. Code: %s", ErrCatalogErrorNotFound, errCode) + return cE, nil } // MustGet returns a custom error from the catalog, if not found, panics. @@ -158,7 +150,7 @@ func NewCatalog(name string) (*Catalog, error) { Name: name, } - if err := validator.New().Struct(c); err != nil { + if err := validate.Struct(c); err != nil { return nil, ErrCatalogInvalidName } diff --git a/customerror.go b/customerror.go index 2f0bde6..66f8a9c 100644 --- a/customerror.go +++ b/customerror.go @@ -9,7 +9,7 @@ import ( "fmt" "log" "net/http" - "os" + "reflect" "strings" "sync" @@ -17,6 +17,27 @@ import ( "github.com/go-playground/validator/v10" ) +////// +// Consts, vars, and types. +////// + +// validate is the shared, reusable validator instance. The validator caches +// struct metadata internally, so a single instance is meant to be reused +// across calls (and is safe for concurrent use). +var validate = validator.New() + +// reservedJSONKeys are the top-level keys owned by the CustomError structure +// when marshaling to JSON. User-provided fields must not be allowed to clobber +// them. +var reservedJSONKeys = map[string]struct{}{ + "message": {}, + "code": {}, + "tags": {}, + "retryable": {}, + "retried": {}, + "statusCode": {}, +} + ////// // Helpers. ////// @@ -119,52 +140,53 @@ func Copy(src, target *CustomError) *CustomError { return target } -// Process fields and add them to the error message. +// Process fields and add them to the error message. If `fields` is nil or +// empty, the message is returned unchanged (no dangling ". Fields:"). func processFields( errMsg string, fields *sync.Map, ) string { - if fields != nil { - errMsg = fmt.Sprintf("%s. Fields:", errMsg) - - fields.Range(func(k, v interface{}) bool { - errMsg = fmt.Sprintf("%s %s=%v,", errMsg, k, v) - - return true - }) - - errMsg = strings.TrimSuffix(errMsg, ",") + if fields == nil { + return errMsg } - return errMsg -} + var b strings.Builder -// mapToSyncMap converts a map to a sync.Map. -func mapToSyncMap(m map[string]interface{}) *sync.Map { - sm := &sync.Map{} + fields.Range(func(k, v interface{}) bool { + fmt.Fprintf(&b, " %v=%v,", k, v) - for k, v := range m { - sm.Store(k, v) + return true + }) + + if b.Len() == 0 { + return errMsg } - return sm + return fmt.Sprintf("%s. Fields:%s", errMsg, strings.TrimSuffix(b.String(), ",")) } -// syncMapToMap converts a sync.Map to a map. -func syncMapToMap(sm *sync.Map) map[string]interface{} { - m := make(map[string]interface{}) +// addUserFieldsToJSON copies user-provided fields into the JSON map `temp`, +// skipping empty keys, nil values, and any key reserved by the CustomError +// structure (so user fields can never clobber structural JSON keys). +func addUserFieldsToJSON(temp map[string]interface{}, fields *sync.Map) { + if fields == nil { + return + } - if sm != nil { - sm.Range(func(k, v interface{}) bool { - if str, ok := k.(string); ok { - m[str] = v - } + fields.Range(func(k, v interface{}) bool { + key, ok := k.(string) + if !ok || key == "" || v == nil { + return true + } + if _, reserved := reservedJSONKeys[key]; reserved { return true - }) - } + } + + temp[key] = v - return m + return true + }) } // Set is a wrapper around the treeset.Set, providing a collection @@ -206,10 +228,6 @@ type CustomError struct { // Message in different languages. LanguageMessageMap LanguageMessageMap `json:"languageMessageMap"` - // LanguageErrorTypeMap is a map of language prefixes to templates such - // as "missing %s", "%s required", "%s invalid", etc. - LanguageErrorTypeMap LanguageErrorMap `json:"languageErrorTypeMap"` - // Retryable indicates if the error is retryable. Retryable bool `json:"retryable"` @@ -250,7 +268,7 @@ func (cE *CustomError) Error() string { errMsg = fmt.Errorf("%s. Original Error: %w", errMsg, cE.Err).Error() } - if cE.Tags != nil { + if cE.Tags != nil && !cE.Tags.Empty() { errMsg = fmt.Sprintf("%s. Tags: %s", errMsg, cE.Tags.String()) } @@ -270,6 +288,17 @@ func (cE *CustomError) Error() string { // //nolint:errorlint func (cE *CustomError) Is(err error) bool { + // Preserve the original nil semantics. + if cE.Err == nil || err == nil { + return cE.Err == err + } + + // Guard against panics when comparing non-comparable error values (e.g. an + // error whose dynamic type contains a slice or a map). + if !reflect.TypeOf(cE.Err).Comparable() || !reflect.TypeOf(err).Comparable() { + return false + } + return cE.Err == err } @@ -308,19 +337,8 @@ func (cE *CustomError) MarshalJSON() ([]byte, error) { temp["retried"] = cE.Retried } - if cE.Fields != nil { - // Convert the sync.Map to a regular map so that we can iterate over its keys. - fields := syncMapToMap(cE.Fields) - - // Populate the fields of the temporary map. - if len(fields) > 0 { - for k, v := range fields { - if k != "" && v != nil { - temp[k] = v - } - } - } - } + // Add user fields, skipping any that would clobber the structural keys. + addUserFieldsToJSON(temp, cE.Fields) if cE.StatusCode > 0 { temp["statusCode"] = cE.StatusCode @@ -369,7 +387,7 @@ func (cE *CustomError) APIError() string { errMsg = fmt.Errorf("%s. Original Error: %w", errMsg, cE.Err).Error() } - if cE.Tags != nil { + if cE.Tags != nil && !cE.Tags.Empty() { errMsg = fmt.Sprintf("%s. Tags: %s", errMsg, cE.Tags.String()) } @@ -401,13 +419,16 @@ func (cE *CustomError) FormatError(errorType string, opts ...Option) *CustomErro // Get the template by the language. template, err := GetTemplate(string(finalCE.language), errorType) if err != nil { - // Get the template by the root language. - template2, err := GetTemplate(finalCE.language.GetRoot(), errorType) - if err != nil { - panic(err) + // Fall back to the root language (e.g. "pt-BR" -> "pt"). + rootTemplate, rootErr := GetTemplate(finalCE.language.GetRoot(), errorType) + if rootErr != nil { + // No template available for this language/error type. Degrade + // gracefully by returning the (unprefixed) message instead of + // panicking. + return finalCE } - template = template2 + template = rootTemplate } finalCE.Message = fmt.Sprintf(template, finalCE.Message) @@ -431,9 +452,24 @@ func (cE *CustomError) FormatError(errorType string, opts ...Option) *CustomErro // NOTE: Status code can be redefined, call `SetStatusCode`. func (cE *CustomError) NewFailedToError(opts ...Option) error { finalCE := cE.FormatError(string(FailedTo), opts...) + if finalCE == nil { + return nil + } + + // Honor WithIgnoreFunc/WithIgnoreString consistently. + if finalCE.ignore { + return nil + } if finalCE.language == "" { - finalCE = Copy(NewFailedToError(finalCE.Message, opts...).(*CustomError), finalCE) + // The prefixed message may itself trigger an ignore option, in which + // case the inner constructor returns nil; guard the type assertion. + inner := NewFailedToError(finalCE.Message, opts...) + if inner == nil { + return nil + } + + finalCE = Copy(inner.(*CustomError), finalCE) return finalCE } @@ -450,9 +486,24 @@ func (cE *CustomError) NewFailedToError(opts ...Option) error { // NOTE: Status code can be redefined, call `SetStatusCode`. func (cE *CustomError) NewInvalidError(opts ...Option) error { finalCE := cE.FormatError(string(Invalid), opts...) + if finalCE == nil { + return nil + } + + // Honor WithIgnoreFunc/WithIgnoreString consistently. + if finalCE.ignore { + return nil + } if finalCE.language == "" { - finalCE = Copy(NewInvalidError(finalCE.Message, opts...).(*CustomError), finalCE) + // The prefixed message may itself trigger an ignore option, in which + // case the inner constructor returns nil; guard the type assertion. + inner := NewInvalidError(finalCE.Message, opts...) + if inner == nil { + return nil + } + + finalCE = Copy(inner.(*CustomError), finalCE) return finalCE } @@ -469,9 +520,24 @@ func (cE *CustomError) NewInvalidError(opts ...Option) error { // NOTE: Status code can be redefined, call `SetStatusCode`. func (cE *CustomError) NewMissingError(opts ...Option) error { finalCE := cE.FormatError(Missing.String(), opts...) + if finalCE == nil { + return nil + } + + // Honor WithIgnoreFunc/WithIgnoreString consistently. + if finalCE.ignore { + return nil + } if finalCE.language == "" { - finalCE = Copy(NewMissingError(finalCE.Message, opts...).(*CustomError), finalCE) + // The prefixed message may itself trigger an ignore option, in which + // case the inner constructor returns nil; guard the type assertion. + inner := NewMissingError(finalCE.Message, opts...) + if inner == nil { + return nil + } + + finalCE = Copy(inner.(*CustomError), finalCE) return finalCE } @@ -488,9 +554,24 @@ func (cE *CustomError) NewMissingError(opts ...Option) error { // NOTE: Status code can be redefined, call `SetStatusCode`. func (cE *CustomError) NewRequiredError(opts ...Option) error { finalCE := cE.FormatError(Required.String(), opts...) + if finalCE == nil { + return nil + } + + // Honor WithIgnoreFunc/WithIgnoreString consistently. + if finalCE.ignore { + return nil + } if finalCE.language == "" { - finalCE = Copy(NewRequiredError(finalCE.Message, opts...).(*CustomError), finalCE) + // The prefixed message may itself trigger an ignore option, in which + // case the inner constructor returns nil; guard the type assertion. + inner := NewRequiredError(finalCE.Message, opts...) + if inner == nil { + return nil + } + + finalCE = Copy(inner.(*CustomError), finalCE) return finalCE } @@ -509,15 +590,23 @@ func (cE *CustomError) NewHTTPError(statusCode int, opts ...Option) error { return nil } - if cE.StatusCode == 0 { - cE.StatusCode = statusCode - } + // Work on a copy so the receiver (often a reusable factory/catalog error) + // is never mutated. + finalCE := Copy(cE, &CustomError{}) - finalCE := &CustomError{} + // Use the receiver's status code if it was explicitly set, otherwise fall + // back to the provided one. + if finalCE.StatusCode == 0 { + finalCE.StatusCode = statusCode + } - finalCE = Copy(cE, finalCE) + httpErr := NewHTTPError(finalCE.StatusCode, opts...) + if httpErr == nil { + // Ignored via WithIgnoreFunc/WithIgnoreString. + return nil + } - httpCE := NewHTTPError(finalCE.StatusCode, opts...).(*CustomError) + httpCE := httpErr.(*CustomError) finalErrorMessage := httpCE.Message @@ -526,6 +615,10 @@ func (cE *CustomError) NewHTTPError(statusCode int, opts ...Option) error { opt(finalCE) } + if finalCE.ignore { + return nil + } + finalCE.Message = finalErrorMessage finalCE = Copy(httpCE, finalCE) @@ -549,7 +642,18 @@ func (cE *CustomError) New(opts ...Option) error { opt(finalCE) } - finalCE = Copy(New(finalCE.Message, opts...).(*CustomError), finalCE) + // Honor WithIgnoreFunc/WithIgnoreString and guard the type assertion in + // case the inner constructor ignored the error (returned nil). + if finalCE.ignore { + return nil + } + + inner := New(finalCE.Message, opts...) + if inner == nil { + return nil + } + + finalCE = Copy(inner.(*CustomError), finalCE) return finalCE } @@ -558,17 +662,57 @@ func (cE *CustomError) New(opts ...Option) error { // Exported functionalities. ////// -// Wrap `customError` around `errors`. +// wrappedError is the error type returned by Wrap. It preserves the identity of +// every wrapped error (so errors.Is/errors.As work for all of them) while +// keeping a human-readable, stable message format. +type wrappedError struct { + // customError is the primary error being wrapped. + customError error + + // errs are the additional errors wrapped around customError. + errs []error +} + +// Error implements the error interface, preserving the message format: +// ". Wrapped Error(s): . ...". +func (w *wrappedError) Error() string { + errMsgs := make([]string, 0, len(w.errs)) + + for _, err := range w.errs { + errMsgs = append(errMsgs, err.Error()) + } + + return fmt.Sprintf("%s. Wrapped Error(s): %s", w.customError.Error(), strings.Join(errMsgs, ". ")) +} + +// Unwrap returns all wrapped errors, enabling errors.Is/errors.As to match any +// of them (Go 1.20+ multi-error unwrapping). +func (w *wrappedError) Unwrap() []error { + all := make([]error, 0, len(w.errs)+1) + + all = append(all, w.customError) + all = append(all, w.errs...) + + return all +} + +// Wrap wraps `customError` together with the additional `errors`. The returned +// error preserves the identity of every wrapped error, so errors.Is and +// errors.As match against `customError` and each of `errors`. Nil errors are +// ignored. func Wrap(customError error, errors ...error) error { - errMsgs := []string{} + nonNil := make([]error, 0, len(errors)) for _, err := range errors { if err != nil { - errMsgs = append(errMsgs, err.Error()) + nonNil = append(nonNil, err) } } - return fmt.Errorf("%w. Wrapped Error(s): %s", customError, strings.Join(errMsgs, ". ")) + return &wrappedError{ + customError: customError, + errs: nonNil, + } } // NewChildError creates a new `CustomError` with the same fields and tags of @@ -620,7 +764,10 @@ func newInternal(opts ...Option) *CustomError { // New creates a new validated custom error returning it as en `error`. // -//nolint:revive +// NOTE: Creating an error with invalid attributes (for example, an empty +// message) is a programming error. In that case New panics (recoverable) +// rather than terminating the host process - a library must never call +// os.Exit on its caller's behalf. func New(message string, opts ...Option) error { cE := newInternal(prependOptions(opts, WithMessage(message))...) @@ -629,18 +776,8 @@ func New(message string, opts ...Option) error { return nil } - if cE == nil { - log.Panicln("Failed to create custom error.") - } - - if err := validator.New().Struct(cE); err != nil { - if os.Getenv("CUSTOMERROR_ENVIRONMENT") == "testing" { - log.Panicf("Invalid custom error. %s\n", err) - } - - log.Fatalf("Invalid custom error. %s\n", err) - - return nil + if err := validate.Struct(cE); err != nil { + log.Panicf("Invalid custom error. %s\n", err) } return cE @@ -661,10 +798,6 @@ func Factory(message string, opts ...Option) *CustomError { return nil } - if cE == nil { - log.Panicln("Failed to create custom error.") - } - return cE } @@ -686,15 +819,22 @@ func To(err error) (*CustomError, bool) { return cE, true } -// From modifies the error with the given options, if `err` isn't a custom error -// it then returns a new custom error with the given options. +// From returns a copy of `err` with the given options applied. If `err` is a +// `CustomError`, the original is left untouched (not mutated) and a modified +// copy is returned - this makes it safe to use with shared sentinel errors. If +// `err` isn't a custom error, a new custom error wrapping it (with the given +// options) is returned. func From(err error, opts ...Option) error { if cE, ok := To(err); ok { + // Operate on a copy so the original error (which may be a shared + // sentinel) is never mutated. + finalCE := Copy(cE, &CustomError{}) + for _, opt := range opts { - opt(cE) + opt(finalCE) } - return cE + return finalCE } // WithError properly deals with Golang errors (unwrapping, etc). diff --git a/go.mod b/go.mod index 588af8f..e85ace3 100644 --- a/go.mod +++ b/go.mod @@ -1,24 +1,23 @@ -module github.com/thalesfsp/customerror +module github.com/thalesfsp/customerror/v2 -go 1.19 +go 1.25.0 require ( github.com/eapache/go-resiliency v1.7.0 github.com/emirpasic/gods v1.18.1 - github.com/go-playground/validator/v10 v10.22.1 - github.com/stretchr/testify v1.8.4 + github.com/go-playground/validator/v10 v10.30.3 + github.com/stretchr/testify v1.11.1 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect - github.com/gabriel-vasile/mimetype v1.4.5 // indirect + github.com/gabriel-vasile/mimetype v1.4.13 // indirect github.com/go-playground/locales v0.14.1 // indirect github.com/go-playground/universal-translator v0.18.1 // indirect github.com/leodido/go-urn v1.4.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/crypto v0.28.0 // indirect - golang.org/x/net v0.30.0 // indirect - golang.org/x/sys v0.26.0 // indirect - golang.org/x/text v0.19.0 // indirect + golang.org/x/crypto v0.52.0 // indirect + golang.org/x/sys v0.45.0 // indirect + golang.org/x/text v0.37.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 3cfa583..0ed917f 100644 --- a/go.sum +++ b/go.sum @@ -4,29 +4,28 @@ github.com/eapache/go-resiliency v1.7.0 h1:n3NRTnBn5N0Cbi/IeOHuQn9s2UwVUH7Ga0ZWc github.com/eapache/go-resiliency v1.7.0/go.mod h1:5yPzW0MIvSe0JDsv0v+DvcjEv2FyD6iZYSs1ZI+iQho= github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc= github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FMNAnJvWQ= -github.com/gabriel-vasile/mimetype v1.4.5 h1:J7wGKdGu33ocBOhGy0z653k/lFKLFDPJMG8Gql0kxn4= -github.com/gabriel-vasile/mimetype v1.4.5/go.mod h1:ibHel+/kbxn9x2407k1izTA1S81ku1z/DlgOW2QE0M4= +github.com/gabriel-vasile/mimetype v1.4.13 h1:46nXokslUBsAJE/wMsp5gtO500a4F3Nkz9Ufpk2AcUM= +github.com/gabriel-vasile/mimetype v1.4.13/go.mod h1:d+9Oxyo1wTzWdyVUPMmXFvp4F9tea18J8ufA774AB3s= github.com/go-playground/assert/v2 v2.2.0 h1:JvknZsQTYeFEAhQwI4qEt9cyV5ONwRHC+lYKSsYSR8s= +github.com/go-playground/assert/v2 v2.2.0/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4= github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA= github.com/go-playground/locales v0.14.1/go.mod h1:hxrqLVvrK65+Rwrd5Fc6F2O76J/NuW9t0sjnWqG1slY= github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY= github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY= -github.com/go-playground/validator/v10 v10.22.1 h1:40JcKH+bBNGFczGuoBYgX4I6m/i27HYW8P9FDk5PbgA= -github.com/go-playground/validator/v10 v10.22.1/go.mod h1:dbuPbCMFw/DrkbEynArYaCwl3amGuJotoKCe95atGMM= +github.com/go-playground/validator/v10 v10.30.3 h1:4MU6YkEwx7GbcPJOZxrtbu+QfF3pJLJuaYTeAH0DYy8= +github.com/go-playground/validator/v10 v10.30.3/go.mod h1:4Axh7oCNGcoGkqLoE4YWt6n20mcEIsPRlB7vPk3lpyc= github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ= github.com/leodido/go-urn v1.4.0/go.mod h1:bvxc+MVxLKB4z00jd1z+Dvzr47oO32F/QSNjSBOlFxI= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= -github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -golang.org/x/crypto v0.28.0 h1:GBDwsMXVQi34v5CCYUm2jkJvu4cbtru2U4TN2PSyQnw= -golang.org/x/crypto v0.28.0/go.mod h1:rmgy+3RHxRZMyY0jjAJShp2zgEdOqj2AO7U0pYmeQ7U= -golang.org/x/net v0.30.0 h1:AcW1SDZMkb8IpzCdQUaIq2sP4sZ4zw+55h6ynffypl4= -golang.org/x/net v0.30.0/go.mod h1:2wGyMJ5iFasEhkwi13ChkO/t1ECNC4X4eBKkVFyYFlU= -golang.org/x/sys v0.26.0 h1:KHjCJyddX0LoSTb3J+vWpupP9p0oznkqVk/IfjymZbo= -golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/text v0.19.0 h1:kTxAhCbGbxhK0IwgSKiMO5awPoDQ0RpfiVYBfK860YM= -golang.org/x/text v0.19.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +golang.org/x/crypto v0.52.0 h1:RMs7fP2rXdep0CftQlK8Uf+kibLm7qkCcradZWYz988= +golang.org/x/crypto v0.52.0/go.mod h1:1QgfPxDqh0T2M/elOJtp9RvuR95kVjir0e6/BvEmGbc= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= +golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/language.go b/language.go index 75b1bca..a88d404 100644 --- a/language.go +++ b/language.go @@ -10,8 +10,9 @@ import ( // Consts, vars, and types. ////// +// Built-in language codes (ISO 639-1) supported out of the box. const ( - Chinese Language = "ch" + Chinese Language = "zh" English Language = "en" French Language = "fr" German Language = "de" @@ -43,8 +44,10 @@ var ( } // LanguageRegex is a regular expression to validate language codes based on - // ISO 639-1 and ISO 3166-1 alpha-2. - LanguageRegex = regexp.MustCompile("^[a-z]{2}(-[A-Z]{2})?$|default") + // ISO 639-1 and ISO 3166-1 alpha-2. The special value "default" is also + // accepted. Every alternative is fully anchored, so partial matches such as + // "mydefault" are correctly rejected. + LanguageRegex = regexp.MustCompile("^[a-z]{2}(-[A-Z]{2})?$|^default$") ) type ( diff --git a/languageprefixtemplate.go b/languageprefixtemplate.go index 87b1bff..0b88010 100644 --- a/languageprefixtemplate.go +++ b/languageprefixtemplate.go @@ -62,7 +62,7 @@ func (e ErrorType) String() string { // It initializes the map with built-in languages and their respective error // templates. // -//nolint:gosmopolitan +//nolint:gosmopolitan,misspell func GetLanguageErrorMap() LanguageErrorMap { once.Do(func() { languageErrorTypeMap := &sync.Map{} @@ -144,7 +144,7 @@ func GetLanguageErrorMap() LanguageErrorMap { // Initialize error templates for Italian itErrorTypePrefixTemplateMap := &sync.Map{} - itErrorTypePrefixTemplateMap.Store(FailedTo, "impossible %s") + itErrorTypePrefixTemplateMap.Store(FailedTo, "impossibile %s") itErrorTypePrefixTemplateMap.Store(Invalid, "%s non valido") itErrorTypePrefixTemplateMap.Store(Missing, "mancante %s") itErrorTypePrefixTemplateMap.Store(Required, "%s richiesto") @@ -210,8 +210,9 @@ func GetTemplate(language, errorType string) (string, error) { return template.(string), nil } -// AddNewLanguage allows setting a new language to the error handling system, -// seeting the built-in error templates for each error type. +// AddNewLanguage registers (or updates) a language in the error handling +// system, setting the built-in error templates for each error type. If the +// language already exists, its templates are overwritten. func AddNewLanguage( language string, errorTypePrefixTemplateMap ErrorPrefixMap, @@ -221,7 +222,7 @@ func AddNewLanguage( return err } - GetLanguageErrorMap().LoadOrStore(l, errorTypePrefixTemplateMap) + GetLanguageErrorMap().Store(l, errorTypePrefixTemplateMap) return nil } diff --git a/options.go b/options.go index baa9691..147ca33 100644 --- a/options.go +++ b/options.go @@ -28,14 +28,17 @@ import ( type Option func(s *CustomError) // Prepend options. +// +// It returns a brand-new slice with `item` first, followed by `source`. A +// fresh slice is always allocated so the caller's backing array is never +// mutated (avoids append/copy aliasing bugs). func prependOptions(source []Option, item Option) []Option { - source = append(source, nil) - - copy(source[1:], source) + result := make([]Option, 0, len(source)+1) - source[0] = item + result = append(result, item) + result = append(result, source...) - return source + return result } ////// @@ -120,14 +123,18 @@ func WithTag(tag ...string) Option { } } -// WithFields allows to set fields for the error. +// WithFields allows to set fields for the error. Existing fields are preserved; +// keys present in `fields` are added or overwritten (consistent with +// `WithField`). func WithFields(fields map[string]interface{}) Option { return func(cE *CustomError) { if cE.Fields == nil { cE.Fields = &sync.Map{} } - cE.Fields = mapToSyncMap(fields) + for k, v := range fields { + cE.Fields.Store(k, v) + } } } @@ -151,7 +158,9 @@ func WithLanguage(lang string) Option { return func(cE *CustomError) { l, err := NewLanguage(lang) if err != nil { - panic(err) + // Invalid language code: ignore it and keep the default message + // (as documented) instead of panicking. + return } if cE.LanguageMessageMap == nil { @@ -184,15 +193,19 @@ func WithLanguage(lang string) Option { // languages, combinations, and their translations. // // SEE: `i18n.md` file for more information. +// +// NOTE: If `lang` is not a valid language code, the translation is ignored +// (not stored) instead of panicking. func WithTranslation(lang, message string) Option { return func(cE *CustomError) { - if cE.LanguageMessageMap == nil { - cE.LanguageMessageMap = &sync.Map{} - } - l, err := NewLanguage(lang) if err != nil { - panic(err) + // Invalid language code: ignore it instead of panicking. + return + } + + if cE.LanguageMessageMap == nil { + cE.LanguageMessageMap = &sync.Map{} } cE.LanguageMessageMap.Store(l, message)