Skip to content

Commit ebcb7d6

Browse files
cpuguy83cpuguy83
committed
Remove string checking in API error handling
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 <cpuguy83@gmail.com>
1 parent b649834 commit ebcb7d6

File tree

127 files changed

+1777
-871
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

127 files changed

+1777
-871
lines changed

‎api/errdefs/defs.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package errdefs
2+
3+
// ErrNotFound signals that the requested object doesn't exist
4+
type ErrNotFound interface {
5+
NotFound()
6+
}
7+
8+
// ErrInvalidParameter signals that the user input is invalid
9+
type ErrInvalidParameter interface {
10+
InvalidParameter()
11+
}
12+
13+
// ErrConflict signals that some internal state conflicts with the requested action and can't be performed.
14+
// A change in state should be able to clear this error.
15+
type ErrConflict interface {
16+
Conflict()
17+
}
18+
19+
// ErrUnauthorized is used to signify that the user is not authorized to perform a specific action
20+
type ErrUnauthorized interface {
21+
Unauthorized()
22+
}
23+
24+
// ErrUnavailable signals that the requested action/subsystem is not available.
25+
type ErrUnavailable interface {
26+
Unavailable()
27+
}
28+
29+
// ErrForbidden signals that the requested action cannot be performed under any circumstances.
30+
// When a ErrForbidden is returned, the caller should never retry the action.
31+
type ErrForbidden interface {
32+
Forbidden()
33+
}
34+
35+
// ErrSystem signals that some internal error occurred.
36+
// An example of this would be a failed mount request.
37+
type ErrSystem interface {
38+
ErrSystem()
39+
}
40+
41+
// ErrNotModified signals that an action can't be performed because it's already in the desired state
42+
type ErrNotModified interface {
43+
NotModified()
44+
}
45+
46+
// ErrNotImplemented signals that the requested action/feature is not implemented on the system as configured.
47+
type ErrNotImplemented interface {
48+
NotImplemented()
49+
}
50+
51+
// ErrUnknown signals that the kind of error that occurred is not known.
52+
type ErrUnknown interface {
53+
Unknown()
54+
}

‎api/errdefs/doc.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Package errdefs defines a set of error interfaces that packages should use for communicating classes of errors.
2+
// Errors that cross the package boundary should implement one (and only one) of these interfaces.
3+
//
4+
// Packages should not reference these interfaces directly, only implement them.
5+
// To check if a particular error implements one of these interfaces, there are helper
6+
// functions provided (e.g. `Is<SomeError>`) which can be used rather than asserting the interfaces directly.
7+
// If you must assert on these interfaces, be sure to check the causal chain (`err.Cause()`).
8+
package errdefs

‎api/errdefs/is.go‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package errdefs
2+
3+
type causer interface {
4+
Cause() error
5+
}
6+
7+
func getImplementer(err error) error {
8+
switch e := err.(type) {
9+
case
10+
ErrNotFound,
11+
ErrInvalidParameter,
12+
ErrConflict,
13+
ErrUnauthorized,
14+
ErrUnavailable,
15+
ErrForbidden,
16+
ErrSystem,
17+
ErrNotModified,
18+
ErrNotImplemented,
19+
ErrUnknown:
20+
return e
21+
case causer:
22+
return getImplementer(e.Cause())
23+
default:
24+
return err
25+
}
26+
}
27+
28+
// IsNotFound returns if the passed in error is a ErrNotFound
29+
func IsNotFound(err error) bool {
30+
_, ok := getImplementer(err).(ErrNotFound)
31+
return ok
32+
}
33+
34+
// IsInvalidParameter returns if the passed in error is an ErrInvalidParameter
35+
func IsInvalidParameter(err error) bool {
36+
_, ok := getImplementer(err).(ErrInvalidParameter)
37+
return ok
38+
}
39+
40+
// IsConflict returns if the passed in error is a ErrConflict
41+
func IsConflict(err error) bool {
42+
_, ok := getImplementer(err).(ErrConflict)
43+
return ok
44+
}
45+
46+
// IsUnauthorized returns if the the passed in error is an ErrUnauthorized
47+
func IsUnauthorized(err error) bool {
48+
_, ok := getImplementer(err).(ErrUnauthorized)
49+
return ok
50+
}
51+
52+
// IsUnavailable returns if the passed in error is an ErrUnavailable
53+
func IsUnavailable(err error) bool {
54+
_, ok := getImplementer(err).(ErrUnavailable)
55+
return ok
56+
}
57+
58+
// IsForbidden returns if the passed in error is a ErrForbidden
59+
func IsForbidden(err error) bool {
60+
_, ok := getImplementer(err).(ErrForbidden)
61+
return ok
62+
}
63+
64+
// IsSystem returns if the passed in error is a ErrSystem
65+
func IsSystem(err error) bool {
66+
_, ok := getImplementer(err).(ErrSystem)
67+
return ok
68+
}
69+
70+
// IsNotModified returns if the passed in error is a NotModified error
71+
func IsNotModified(err error) bool {
72+
_, ok := getImplementer(err).(ErrNotModified)
73+
return ok
74+
}
75+
76+
// IsNotImplemented returns if the passed in error is a ErrNotImplemented
77+
func IsNotImplemented(err error) bool {
78+
_, ok := getImplementer(err).(ErrNotImplemented)
79+
return ok
80+
}
81+
82+
// IsUnknown returns if the passed in error is an ErrUnknown
83+
func IsUnknown(err error) bool {
84+
_, ok := getImplementer(err).(ErrUnknown)
85+
return ok
86+
}

