Skip to content

Commit

Permalink
Remove string checking in API error handling
Browse files Browse the repository at this point in the history
Use strongly typed errors to set HTTP status codes.
Error interfaces are defined in the api/errors package and errors
returned from controllers are checked against these interfaces.

Errors can be wraeped in a pkg/errors.Causer, as long as somewhere in the
line of causes one of the interfaces is implemented. The special error
interfaces take precedence over Causer, meaning if both Causer and one
of the new error interfaces are implemented, the Causer is not
traversed.

Signed-off-by: Brian Goff <[email protected]>
  • Loading branch information
cpuguy83 committed Aug 15, 2017
1 parent b649834 commit ebcb7d6
Show file tree
Hide file tree
Showing 127 changed files with 1,777 additions and 871 deletions.
54 changes: 54 additions & 0 deletions api/errdefs/defs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package errdefs

// ErrNotFound signals that the requested object doesn't exist
type ErrNotFound interface {
NotFound()
}

// ErrInvalidParameter signals that the user input is invalid
type ErrInvalidParameter interface {
InvalidParameter()
}

// ErrConflict signals that some internal state conflicts with the requested action and can't be performed.
// A change in state should be able to clear this error.
type ErrConflict interface {
Conflict()
}

// ErrUnauthorized is used to signify that the user is not authorized to perform a specific action
type ErrUnauthorized interface {
Unauthorized()
}

// ErrUnavailable signals that the requested action/subsystem is not available.
type ErrUnavailable interface {
Unavailable()
}

// ErrForbidden signals that the requested action cannot be performed under any circumstances.
// When a ErrForbidden is returned, the caller should never retry the action.
type ErrForbidden interface {
Forbidden()
}

// ErrSystem signals that some internal error occurred.
// An example of this would be a failed mount request.
type ErrSystem interface {
ErrSystem()
}

// ErrNotModified signals that an action can't be performed because it's already in the desired state
type ErrNotModified interface {
NotModified()
}

// ErrNotImplemented signals that the requested action/feature is not implemented on the system as configured.
type ErrNotImplemented interface {
NotImplemented()
}

// ErrUnknown signals that the kind of error that occurred is not known.
type ErrUnknown interface {
Unknown()
}
8 changes: 8 additions & 0 deletions api/errdefs/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Package errdefs defines a set of error interfaces that packages should use for communicating classes of errors.
// Errors that cross the package boundary should implement one (and only one) of these interfaces.
//
// Packages should not reference these interfaces directly, only implement them.
// To check if a particular error implements one of these interfaces, there are helper
// functions provided (e.g. `Is<SomeError>`) which can be used rather than asserting the interfaces directly.
// If you must assert on these interfaces, be sure to check the causal chain (`err.Cause()`).
package errdefs
86 changes: 86 additions & 0 deletions api/errdefs/is.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package errdefs

type causer interface {
Cause() error
}

func getImplementer(err error) error {
switch e := err.(type) {
case
ErrNotFound,
ErrInvalidParameter,
ErrConflict,
ErrUnauthorized,
ErrUnavailable,
ErrForbidden,
ErrSystem,
ErrNotModified,
ErrNotImplemented,
ErrUnknown:
return e
case causer:
return getImplementer(e.Cause())
default:
return err
}
}

// IsNotFound returns if the passed in error is a ErrNotFound
func IsNotFound(err error) bool {
_, ok := getImplementer(err).(ErrNotFound)
return ok
}

// IsInvalidParameter returns if the passed in error is an ErrInvalidParameter
func IsInvalidParameter(err error) bool {
_, ok := getImplementer(err).(ErrInvalidParameter)
return ok
}

// IsConflict returns if the passed in error is a ErrConflict
func IsConflict(err error) bool {
_, ok := getImplementer(err).(ErrConflict)
return ok
}

// IsUnauthorized returns if the the passed in error is an ErrUnauthorized
func IsUnauthorized(err error) bool {
_, ok := getImplementer(err).(ErrUnauthorized)
return ok
}

// IsUnavailable returns if the passed in error is an ErrUnavailable
func IsUnavailable(err error) bool {
_, ok := getImplementer(err).(ErrUnavailable)
return ok
}

// IsForbidden returns if the passed in error is a ErrForbidden
func IsForbidden(err error) bool {
_, ok := getImplementer(err).(ErrForbidden)
return ok
}

// IsSystem returns if the passed in error is a ErrSystem
func IsSystem(err error) bool {
_, ok := getImplementer(err).(ErrSystem)
return ok
}

