Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 12 additions & 19 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
linters-settings:
govet:
check-shadowing: true
golint:
min-confidence: 0
gocyclo:
min-complexity: 45
maligned:
suggest-new: true
dupl:
threshold: 200
goconst:
Expand All @@ -16,8 +10,6 @@ linters-settings:
linters:
enable-all: true
disable:
- errname # this repo doesn't follow the convention advised by this linter
- maligned
- unparam
- lll
- gochecknoinits
Expand All @@ -30,17 +22,13 @@ linters:
- wrapcheck
- testpackage
- nlreturn
- gomnd
- exhaustivestruct
- goerr113
- errorlint
- nestif
- godot
- gofumpt
- paralleltest
- tparallel
- thelper
- ifshort
- exhaustruct
- varnamelen
- gci
Expand All @@ -53,10 +41,15 @@ linters:
- forcetypeassert
- cyclop
# deprecated linters
- deadcode
- interfacer
- scopelint
- varcheck
- structcheck
- golint
- nosnakecase
#- deadcode
#- interfacer
#- scopelint
#- varcheck
#- structcheck
#- golint
#- nosnakecase
#- maligned
#- goerr113
#- ifshort
#- gomnd
#- exhaustivestruct
2 changes: 1 addition & 1 deletion api.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func ServeError(rw http.ResponseWriter, r *http.Request, err error) {
}