‎api/errors/errors.go‎

Lines changed: 0 additions & 47 deletions
This file was deleted.

‎api/errors/errors_test.go‎

Lines changed: 0 additions & 64 deletions
This file was deleted.

‎api/server/httputils/errors.go‎

Lines changed: 36 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
package httputils
22

33
import (
4+
"fmt"
45
"net/http"
5-
"strings"
66

7+
"github.com/docker/docker/api/errdefs"
78
"github.com/docker/docker/api/types"
89
"github.com/docker/docker/api/types/versions"
910
"github.com/gorilla/mux"
@@ -20,13 +21,8 @@ type httpStatusError interface {
2021
HTTPErrorStatusCode() int
2122
}
2223

23-
// inputValidationError is an interface
24-
// that errors generated by invalid
25-
// inputs can implement to tell the
26-
// api layer to set a 400 status code
27-
// in the response.
28-
type inputValidationError interface {
29-
IsValidationError() bool
24+
type causer interface {
25+
Cause() error
3026
}
3127

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

3935
var statusCode int
40-
errMsg := err.Error()
4136

42-
switch e := err.(type) {
43-
case httpStatusError:
44-
statusCode = e.HTTPErrorStatusCode()
45-
case inputValidationError:
37+
// Stop right there
38+
// Are you sure you should be adding a new error class here? Do one of the existing ones work?
39+
40+
// Note that the below functions are already checking the error causal chain for matches.
41+
switch {
42+
case errdefs.IsNotFound(err):
43+
statusCode = http.StatusNotFound
44+
case errdefs.IsInvalidParameter(err):
4645
statusCode = http.StatusBadRequest
46+
case errdefs.IsConflict(err):
47+
statusCode = http.StatusConflict
48+
case errdefs.IsUnauthorized(err):
49+
statusCode = http.StatusUnauthorized
50+
case errdefs.IsUnavailable(err):
51+
statusCode = http.StatusServiceUnavailable
52+
case errdefs.IsForbidden(err):
53+
statusCode = http.StatusForbidden
54+
case errdefs.IsNotModified(err):
55+
statusCode = http.StatusNotModified
56+
case errdefs.IsNotImplemented(err):
57+
statusCode = http.StatusNotImplemented
58+
case errdefs.IsSystem(err) || errdefs.IsUnknown(err):
59+
statusCode = http.StatusInternalServerError
4760
default:
4861
statusCode = statusCodeFromGRPCError(err)
4962
if statusCode != http.StatusInternalServerError {
5063
return statusCode
5164
}
5265

53-
// FIXME: this is brittle and should not be necessary, but we still need to identify if
54-
// there are errors falling back into this logic.
55-
// If we need to differentiate between different possible error types,
56-
// we should create appropriate error types that implement the httpStatusError interface.
57-
errStr := strings.ToLower(errMsg)
58-
59-
for _, status := range []struct {
60-
keyword string
61-
code int
62-
}{
63-
{"not found", http.StatusNotFound},
64-
{"cannot find", http.StatusNotFound},
65-
{"no such", http.StatusNotFound},
66-
{"bad parameter", http.StatusBadRequest},
67-
{"no command", http.StatusBadRequest},
68-
{"conflict", http.StatusConflict},
69-
{"impossible", http.StatusNotAcceptable},
70-
{"wrong login/password", http.StatusUnauthorized},
71-
{"unauthorized", http.StatusUnauthorized},
72-
{"hasn't been activated", http.StatusForbidden},
73-
{"this node", http.StatusServiceUnavailable},
74-
{"needs to be unlocked", http.StatusServiceUnavailable},
75-
{"certificates have expired", http.StatusServiceUnavailable},
76-
{"repository does not exist", http.StatusNotFound},
77-
} {
78-
if strings.Contains(errStr, status.keyword) {
79-
statusCode = status.code
80-
break
81-
}
66+
if e, ok := err.(causer); ok {
67+
return GetHTTPErrorStatusCode(e.Cause())
8268
}
69+
70+
logrus.WithFields(logrus.Fields{
71+
"module": "api",
72+
"error_type": fmt.Sprintf("%T", err),
73+
}).Debugf("FIXME: Got an API for which error does not match any expected type!!!: %+v", err)
8374
}
8475

8576
if statusCode == 0 {
@@ -133,6 +124,9 @@ func statusCodeFromGRPCError(err error) int {
133124
case codes.Unavailable: // code 14
134125
return http.StatusServiceUnavailable
135126
default:
127+
if e, ok := err.(causer); ok {
128+
return statusCodeFromGRPCError(e.Cause())
129+
}
136130
// codes.Canceled(1)
137131
// codes.Unknown(2)
138132
// codes.DeadlineExceeded(4)

0 commit comments

Comments
 (0)