Browse code

errdefs: move GetHTTPErrorStatusCode to api/server/httpstatus

This reverts the changes made in 2a9c987e5a72549775ffa4dc31595ceff4f06a78, which
moved the GetHTTPErrorStatusCode() utility to the errdefs package.

While it seemed to make sense at the time to have the errdefs package provide
conversion both from HTTP status codes errdefs and the reverse, a side-effect
of the move was that the errdefs package now had a dependency on various external
modules, to handle conversio of errors coming from those sub-systems, such as;

- github.com/containerd/containerd
- github.com/docker/distribution
- google.golang.org/grpc

This patch moves the conversion from (errdef-) errors to HTTP status-codes to a
api/server/httpstatus package, which is only used by the API server, and should
not be needed by client-code using the errdefs package.

The MakeErrorHandler() utility was moved to the API server itself, as that's the
only place it's used. While the same applies to the GetHTTPErrorStatusCode func,
I opted for keeping that in its own package for a slightly cleaner interface.

Why not move it into the api/server/httputils package?

The api/server/httputils package is also imported in the client package, which
uses the httputils.ParseForm() and httputils.HijackConnection() functions as
part of the TestTLSCloseWriter() test. While this is only used in tests, I
wanted to avoid introducing the indirect depdencencies outside of the api/server
code.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Sebastiaan van Stijn authored on 2022/03/21 19:27:39
Showing 7 changed files
1 1
new file mode 100644
... ...
@@ -0,0 +1,34 @@
0
+package server
1
+
2
+import (
3
+	"net/http"
4
+
5
+	"github.com/docker/docker/api/server/httpstatus"
6
+	"github.com/docker/docker/api/server/httputils"
7
+	"github.com/docker/docker/api/types"
8
+	"github.com/docker/docker/api/types/versions"
9
+	"github.com/gorilla/mux"
10
+	"google.golang.org/grpc/status"
11
+)
12
+
13
+// makeErrorHandler makes an HTTP handler that decodes a Docker error and
14
+// returns it in the response.
15
+func makeErrorHandler(err error) http.HandlerFunc {
16
+	return func(w http.ResponseWriter, r *http.Request) {
17
+		statusCode := httpstatus.FromError(err)
18
+		vars := mux.Vars(r)
19
+		if apiVersionSupportsJSONErrors(vars["version"]) {
20
+			response := &types.ErrorResponse{
21
+				Message: err.Error(),
22
+			}
23
+			_ = httputils.WriteJSON(w, statusCode, response)
24
+		} else {
25
+			http.Error(w, status.Convert(err).Message(), statusCode)
26
+		}
27
+	}
28
+}
29
+
30
+func apiVersionSupportsJSONErrors(version string) bool {
31
+	const firstAPIVersionWithJSONErrors = "1.23"
32
+	return version == "" || versions.GreaterThan(version, firstAPIVersionWithJSONErrors)
33
+}
0 34
new file mode 100644
... ...
@@ -0,0 +1,150 @@
0
+package httpstatus // import "github.com/docker/docker/api/server/httpstatus"
1
+
2
+import (
3
+	"fmt"
4
+	"net/http"
5
+
6
+	containerderrors "github.com/containerd/containerd/errdefs"
7
+	"github.com/docker/distribution/registry/api/errcode"
8
+	"github.com/docker/docker/errdefs"
9
+	"github.com/sirupsen/logrus"
10
+	"google.golang.org/grpc/codes"
11
+	"google.golang.org/grpc/status"
12
+)
13
+
14
+type causer interface {
15
+	Cause() error
16
+}
17
+
18
+// FromError retrieves status code from error message.
19
+func FromError(err error) int {
20
+	if err == nil {
21
+		logrus.WithFields(logrus.Fields{"error": err}).Error("unexpected HTTP error handling")
22
+		return http.StatusInternalServerError
23
+	}
24
+
25
+	var statusCode int
26
+
27
+	// Stop right there
28
+	// Are you sure you should be adding a new error class here? Do one of the existing ones work?
29
+
30
+	// Note that the below functions are already checking the error causal chain for matches.
31
+	switch {
32
+	case errdefs.IsNotFound(err):
33
+		statusCode = http.StatusNotFound
34
+	case errdefs.IsInvalidParameter(err):
35
+		statusCode = http.StatusBadRequest
36
+	case errdefs.IsConflict(err):
37
+		statusCode = http.StatusConflict
38
+	case errdefs.IsUnauthorized(err):
39
+		statusCode = http.StatusUnauthorized
40
+	case errdefs.IsUnavailable(err):
41
+		statusCode = http.StatusServiceUnavailable
42
+	case errdefs.IsForbidden(err):
43
+		statusCode = http.StatusForbidden
44
+	case errdefs.IsNotModified(err):
45
+		statusCode = http.StatusNotModified
46
+	case errdefs.IsNotImplemented(err):
47
+		statusCode = http.StatusNotImplemented
48
+	case errdefs.IsSystem(err) || errdefs.IsUnknown(err) || errdefs.IsDataLoss(err) || errdefs.IsDeadline(err) || errdefs.IsCancelled(err):
49
+		statusCode = http.StatusInternalServerError
50
+	default:
51
+		statusCode = statusCodeFromGRPCError(err)
52
+		if statusCode != http.StatusInternalServerError {
53
+			return statusCode
54
+		}
55
+		statusCode = statusCodeFromContainerdError(err)
56
+		if statusCode != http.StatusInternalServerError {
57
+			return statusCode
58
+		}
59
+		statusCode = statusCodeFromDistributionError(err)
60
+		if statusCode != http.StatusInternalServerError {
61
+			return statusCode
62
+		}
63
+		if e, ok := err.(causer); ok {
64
+			return FromError(e.Cause())
65
+		}
66
+
67
+		logrus.WithFields(logrus.Fields{
68
+			"module":     "api",
69
+			"error_type": fmt.Sprintf("%T", err),
70
+		}).Debugf("FIXME: Got an API for which error does not match any expected type!!!: %+v", err)
71
+	}
72
+
73
+	if statusCode == 0 {
74
+		statusCode = http.StatusInternalServerError
75
+	}
76
+
77
+	return statusCode
78
+}
79
+
80
+// statusCodeFromGRPCError returns status code according to gRPC error
81
+func statusCodeFromGRPCError(err error) int {
82
+	switch status.Code(err) {
83
+	case codes.InvalidArgument: // code 3
84
+		return http.StatusBadRequest
85
+	case codes.NotFound: // code 5
86
+		return http.StatusNotFound
87
+	case codes.AlreadyExists: // code 6
88
+		return http.StatusConflict
89
+	case codes.PermissionDenied: // code 7
90
+		return http.StatusForbidden
91
+	case codes.FailedPrecondition: // code 9
92
+		return http.StatusBadRequest
93
+	case codes.Unauthenticated: // code 16
94
+		return http.StatusUnauthorized
95
+	case codes.OutOfRange: // code 11
96
+		return http.StatusBadRequest
97
+	case codes.Unimplemented: // code 12
98
+		return http.StatusNotImplemented
99
+	case codes.Unavailable: // code 14
100
+		return http.StatusServiceUnavailable
101
+	default:
102
+		// codes.Canceled(1)
103
+		// codes.Unknown(2)
104
+		// codes.DeadlineExceeded(4)
105
+		// codes.ResourceExhausted(8)
106
+		// codes.Aborted(10)
107
+		// codes.Internal(13)
108
+		// codes.DataLoss(15)
109
+		return http.StatusInternalServerError
110
+	}
111
+}
112
+
113
+// statusCodeFromDistributionError returns status code according to registry errcode
114
+// code is loosely based on errcode.ServeJSON() in docker/distribution
115
+func statusCodeFromDistributionError(err error) int {
116
+	switch errs := err.(type) {
117
+	case errcode.Errors:
118
+		if len(errs) < 1 {
119
+			return http.StatusInternalServerError
120
+		}
121
+		if _, ok := errs[0].(errcode.ErrorCoder); ok {
122
+			return statusCodeFromDistributionError(errs[0])
123
+		}
124
+	case errcode.ErrorCoder:
125
+		return errs.ErrorCode().Descriptor().HTTPStatusCode
126
+	}
127
+	return http.StatusInternalServerError
128
+}
129
+
130
+// statusCodeFromContainerdError returns status code for containerd errors when
131
+// consumed directly (not through gRPC)
132
+func statusCodeFromContainerdError(err error) int {
133
+	switch {
134
+	case containerderrors.IsInvalidArgument(err):
135
+		return http.StatusBadRequest
136
+	case containerderrors.IsNotFound(err):
137
+		return http.StatusNotFound
138
+	case containerderrors.IsAlreadyExists(err):
139
+		return http.StatusConflict
140
+	case containerderrors.IsFailedPrecondition(err):
141
+		return http.StatusPreconditionFailed
142
+	case containerderrors.IsUnavailable(err):
143
+		return http.StatusServiceUnavailable
144
+	case containerderrors.IsNotImplemented(err):
145
+		return http.StatusNotImplemented
146
+	default:
147
+		return http.StatusInternalServerError
148
+	}
149
+}
0 150
deleted file mode 100644
... ...
@@ -1,9 +0,0 @@
1
-package httputils // import "github.com/docker/docker/api/server/httputils"
2
-import "github.com/docker/docker/errdefs"
3
-
4
-// GetHTTPErrorStatusCode retrieves status code from error message.
5
-//
6
-// Deprecated: use errdefs.GetHTTPErrorStatusCode
7
-func GetHTTPErrorStatusCode(err error) int {
8
-	return errdefs.GetHTTPErrorStatusCode(err)
9
-}
... ...
@@ -7,13 +7,9 @@ import (
7 7
 	"net/http"
8 8
 	"strings"
9 9
 
10
-	"github.com/docker/docker/api/types"
11
-	"github.com/docker/docker/api/types/versions"
12 10
 	"github.com/docker/docker/errdefs"
13
-	"github.com/gorilla/mux"
14 11
 	"github.com/pkg/errors"
15 12
 	"github.com/sirupsen/logrus"
16
-	"google.golang.org/grpc/status"
17 13
 )
