Skip to content

Commit 2afb3ef

Browse files
yongtangyongtang
authored
Merge pull request moby#34188 from cpuguy83/32144_api_error_handling
Remove string checking in API error handling
2 parents 3d843e8 + ebcb7d6 commit 2afb3ef

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)