Skip to content

Commit

Permalink
Removed global formatter for error messages (#13)
Browse files Browse the repository at this point in the history
* Removed global formatter for error messages as it is potentially dangerous. One can unexpectedly change error messages from other packages if they also use simplerr. If error checks are using the error message then it could break things

* updated lint rules

* update github lint action

---------

Co-authored-by: Calvin Lobo <calvin.lobo@hootsuite.com>
  • Loading branch information
lobocv and Calvin Lobo authored Jul 7, 2024
1 parent 663e3b2 commit b84ffca
Show file tree
Hide file tree
Showing 7 changed files with 663 additions and 45 deletions.
17 changes: 12 additions & 5 deletions .github/workflows/golangci-lint.yaml
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
name: golangci-lint

# Controls when the action will run. Triggers the workflow on push or pull requests
on: [push, pull_request]
on:
push:
branches:
- main
- master
pull_request:
permissions:
contents: read
jobs:
golangci:
name: lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: stable
- name: golangci-lint
uses: golangci/golangci-lint-action@v2
uses: golangci/golangci-lint-action@v6
with:
# Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
version: latest
version: v1.59
20 changes: 9 additions & 11 deletions .golangci.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# options for analysis running
run:
# timeout for analysis, e.g. 30s, 5m, default is 1m
deadline: 2m
timeout: 2m

issues:
# Only report issues for changes since master
Expand All @@ -10,7 +10,8 @@ issues:
# output configuration options
output:
# colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number"
format: colored-line-number
formats:
- format: colored-line-number

linters-settings:
errcheck:
Expand All @@ -32,13 +33,8 @@ linters-settings:
# simplify code: gofmt with `-s` option, true by default
simplify: true

golint:
# minimal confidence for issues, default is 0.8
min-confidence: 0.8

govet:
# report about shadowed variables
check-shadowing: true
enable-all: true
disable:
# Do not check field memory alignment because in most cases the performance gain is not worth the headache
Expand All @@ -56,8 +52,6 @@ linters:
- gofmt
# Checks error handling
- errcheck
# Checks deadcode
- deadcode
# Linter for Go source code that specializes in simplifying a code
- gosimple
# Vet examines Go source code and reports suspicious constructs, such as Printf calls whose
Expand All @@ -67,5 +61,9 @@ linters:
- ineffassign
# Static code analytics
- staticcheck
# Finds unused struct fields
- structcheck
# Reports unused function parameters.
- unparam
# Check if variables or functions are unused
- unused
# Remove unnecessary type conversions.
- unconvert
3 changes: 3 additions & 0 deletions ecosystem/http/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ func ensureHeaderOrder(t *testing.T, resp *httptest.ResponseRecorder, req *http.

func TestPreAndPostCallMiddleware(t *testing.T) {

//nolint:unparam
ep := func(writer http.ResponseWriter, request *http.Request) error {
request.Header.Add("call", timestamp())

Expand Down Expand Up @@ -176,6 +177,7 @@ func TestPreAndPostCallMiddleware(t *testing.T) {

func TestMiddlewareAdapter(t *testing.T) {

//nolint:unparam
ep := func(writer http.ResponseWriter, request *http.Request) error {
request.Header.Add("call", timestamp())

Expand Down Expand Up @@ -244,6 +246,7 @@ func TestMiddlewareAdapter(t *testing.T) {

func TestMiddlewareReverseAdapter(t *testing.T) {

//nolint:unparam
ep := func(writer http.ResponseWriter, request *http.Request) error {
request.Header.Add("call", timestamp())

Expand Down
5 changes: 4 additions & 1 deletion errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ func New(_fmt string, args ...interface{}) *SimpleError {

// Error satisfies the `error` interface. It uses the `simplerr.Formatter` to generate an error string.
func (e *SimpleError) Error() string {
return Formatter(e)
if parent := e.Unwrap(); parent != nil {
return fmt.Sprintf("%s: %s", e.GetMessage(), parent.Error())
}
return e.msg
}

// Message sets the message text on the error. This message it used to wrap the underlying error, if it exists.
Expand Down
11 changes: 0 additions & 11 deletions errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,17 +517,6 @@ func (s *TestSuite) TestErrorFormatting() {
serr3 := New("something")
s.Equal("something", serr3.Error())

// Change the error formatting style
Formatter = func(e *SimpleError) string {
parent := e.Unwrap()
if parent == nil {
return e.GetMessage()
}
return strings.Join([]string{e.GetMessage(), parent.Error()}, "\n")
}
s.Equal("wrapper 1\noriginal", serr1.Error())
s.Equal("wrapper 2\nwrapper 1\noriginal", serr2.Error())
Formatter = DefaultFormatter
}

func (s *TestSuite) TestStackTrace() {
Expand Down
14 changes: 0 additions & 14 deletions formatters.go

This file was deleted.

Loading

0 comments on commit b84ffca

Please sign in to comment.