18 14
 
19 15
 // APIVersionKey is the client's requested API version.
... ...
@@ -92,28 +88,6 @@ func VersionFromContext(ctx context.Context) string {
92 92
 	return ""
93 93
 }
94 94
 
95
-// MakeErrorHandler makes an HTTP handler that decodes a Docker error and
96
-// returns it in the response.
97
-func MakeErrorHandler(err error) http.HandlerFunc {
98
-	return func(w http.ResponseWriter, r *http.Request) {
99
-		statusCode := errdefs.GetHTTPErrorStatusCode(err)
100
-		vars := mux.Vars(r)
101
-		if apiVersionSupportsJSONErrors(vars["version"]) {
102
-			response := &types.ErrorResponse{
103
-				Message: err.Error(),
104
-			}
105
-			_ = WriteJSON(w, statusCode, response)
106
-		} else {
107
-			http.Error(w, status.Convert(err).Message(), statusCode)
108
-		}
109
-	}
110
-}
111
-
112
-func apiVersionSupportsJSONErrors(version string) bool {
113
-	const firstAPIVersionWithJSONErrors = "1.23"
114
-	return version == "" || versions.GreaterThan(version, firstAPIVersionWithJSONErrors)
115
-}
116
-
117 95
 // matchesContentType validates the content type against the expected one