func asHTTPCode(input int) int {
if input >= 600 {
if input >= maximumValidHTTPCode {
return DefaultHTTPCode
}
return input
Expand Down
272 changes: 164 additions & 108 deletions api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//nolint:err113
package errors

import (
Expand All @@ -31,120 +32,175 @@ type customError struct {
}

func TestServeError(t *testing.T) {
// method not allowed wins
// err abides by the Error interface
err := MethodNotAllowed("GET", []string{"POST", "PUT"})
recorder := httptest.NewRecorder()
ServeError(recorder, nil, err)
assert.Equal(t, http.StatusMethodNotAllowed, recorder.Code)
assert.Equal(t, "POST,PUT", recorder.Header().Get("Allow"))
// assert.Equal(t, "application/json", recorder.Header().Get("content-type"))
assert.Equal(t, `{"code":405,"message":"method GET is not allowed, but [POST,PUT] are"}`, recorder.Body.String())

// renders status code from error when present
err = NotFound("")
recorder = httptest.NewRecorder()
ServeError(recorder, nil, err)
assert.Equal(t, http.StatusNotFound, recorder.Code)
// assert.Equal(t, "application/json", recorder.Header().Get("content-type"))
assert.Equal(t, `{"code":404,"message":"Not found"}`, recorder.Body.String())

// renders mapped status code from error when present
err = InvalidTypeName("someType")
recorder = httptest.NewRecorder()
ServeError(recorder, nil, err)
assert.Equal(t, http.StatusUnprocessableEntity, recorder.Code)
// assert.Equal(t, "application/json", recorder.Header().Get("content-type"))
assert.Equal(t, `{"code":601,"message":"someType is an invalid type name"}`, recorder.Body.String())

// same, but override DefaultHTTPCode
func() {
oldDefaultHTTPCode := DefaultHTTPCode
defer func() { DefaultHTTPCode = oldDefaultHTTPCode }()
DefaultHTTPCode = http.StatusBadRequest

err = InvalidTypeName("someType")
recorder = httptest.NewRecorder()
t.Run("method not allowed wins", func(t *testing.T) {
// err abides by the Error interface
err := MethodNotAllowed("GET", []string{"POST", "PUT"})
require.Error(t, err)

recorder := httptest.NewRecorder()
ServeError(recorder, nil, err)
assert.Equal(t, http.StatusMethodNotAllowed, recorder.Code)
assert.Equal(t, "POST,PUT", recorder.Header().Get("Allow"))
// assert.Equal(t, "application/json", recorder.Header().Get("content-type"))
assert.JSONEq(t,
`{"code":405,"message":"method GET is not allowed, but [POST,PUT] are"}`,
recorder.Body.String(),
)
})

t.Run("renders status code from error", func(t *testing.T) {
err := NotFound("")
require.Error(t, err)

recorder := httptest.NewRecorder()
ServeError(recorder, nil, err)
assert.Equal(t, http.StatusNotFound, recorder.Code)
// assert.Equal(t, "application/json", recorder.Header().Get("content-type"))
assert.JSONEq(t,
`{"code":404,"message":"Not found"}`,
recorder.Body.String(),
)
})

t.Run("renders mapped status code from error", func(t *testing.T) {
// renders mapped status code from error when present
err := InvalidTypeName("someType")
require.Error(t, err)

recorder := httptest.NewRecorder()
ServeError(recorder, nil, err)
assert.Equal(t, http.StatusBadRequest, recorder.Code)
assert.Equal(t, http.StatusUnprocessableEntity, recorder.Code)
// assert.Equal(t, "application/json", recorder.Header().Get("content-type"))
assert.Equal(t, `{"code":601,"message":"someType is an invalid type name"}`, recorder.Body.String())
}()

// defaults to internal server error
simpleErr := errors.New("some error")
recorder = httptest.NewRecorder()
ServeError(recorder, nil, simpleErr)
assert.Equal(t, http.StatusInternalServerError, recorder.Code)
// assert.Equal(t, "application/json", recorder.Header().Get("content-type"))
assert.Equal(t, `{"code":500,"message":"some error"}`, recorder.Body.String())

// composite errors

// unrecognized: return internal error with first error only - the second error is ignored
compositeErr := &CompositeError{
Errors: []error{
errors.New("firstError"),
errors.New("anotherError"),
},
}
recorder = httptest.NewRecorder()
ServeError(recorder, nil, compositeErr)
assert.Equal(t, http.StatusInternalServerError, recorder.Code)
assert.Equal(t, `{"code":500,"message":"firstError"}`, recorder.Body.String())

// recognized: return internal error with first error only - the second error is ignored
compositeErr = &CompositeError{
Errors: []error{
New(600, "myApiError"),
New(601, "myOtherApiError"),
},
}
recorder = httptest.NewRecorder()
ServeError(recorder, nil, compositeErr)
assert.Equal(t, CompositeErrorCode, recorder.Code)
assert.Equal(t, `{"code":600,"message":"myApiError"}`, recorder.Body.String())

// recognized API Error, flattened
compositeErr = &CompositeError{
Errors: []error{
&CompositeError{
assert.JSONEq(t,
`{"code":601,"message":"someType is an invalid type name"}`,
recorder.Body.String(),
)
})

t.Run("overrides DefaultHTTPCode", func(t *testing.T) {
func() {
oldDefaultHTTPCode := DefaultHTTPCode
defer func() { DefaultHTTPCode = oldDefaultHTTPCode }()
DefaultHTTPCode = http.StatusBadRequest

err := InvalidTypeName("someType")
require.Error(t, err)

recorder := httptest.NewRecorder()
ServeError(recorder, nil, err)
assert.Equal(t, http.StatusBadRequest, recorder.Code)
// assert.Equal(t, "application/json", recorder.Header().Get("content-type"))
assert.JSONEq(t,
`{"code":601,"message":"someType is an invalid type name"}`,
recorder.Body.String(),
)
}()
})

t.Run("defaults to internal server error", func(t *testing.T) {
simpleErr := errors.New("some error")
recorder := httptest.NewRecorder()
ServeError(recorder, nil, simpleErr)
assert.Equal(t, http.StatusInternalServerError, recorder.Code)
// assert.Equal(t, "application/json", recorder.Header().Get("content-type"))
assert.JSONEq(t,
`{"code":500,"message":"some error"}`,
recorder.Body.String(),
)
})

t.Run("with composite erors", func(t *testing.T) {
t.Run("unrecognized - return internal error with first error only - the second error is ignored", func(t *testing.T) {
compositeErr := &CompositeError{
Errors: []error{
errors.New("firstError"),
errors.New("anotherError"),
},
}
recorder := httptest.NewRecorder()
ServeError(recorder, nil, compositeErr)
assert.Equal(t, http.StatusInternalServerError, recorder.Code)
assert.JSONEq(t,
`{"code":500,"message":"firstError"}`,
recorder.Body.String(),
)
})

t.Run("recognized - return internal error with first error only - the second error is ignored", func(t *testing.T) {
compositeErr := &CompositeError{
Errors: []error{
New(600, "myApiError"),
New(601, "myOtherApiError"),
},
},
},
}
recorder = httptest.NewRecorder()
ServeError(recorder, nil, compositeErr)
assert.Equal(t, CompositeErrorCode, recorder.Code)
assert.Equal(t, `{"code":600,"message":"myApiError"}`, recorder.Body.String())

// check guard against empty CompositeError (e.g. nil Error interface)
compositeErr = &CompositeError{
Errors: []error{
&CompositeError{
Errors: []error{},
},
},
}
recorder = httptest.NewRecorder()
ServeError(recorder, nil, compositeErr)
assert.Equal(t, http.StatusInternalServerError, recorder.Code)
assert.Equal(t, `{"code":500,"message":"Unknown error"}`, recorder.Body.String())

// check guard against nil type
recorder = httptest.NewRecorder()
ServeError(recorder, nil, nil)
assert.Equal(t, http.StatusInternalServerError, recorder.Code)
assert.Equal(t, `{"code":500,"message":"Unknown error"}`, recorder.Body.String())

recorder = httptest.NewRecorder()
var z *customError
ServeError(recorder, nil, z)
assert.Equal(t, http.StatusInternalServerError, recorder.Code)
assert.Equal(t, `{"code":500,"message":"Unknown error"}`, recorder.Body.String())
}
recorder := httptest.NewRecorder()
ServeError(recorder, nil, compositeErr)
assert.Equal(t, CompositeErrorCode, recorder.Code)
assert.JSONEq(t,
`{"code":600,"message":"myApiError"}`,
recorder.Body.String(),
)
})

t.Run("recognized API Error, flattened", func(t *testing.T) {
compositeErr := &CompositeError{
Errors: []error{
&CompositeError{
Errors: []error{
New(600, "myApiError"),
New(601, "myOtherApiError"),
},
},
},
}
recorder := httptest.NewRecorder()
ServeError(recorder, nil, compositeErr)
assert.Equal(t, CompositeErrorCode, recorder.Code)
assert.JSONEq(t,
`{"code":600,"message":"myApiError"}`,
recorder.Body.String(),
)
})

// (e.g. nil Error interface)
t.Run("check guard against empty CompositeError", func(t *testing.T) {
compositeErr := &CompositeError{
Errors: []error{
&CompositeError{
Errors: []error{},
},
},
}
recorder := httptest.NewRecorder()
ServeError(recorder, nil, compositeErr)
assert.Equal(t, http.StatusInternalServerError, recorder.Code)
assert.JSONEq(t,
`{"code":500,"message":"Unknown error"}`,
recorder.Body.String(),
)
})

t.Run("check guard against nil type", func(t *testing.T) {
recorder := httptest.NewRecorder()
ServeError(recorder, nil, nil)
assert.Equal(t, http.StatusInternalServerError, recorder.Code)
assert.JSONEq(t,
`{"code":500,"message":"Unknown error"}`,
recorder.Body.String(),
)
})

t.Run("check guard against nil value", func(t *testing.T) {
recorder := httptest.NewRecorder()
var z *customError
ServeError(recorder, nil, z)
assert.Equal(t, http.StatusInternalServerError, recorder.Code)
assert.JSONEq(t,
`{"code":500,"message":"Unknown error"}`,
recorder.Body.String(),
)
})
})
}

func TestAPIErrors(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion headers.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
)

// Validation represents a failure of a precondition
type Validation struct {
type Validation struct { //nolint: errname
code int32
Name string
In string
Expand Down
2 changes: 1 addition & 1 deletion middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (

// APIVerificationFailed is an error that contains all the missing info for a mismatched section
// between the api registrations and the api spec
type APIVerificationFailed struct {
type APIVerificationFailed struct { //nolint: errname
Section string `json:"section,omitempty"`
MissingSpecification []string `json:"missingSpecification,omitempty"`
MissingRegistration []string `json:"missingRegistration,omitempty"`
Expand Down
3 changes: 2 additions & 1 deletion parsing.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package errors
import (
"encoding/json"
"fmt"
"net/http"
)

// ParseError represents a parsing error
Expand Down Expand Up @@ -68,7 +69,7 @@ func NewParseError(name, in, value string, reason error) *ParseError {
msg = fmt.Sprintf(parseErrorTemplContent, name, in, value, reason)
}
return &ParseError{
code: 400,
code: http.StatusBadRequest,
Name: name,
In: in,
Value: value,
Expand Down
Loading
Loading