diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index b312a43..d7f714a 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -9,7 +9,7 @@ on: env: SSH_AUTH_SOCK: /tmp/ssh_agent.sock - GOLANGCI_LINT_VERSION: v1.57.2 + GOLANGCI_LINT_VERSION: v2.6.2 jobs: lint: @@ -32,7 +32,7 @@ jobs: run: go mod tidy && git diff --exit-code - name: golangci-lint - uses: golangci/golangci-lint-action@v4 + uses: golangci/golangci-lint-action@v8 with: version: ${{ env.GOLANGCI_LINT_VERSION }} skip-cache: true # cache/restore is done by actions/setup-go@v3 step diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index cb7b599..9878ed2 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -13,7 +13,8 @@ jobs: strategy: matrix: go-version: - - 1.21.x + - 1.24.x + - 1.25.x os: [ ubuntu-latest ] runs-on: ${{ matrix.os }} steps: @@ -21,7 +22,7 @@ jobs: uses: actions/checkout@master - name: Set up Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v5 with: go-version: ${{ matrix.go-version }} cache: true # caching and restoring go modules and build outputs diff --git a/.golangci.yml b/.golangci.yml index 0a3335b..eace587 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,125 +1,85 @@ +version: "2" +run: + tests: true linters: - # Please, do not use `enable-all`: it's deprecated and will be removed soon. - # Inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint. - # Full list of linters - https://golangci-lint.run/usage/linters - disable-all: true + default: none enable: - - bodyclose # https://github.com/timakin/bodyclose - - gomodguard - - errcheck # Mandatory. Do not disable. + - bodyclose + - errcheck + - errorlint # Wrapping - gocritic - - goimports + - gomodguard - gosec - - gosimple - govet + - ineffassign + - makezero + - misspell + - nestif # Deep nesting avoidance - noctx - nolintlint - - ineffassign # Mandatory. Do not disable. - - staticcheck # Mandatory. Do not disable. - - stylecheck - - typecheck + - prealloc + - staticcheck + - testifylint # Helps catch expected/got reversals - unused - -# Other linters: -# - dogsled -# - dupl -# - exportloopref -# - exhaustive # e.g. missing cases in switch of type -# - funlen -# - gochecknoinits -# - gocognit -# - goconst -# - gocyclo -# - goerr113 -# - gofmt -# - goprintffuncname -# - lll -# - misspell -# - nakedret -# - nlreturn -# - prealloc -# - revive -# - rowserrcheck -# - stylecheck -# - unconvert -# - unparam - -linters-settings: - gocritic: - enabled-tags: - - diagnostic - - experimental - - opinionated - - performance - - style - disabled-checks: - - dupImport # https://github.com/go-critic/go-critic/issues/845 - - whyNoLint # checked by nolintlint linter - - hugeParam # TODO(vtopc): configure(80 bytes is probably not so much) and enable. - - rangeValCopy # TODO(vtopc): configure(disable for tests) and enable. - - appendAssign - - commentedOutCode - - errcheck: - # List of functions to exclude from checking, where each entry is a single function to exclude. - # See https://github.com/kisielk/errcheck#excluding-functions for details. - exclude-functions: - - (io.Closer).Close - - (io.ReadCloser).Close - - govet: - enable-all: true - disable: - - shadow - - fieldalignment - - gomodguard: - blocked: - # List of blocked modules. - # Default: [] - modules: - - github.com/golang/protobuf: - recommendations: - - google.golang.org/protobuf - reason: "see https://developers.google.com/protocol-buffers/docs/reference/go/faq#modules" - - github.com/pkg/errors: - recommendations: - - errors - - github.com/mailgun/errors - reason: "Deprecated" - - stylecheck: - # https://staticcheck.io/docs/options#checks - checks: ["all"] - + settings: + errcheck: + exclude-functions: + - (io.Closer).Close + - (io.ReadCloser).Close + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + testifylint: + disable: + - error-nil # prefer explicit checks instead for error contract adherence + - error-is-as # prefer explicit checks instead for error contract adherence + gomodguard: + blocked: + modules: + - github.com/golang/protobuf: + recommendations: + - google.golang.org/protobuf + reason: see https://developers.google.com/protocol-buffers/docs/reference/go/faq#modules + - github.com/pkg/errors: + recommendations: + - errors + - github.com/mailgun/errors + reason: Deprecated + govet: + disable: + - fieldalignment + enable-all: true + staticcheck: + checks: + - all + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + rules: + - linters: + - errcheck + path: '(.+)_test\.go' + paths: + - third_party$ + - builtin$ + - examples$ issues: - # Maximum issues count per one linter. Set to 0 to disable. Default is 50. max-issues-per-linter: 0 - - # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. max-same-issues: 50 - - exclude: - # Some packages have deprecated fields which continue to be useful - - SA1019 - - exclude-rules: - # Exclude some rules from tests. - - path: '_test\.go$' - linters: - - gosec - - noctx - - path: '_test\.go$' - text: "unnamedResult:" - - path: '.*mxresolv.*' - linters: - - gosec - - -run: - # include test files or not, default is true - tests: true - - # Timeout for analysis, e.g. 30s, 5m. - # Default: 1m - timeout: 5m \ No newline at end of file +formatters: + enable: + - goimports + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/Makefile b/Makefile index f735896..e902cb6 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ GOLANGCI_LINT = $(GOPATH)/bin/golangci-lint -GOLANGCI_LINT_VERSION = v1.57.2 +GOLANGCI_LINT_VERSION = v2.6.2 .PHONY: lint lint: $(GOLANGCI_LINT) @@ -10,4 +10,4 @@ $(GOLANGCI_LINT): .PHONY: test test: - go test -p 1 ./... -short -race -timeout 1m -count=1 + go test -short -race -timeout 1m ./... diff --git a/errors_test.go b/errors_test.go index 7da4dcd..55b8bce 100644 --- a/errors_test.go +++ b/errors_test.go @@ -1,13 +1,57 @@ package errors_test import ( + "fmt" "testing" + stderr "errors" + "github.com/mailgun/errors" "github.com/mailgun/errors/callstack" "github.com/stretchr/testify/assert" ) +// Ensure errors.Is remains stdlib compliant +func TestIs(t *testing.T) { + err := errors.New("bottom") + top := fmt.Errorf("top: %w", err) + + assert.True(t, stderr.Is(top, err)) + assert.True(t, errors.Is(top, err)) +} + +// Ensure errors.As remains stdlib compliant +func TestAs(t *testing.T) { + err := &ErrTest{Msg: "bottom"} + top := fmt.Errorf("top: %w", err) + + var exp *ErrTest + assert.True(t, stderr.As(top, &exp)) + assert.True(t, errors.As(top, &exp)) +} + +func TestLast(t *testing.T) { + err := errors.New("bottom") + err = errors.Wrap(err, "last") + err = errors.Wrap(err, "second") + err = errors.Wrap(err, "first") + err = errors.Errorf("wrapped: %w", err) + + // errors.As() returns the "first" error in the chain with a stack trace + var first callstack.HasStackTrace + assert.True(t, errors.As(err, &first)) + assert.Equal(t, "first: second: last: bottom", first.(error).Error()) + + // errors.Last() returns the last error in the chain with a stack trace + var last callstack.HasStackTrace + assert.True(t, errors.Last(err, &last)) + assert.Equal(t, "last: bottom", last.(error).Error()) + + // If no stack trace is found, then should not set target and should return false + assert.False(t, errors.Last(errors.New("no stack"), &last)) + assert.Equal(t, "last: bottom", last.(error).Error()) +} + type ErrTest struct { Msg string } @@ -39,24 +83,61 @@ func (e *ErrHasFields) HasFields() map[string]any { return e.F } -func TestLast(t *testing.T) { - err := errors.New("bottom") - err = errors.Wrap(err, "last") - err = errors.Wrap(err, "second") - err = errors.Wrap(err, "first") - err = errors.Errorf("wrapped: %w", err) +// Benchmarks for comparison purposes - // errors.As() returns the "first" error in the chain with a stack trace - var first callstack.HasStackTrace - assert.True(t, errors.As(err, &first)) - assert.Equal(t, "first: second: last: bottom", first.(error).Error()) +/* +go test -bench=. -benchmem -count=5 -run="^$" ./... +goos: darwin +goarch: arm64 +pkg: github.com/mailgun/errors +cpu: Apple M3 Pro +BenchmarkErrors-12 4825278 221.8 ns/op 328 B/op 3 allocs/op +BenchmarkErrors-12 5548051 216.4 ns/op 328 B/op 3 allocs/op +BenchmarkErrors-12 5548090 215.5 ns/op 328 B/op 3 allocs/op +BenchmarkErrors-12 5548306 215.7 ns/op 328 B/op 3 allocs/op +BenchmarkErrors-12 5557860 215.8 ns/op 328 B/op 3 allocs/op +*/ +func BenchmarkErrorsWrap(b *testing.B) { + base := errors.New("init") + for b.Loop() { + errors.Wrap(base, "loop") + } +} - // errors.Last() returns the last error in the chain with a stack trace - var last callstack.HasStackTrace - assert.True(t, errors.Last(err, &last)) - assert.Equal(t, "last: bottom", last.(error).Error()) +/* +go test -bench=. -benchmem -count=5 -run="^$" ./... +goos: darwin +goarch: arm64 +pkg: github.com/mailgun/errors +cpu: Apple M3 Pro +BenchmarkErrorsWrapf-12 4733302 252.8 ns/op 336 B/op 4 allocs/op +BenchmarkErrorsWrapf-12 4750388 252.1 ns/op 336 B/op 4 allocs/op +BenchmarkErrorsWrapf-12 4720851 251.7 ns/op 336 B/op 4 allocs/op +BenchmarkErrorsWrapf-12 4740823 252.3 ns/op 336 B/op 4 allocs/op +BenchmarkErrorsWrapf-12 4753670 254.1 ns/op 336 B/op 4 allocs/op +*/ +func BenchmarkErrorsWrapf(b *testing.B) { + base := errors.New("init") + for b.Loop() { + errors.Wrapf(base, "loop %s", "two") + } +} - // If no stack trace is found, then should not set target and should return false - assert.False(t, errors.Last(errors.New("no stack"), &last)) - assert.Equal(t, "last: bottom", last.(error).Error()) +/* +go test -bench=. -benchmem -count=5 -run="^$" ./... +goos: darwin +goarch: arm64 +pkg: github.com/mailgun/errors +cpu: Apple M3 Pro +BenchmarkErrorsStack-12 5713897 210.4 ns/op 304 B/op 3 allocs/op +BenchmarkErrorsStack-12 5677599 210.1 ns/op 304 B/op 3 allocs/op +BenchmarkErrorsStack-12 5701461 210.1 ns/op 304 B/op 3 allocs/op +BenchmarkErrorsStack-12 5655940 210.1 ns/op 304 B/op 3 allocs/op +BenchmarkErrorsStack-12 5574022 210.8 ns/op 304 B/op 3 allocs/op +*/ +func BenchmarkErrorsStack(b *testing.B) { + base := errors.New("init") + for b.Loop() { + _ = errors.Stack(base) + } } diff --git a/fields.go b/fields.go index 7bf4358..6a43ebd 100644 --- a/fields.go +++ b/fields.go @@ -123,15 +123,15 @@ func (c *fields) Unwrap() error { } func (c *fields) Is(target error) bool { - _, ok := target.(*fields) - return ok + return errors.Is(c.wrapped, target) } // Cause returns the wrapped error which was the original // cause of the issue. We only support this because some code // depends on github.com/pkg/errors.Cause() returning the cause // of the error. -// Deprecated: use error.Is() or error.As() instead +// +// Deprecated: use errors.Is() or errors.As() instead func (c *fields) Cause() error { return c.wrapped } func (c *fields) Error() string { diff --git a/fields_test.go b/fields_test.go index 47bc3eb..2d36ca3 100644 --- a/fields_test.go +++ b/fields_test.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "os" - "strings" "testing" "github.com/mailgun/errors" @@ -27,7 +26,7 @@ func TestToMapToLogrusFindsLastStackTrace(t *testing.T) { t.Run("ToMap() finds the last stack in the chain", func(t *testing.T) { m := errors.ToMap(err) assert.NotNil(t, m) - assert.Equal(t, 21, m["excLineNum"]) + assert.Equal(t, 20, m["excLineNum"]) }) t.Run("ToLogrus() finds the last stack in the chain", func(t *testing.T) { @@ -38,7 +37,7 @@ func TestToMapToLogrusFindsLastStackTrace(t *testing.T) { logrus.WithFields(f).Info("test logrus fields") logrus.SetOutput(os.Stdout) fmt.Printf("%s\n", b.String()) - assert.Contains(t, b.String(), "excLineNum=21") + assert.Contains(t, b.String(), "excLineNum=20") }) } @@ -74,7 +73,7 @@ func TestFields(t *testing.T) { t.Run("Can use errors.As() from std `errors` package", func(t *testing.T) { myErr := &ErrTest{} assert.True(t, errors.As(wrap, &myErr)) - assert.Equal(t, myErr.Msg, "query error") + assert.Equal(t, "query error", myErr.Msg) }) t.Run("Extract as Logrus fields", func(t *testing.T) { @@ -100,7 +99,7 @@ func TestFields(t *testing.T) { assert.Equal(t, "message: query error", wrap.Error()) out := fmt.Sprintf("%+v", wrap) - assert.True(t, strings.Contains(out, `message: query error (key1=value1)`)) + assert.Contains(t, out, `message: query error (key1=value1)`) }) t.Run("ToLogrus() should extract the error with StackTrace() from the chain", func(t *testing.T) { @@ -136,7 +135,7 @@ func TestErrorf(t *testing.T) { err := errors.New("this is an error") wrap := errors.Fields{"key1": "value1", "key2": "value2"}.Wrap(err, "message") err = fmt.Errorf("wrapped: %w", wrap) - assert.Equal(t, fmt.Sprintf("%s", err), "wrapped: message: this is an error") + assert.Equal(t, "wrapped: message: this is an error", fmt.Sprintf("%s", err)) } func TestNestedFields(t *testing.T) { diff --git a/go.mod b/go.mod index d82fd74..25ddf4e 100644 --- a/go.mod +++ b/go.mod @@ -1,10 +1,10 @@ module github.com/mailgun/errors -go 1.21 +go 1.24 require ( - github.com/sirupsen/logrus v1.9.0 - github.com/stretchr/testify v1.9.0 + github.com/sirupsen/logrus v1.9.3 + github.com/stretchr/testify v1.11.1 ) require ( diff --git a/go.sum b/go.sum index 56b784f..fc22289 100644 --- a/go.sum +++ b/go.sum @@ -3,12 +3,12 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= 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/sirupsen/logrus v1.9.0 h1:trlNQbNUG3OdDrDil03MCb1H2o9nJ1x4/5LYw7byDE0= -github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= +github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= +github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= -github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +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/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 h1:h+EGohizhe9XlX18rfpa8k8RAc5XyaeamM+0VHRd4lc= golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/stack.go b/stack.go index 9d07098..0e9cfac 100644 --- a/stack.go +++ b/stack.go @@ -36,6 +36,7 @@ func (w *stack) Is(target error) bool { // cause of the issue. We only support this because some code // depends on github.com/pkg/errors.Cause() returning the cause // of the error. +// // Deprecated: use error.Is() or error.As() instead func (w *stack) Cause() error { return w.error } diff --git a/stack_test.go b/stack_test.go index 5ea84c7..3ba6445 100644 --- a/stack_test.go +++ b/stack_test.go @@ -21,7 +21,7 @@ func TestWrapWithFieldsAndStack(t *testing.T) { myErr := &ErrTest{} assert.True(t, errors.Is(err, &ErrTest{})) assert.True(t, errors.As(err, &myErr)) - assert.Equal(t, myErr.Msg, "error") + assert.Equal(t, "error", myErr.Msg) assert.Equal(t, "context: error", err.Error()) // Extract the stack from the error chain @@ -61,7 +61,7 @@ func TestStackWrapped(t *testing.T) { // Can use errors.As() from std `errors` package myErr := &ErrTest{} assert.True(t, errors.As(err, &myErr)) - assert.Equal(t, myErr.Msg, "query error") + assert.Equal(t, "query error", myErr.Msg) } func TestFormatStack(t *testing.T) { diff --git a/wrap.go b/wrap.go index d1d0715..f8c55ce 100644 --- a/wrap.go +++ b/wrap.go @@ -62,6 +62,7 @@ func (e *wrappedError) Is(target error) bool { // cause of the issue. We only support this because some code // depends on github.com/pkg/errors.Cause() returning the cause // of the error. +// // Deprecated: use error.Is() or error.As() instead func (e *wrappedError) Cause() error { return e.wrapped } diff --git a/wrap_test.go b/wrap_test.go index 65e9dbc..08fda2b 100644 --- a/wrap_test.go +++ b/wrap_test.go @@ -2,10 +2,10 @@ package errors_test import ( "bytes" + stderr "errors" "fmt" "io" "os" - "strings" "testing" "github.com/mailgun/errors" @@ -21,6 +21,10 @@ func TestWrap(t *testing.T) { wrapf := errors.Wrapf(err, "message: %d", 1) assert.NotNil(t, wrapf) + // Ensure consistency with stdlib + assert.True(t, stderr.Is(wrap, err)) + assert.True(t, stderr.Is(wrapf, err)) + t.Run("Wrap/Wrapf should return wrap the error", func(t *testing.T) { assert.Equal(t, "message: query error", wrap.Error()) assert.Equal(t, "message: 1: query error", wrapf.Error()) @@ -59,9 +63,9 @@ func TestWrap(t *testing.T) { t.Run("Can use errors.As() from std `errors` package", func(t *testing.T) { myErr := &ErrTest{} assert.True(t, errors.As(wrap, &myErr)) - assert.Equal(t, myErr.Msg, "query error") + assert.Equal(t, "query error", myErr.Msg) assert.True(t, errors.As(wrapf, &myErr)) - assert.Equal(t, myErr.Msg, "query error") + assert.Equal(t, "query error", myErr.Msg) }) t.Run("Extract as Logrus fields", func(t *testing.T) { @@ -89,7 +93,7 @@ func TestWrap(t *testing.T) { assert.Equal(t, "message: query error", wrap.Error()) out := fmt.Sprintf("%+v", wrap) - assert.True(t, strings.Contains(out, `message: query error`)) + assert.Contains(t, out, `message: query error`) }) t.Run("Wrap() should return nil, if error is nil", func(t *testing.T) {