118 96
 func matchesContentType(contentType, expectedType string) bool {
119 97
 	mimetype, _, err := mime.ParseMediaType(contentType)
... ...
@@ -10,6 +10,7 @@ import (
10 10
 	"syscall"
11 11
 
12 12
 	"github.com/containerd/containerd/platforms"
13
+	"github.com/docker/docker/api/server/httpstatus"
13 14
 	"github.com/docker/docker/api/server/httputils"
14 15
 	"github.com/docker/docker/api/types"
15 16
 	"github.com/docker/docker/api/types/backend"
... ...
@@ -641,7 +642,7 @@ func (s *containerRouter) postContainersAttach(ctx context.Context, w http.Respo
641 641
 		// Remember to close stream if error happens
642 642
 		conn, _, errHijack := hijacker.Hijack()
643 643
 		if errHijack == nil {
644
-			statusCode := errdefs.GetHTTPErrorStatusCode(err)
644
+			statusCode := httpstatus.FromError(err)
645 645
 			statusText := http.StatusText(statusCode)
646 646
 			fmt.Fprintf(conn, "HTTP/1.1 %d %s\r\nContent-Type: application/vnd.docker.raw-stream\r\n\r\n%s\r\n", statusCode, statusText, err.Error())
647 647
 			httputils.CloseStreams(conn)
... ...
@@ -7,12 +7,12 @@ import (
7 7
 	"net/http"
8 8
 	"strings"
9 9
 
10
+	"github.com/docker/docker/api/server/httpstatus"
10 11
 	"github.com/docker/docker/api/server/httputils"
11 12
 	"github.com/docker/docker/api/server/middleware"
12 13
 	"github.com/docker/docker/api/server/router"
13 14
 	"github.com/docker/docker/api/server/router/debug"
14 15
 	"github.com/docker/docker/dockerversion"
15
-	"github.com/docker/docker/errdefs"
16 16
 	"github.com/gorilla/mux"
17 17
 	"github.com/sirupsen/logrus"
18 18
 )
... ...
@@ -139,11 +139,11 @@ func (s *Server) makeHTTPHandler(handler httputils.APIFunc) http.HandlerFunc {
139 139
 		}
140 140
 
141 141
 		if err := handlerFunc(ctx, w, r, vars); err != nil {
142
-			statusCode := errdefs.GetHTTPErrorStatusCode(err)
142
+			statusCode := httpstatus.FromError(err)
143 143
 			if statusCode >= 500 {
144 144
 				logrus.Errorf("Handler for %s %s returned error: %v", r.Method, r.URL.Path, err)
145 145
 			}
146
-			httputils.MakeErrorHandler(err)(w, r)
146
+			makeErrorHandler(err)(w, r)
147 147
 		}
148 148
 	}
149 149
 }
... ...
@@ -184,7 +184,7 @@ func (s *Server) createMux() *mux.Router {
184 184
 		m.Path("/debug" + r.Path()).Handler(f)
185 185
 	}
186 186
 
187
-	notFoundHandler := httputils.MakeErrorHandler(pageNotFoundError{})
187
+	notFoundHandler := makeErrorHandler(pageNotFoundError{})
188 188
 	m.HandleFunc(versionMatcher+"/{path:.*}", notFoundHandler)
189 189
 	m.NotFoundHandler = notFoundHandler
190 190
 	m.MethodNotAllowedHandler = notFoundHandler
... ...
@@ -1,78 +1,11 @@
1 1
 package errdefs // import "github.com/docker/docker/errdefs"
2 2
 
3 3
 import (
4
-	"fmt"
5 4
 	"net/http"
6 5
 
7
-	containerderrors "github.com/containerd/containerd/errdefs"
8
-	"github.com/docker/distribution/registry/api/errcode"
9 6
 	"github.com/sirupsen/logrus"
10
-	"google.golang.org/grpc/codes"
11
-	"google.golang.org/grpc/status"
12 7
 )
13 8
 
14
-// GetHTTPErrorStatusCode retrieves status code from error message.
15
-func GetHTTPErrorStatusCode(err error) int {
16
-	if err == nil {
17
-		logrus.WithFields(logrus.Fields{"error": err}).Error("unexpected HTTP error handling")
18
-		return http.StatusInternalServerError
19
-	}
20
-
21
-	var statusCode int
22
-
23
-	// Stop right there
24
-	// Are you sure you should be adding a new error class here? Do one of the existing ones work?
25
-
26
-	// Note that the below functions are already checking the error causal chain for matches.
27
-	switch {
28
-	case IsNotFound(err):
29
-		statusCode = http.StatusNotFound
30
-	case IsInvalidParameter(err):
31
-		statusCode = http.StatusBadRequest
32
-	case IsConflict(err):
33
-		statusCode = http.StatusConflict
34
-	case IsUnauthorized(err):
35
-		statusCode = http.StatusUnauthorized
36
-	case IsUnavailable(err):
37
-		statusCode = http.StatusServiceUnavailable
38
-	case IsForbidden(err):
39
-		statusCode = http.StatusForbidden
40
-	case IsNotModified(err):
41
-		statusCode = http.StatusNotModified
42
-	case IsNotImplemented(err):
43
-		statusCode = http.StatusNotImplemented
44
-	case IsSystem(err) || IsUnknown(err) || IsDataLoss(err) || IsDeadline(err) || IsCancelled(err):
45
-		statusCode = http.StatusInternalServerError
46
-	default:
47
-		statusCode = statusCodeFromGRPCError(err)
48
-		if statusCode != http.StatusInternalServerError {
49
-			return statusCode
50
-		}
51
-		statusCode = statusCodeFromContainerdError(err)
52
-		if statusCode != http.StatusInternalServerError {
53
-			return statusCode
54
-		}
55
-		statusCode = statusCodeFromDistributionError(err)
56
-		if statusCode != http.StatusInternalServerError {
57
-			return statusCode
58
-		}
59
-		if e, ok := err.(causer); ok {
60
-			return GetHTTPErrorStatusCode(e.Cause())
61
-		}
62
-
63
-		logrus.WithFields(logrus.Fields{
64
-			"module":     "api",
65
-			"error_type": fmt.Sprintf("%T", err),
66
-		}).Debugf("FIXME: Got an API for which error does not match any expected type!!!: %+v", err)
67
-	}
68
-
69
-	if statusCode == 0 {
70
-		statusCode = http.StatusInternalServerError
71
-	}
72
-
73
-	return statusCode
74
-}
75
-
76 9
 // FromStatusCode creates an errdef error, based on the provided HTTP status-code
77 10
 func FromStatusCode(err error, statusCode int) error {
78 11
 	if err == nil {
... ...
@@ -118,74 +51,3 @@ func FromStatusCode(err error, statusCode int) error {
118 118
 	}
119 119
 	return err
120 120
 }
121
-
122
-// statusCodeFromGRPCError returns status code according to gRPC error
123
-func statusCodeFromGRPCError(err error) int {
124
-	switch status.Code(err) {
125
-	case codes.InvalidArgument: // code 3
126
-		return http.StatusBadRequest
127
-	case codes.NotFound: // code 5
128
-		return http.StatusNotFound
129
-	case codes.AlreadyExists: // code 6
130
-		return http.StatusConflict
131
-	case codes.PermissionDenied: // code 7
132
-		return http.StatusForbidden
133
-	case codes.FailedPrecondition: // code 9
134
-		return http.StatusBadRequest
135
-	case codes.Unauthenticated: // code 16
136
-		return http.StatusUnauthorized
137
-	case codes.OutOfRange: // code 11
138
-		return http.StatusBadRequest
139
-	case codes.Unimplemented: // code 12
140
-		return http.StatusNotImplemented
141
-	case codes.Unavailable: // code 14
142
-		return http.StatusServiceUnavailable
143
-	default:
144
-		// codes.Canceled(1)
145
-		// codes.Unknown(2)
146
-		// codes.DeadlineExceeded(4)
147
-		// codes.ResourceExhausted(8)
148
-		// codes.Aborted(10)
149
-		// codes.Internal(13)
150
-		// codes.DataLoss(15)
151
-		return http.StatusInternalServerError
152
-	}
153
-}
154
-
155
-// statusCodeFromDistributionError returns status code according to registry errcode
156
-// code is loosely based on errcode.ServeJSON() in docker/distribution
157
-func statusCodeFromDistributionError(err error) int {
158
-	switch errs := err.(type) {
159
-	case errcode.Errors:
160
-		if len(errs) < 1 {
161
-			return http.StatusInternalServerError
162
-		}
163
-		if _, ok := errs[0].(errcode.ErrorCoder); ok {
164
-			return statusCodeFromDistributionError(errs[0])
165
-		}
166
-	case errcode.ErrorCoder:
167
-		return errs.ErrorCode().Descriptor().HTTPStatusCode
168
-	}
169
-	return http.StatusInternalServerError
170
-}
171
-
172
-// statusCodeFromContainerdError returns status code for containerd errors when
173
-// consumed directly (not through gRPC)
174
-func statusCodeFromContainerdError(err error) int {
175
-	switch {
176
-	case containerderrors.IsInvalidArgument(err):
177
-		return http.StatusBadRequest
178
-	case containerderrors.IsNotFound(err):
179
-		return http.StatusNotFound
180
-	case containerderrors.IsAlreadyExists(err):
181
-		return http.StatusConflict
182
-	case containerderrors.IsFailedPrecondition(err):
183
-		return http.StatusPreconditionFailed
184
-	case containerderrors.IsUnavailable(err):
185
-		return http.StatusServiceUnavailable
186
-	case containerderrors.IsNotImplemented(err):
187
-		return http.StatusNotImplemented
188
-	default:
189
-		return http.StatusInternalServerError
190
-	}
191
-}