// IsNotModified returns if the passed in error is a NotModified error
func IsNotModified(err error) bool {
_, ok := getImplementer(err).(ErrNotModified)
return ok
}

// IsNotImplemented returns if the passed in error is a ErrNotImplemented
func IsNotImplemented(err error) bool {
_, ok := getImplementer(err).(ErrNotImplemented)
return ok
}

// IsUnknown returns if the passed in error is an ErrUnknown
func IsUnknown(err error) bool {
_, ok := getImplementer(err).(ErrUnknown)
return ok
}
47 changes: 0 additions & 47 deletions api/errors/errors.go

This file was deleted.

64 changes: 0 additions & 64 deletions api/errors/errors_test.go

This file was deleted.

78 changes: 36 additions & 42 deletions api/server/httputils/errors.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package httputils

import (
"fmt"
"net/http"
"strings"

"github.com/docker/docker/api/errdefs"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/versions"
"github.com/gorilla/mux"
Expand All @@ -20,13 +21,8 @@ type httpStatusError interface {
HTTPErrorStatusCode() int
}

// inputValidationError is an interface
// that errors generated by invalid
// inputs can implement to tell the
// api layer to set a 400 status code
// in the response.
type inputValidationError interface {
IsValidationError() bool
type causer interface {
Cause() error
}

// GetHTTPErrorStatusCode retrieves status code from error message.
Expand All @@ -37,49 +33,44 @@ func GetHTTPErrorStatusCode(err error) int {
}

var statusCode int
errMsg := err.Error()

switch e := err.(type) {
case httpStatusError:
statusCode = e.HTTPErrorStatusCode()
case inputValidationError:
// Stop right there
// Are you sure you should be adding a new error class here? Do one of the existing ones work?

// Note that the below functions are already checking the error causal chain for matches.
switch {
case errdefs.IsNotFound(err):
statusCode = http.StatusNotFound
case errdefs.IsInvalidParameter(err):
statusCode = http.StatusBadRequest
case errdefs.IsConflict(err):
statusCode = http.StatusConflict
case errdefs.IsUnauthorized(err):
statusCode = http.StatusUnauthorized
case errdefs.IsUnavailable(err):
statusCode = http.StatusServiceUnavailable
case errdefs.IsForbidden(err):
statusCode = http.StatusForbidden
case errdefs.IsNotModified(err):
statusCode = http.StatusNotModified
case errdefs.IsNotImplemented(err):
statusCode = http.StatusNotImplemented
case errdefs.IsSystem(err) || errdefs.IsUnknown(err):
statusCode = http.StatusInternalServerError
default:
statusCode = statusCodeFromGRPCError(err)
if statusCode != http.StatusInternalServerError {
return statusCode
}

// FIXME: this is brittle and should not be necessary, but we still need to identify if
// there are errors falling back into this logic.
// If we need to differentiate between different possible error types,
// we should create appropriate error types that implement the httpStatusError interface.
errStr := strings.ToLower(errMsg)

for _, status := range []struct {
keyword string
code int
}{
{"not found", http.StatusNotFound},
{"cannot find", http.StatusNotFound},
{"no such", http.StatusNotFound},
{"bad parameter", http.StatusBadRequest},
{"no command", http.StatusBadRequest},
{"conflict", http.StatusConflict},
{"impossible", http.StatusNotAcceptable},
{"wrong login/password", http.StatusUnauthorized},
{"unauthorized", http.StatusUnauthorized},
{"hasn't been activated", http.StatusForbidden},
{"this node", http.StatusServiceUnavailable},
{"needs to be unlocked", http.StatusServiceUnavailable},
{"certificates have expired", http.StatusServiceUnavailable},
{"repository does not exist", http.StatusNotFound},
} {
if strings.Contains(errStr, status.keyword) {
statusCode = status.code
break
}
if e, ok := err.(causer); ok {
return GetHTTPErrorStatusCode(e.Cause())
}

logrus.WithFields(logrus.Fields{
"module": "api",
"error_type": fmt.Sprintf("%T", err),
}).Debugf("FIXME: Got an API for which error does not match any expected type!!!: %+v", err)
}

if statusCode == 0 {
Expand Down Expand Up @@ -133,6 +124,9 @@ func statusCodeFromGRPCError(err error) int {
case codes.Unavailable: // code 14
return http.StatusServiceUnavailable
default:
if e, ok := err.(causer); ok {
return statusCodeFromGRPCError(e.Cause())
}
// codes.Canceled(1)
// codes.Unknown(2)
// codes.DeadlineExceeded(4)
Expand Down
Loading

0 comments on commit ebcb7d6

Please sign in to comment.