Merge pull request #43406 from dims/cherry-pick-thaJeztah-errdefs_reduce_client_deps_alternative

[20.10] errdefs: move GetHTTPErrorStatusCode to api/server/httpstatus

api/server/errorhandler.go

@@ -0,0 +1,34 @@
+package server
+
+import (
+	"net/http"
+
+	"github.com/docker/docker/api/server/httpstatus"
+	"github.com/docker/docker/api/server/httputils"
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/versions"
+	"github.com/gorilla/mux"
+	"google.golang.org/grpc/status"
+)
+
+// makeErrorHandler makes an HTTP handler that decodes a Docker error and
+// returns it in the response.
+func makeErrorHandler(err error) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		statusCode := httpstatus.FromError(err)
+		vars := mux.Vars(r)
+		if apiVersionSupportsJSONErrors(vars["version"]) {
+			response := &types.ErrorResponse{
+				Message: err.Error(),
+			}
+			_ = httputils.WriteJSON(w, statusCode, response)
+		} else {
+			http.Error(w, status.Convert(err).Message(), statusCode)
+		}
+	}
+}
+
+func apiVersionSupportsJSONErrors(version string) bool {
+	const firstAPIVersionWithJSONErrors = "1.23"
+	return version == "" || versions.GreaterThan(version, firstAPIVersionWithJSONErrors)
+}

api/server/httpstatus/status.go

@@ -0,0 +1,150 @@
+package httpstatus // import "github.com/docker/docker/api/server/httpstatus"
+
+import (
+	"fmt"
+	"net/http"
+
+	containerderrors "github.com/containerd/containerd/errdefs"
+	"github.com/docker/distribution/registry/api/errcode"
+	"github.com/docker/docker/errdefs"
+	"github.com/sirupsen/logrus"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+type causer interface {
+	Cause() error
+}
+
+// FromError retrieves status code from error message.
+func FromError(err error) int {
+	if err == nil {
+		logrus.WithFields(logrus.Fields{"error": err}).Error("unexpected HTTP error handling")
+		return http.StatusInternalServerError
+	}
+
+	var statusCode int
+
+	// 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) || errdefs.IsDataLoss(err) || errdefs.IsDeadline(err) || errdefs.IsCancelled(err):
+		statusCode = http.StatusInternalServerError
+	default:
+		statusCode = statusCodeFromGRPCError(err)
+		if statusCode != http.StatusInternalServerError {
+			return statusCode
+		}
+		statusCode = statusCodeFromContainerdError(err)
+		if statusCode != http.StatusInternalServerError {
+			return statusCode
+		}
+		statusCode = statusCodeFromDistributionError(err)
+		if statusCode != http.StatusInternalServerError {
+			return statusCode
+		}
+		if e, ok := err.(causer); ok {
+			return FromError(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 {
+		statusCode = http.StatusInternalServerError
+	}
+
+	return statusCode
+}
+
+// statusCodeFromGRPCError returns status code according to gRPC error
+func statusCodeFromGRPCError(err error) int {
+	switch status.Code(err) {
+	case codes.InvalidArgument: // code 3
+		return http.StatusBadRequest
+	case codes.NotFound: // code 5
+		return http.StatusNotFound
+	case codes.AlreadyExists: // code 6
+		return http.StatusConflict
+	case codes.PermissionDenied: // code 7
+		return http.StatusForbidden
+	case codes.FailedPrecondition: // code 9
+		return http.StatusBadRequest
+	case codes.Unauthenticated: // code 16
+		return http.StatusUnauthorized
+	case codes.OutOfRange: // code 11
+		return http.StatusBadRequest
+	case codes.Unimplemented: // code 12
+		return http.StatusNotImplemented
+	case codes.Unavailable: // code 14
+		return http.StatusServiceUnavailable
+	default:
+		// codes.Canceled(1)
+		// codes.Unknown(2)
+		// codes.DeadlineExceeded(4)
+		// codes.ResourceExhausted(8)
+		// codes.Aborted(10)
+		// codes.Internal(13)
+		// codes.DataLoss(15)
+		return http.StatusInternalServerError
+	}
+}
+
+// statusCodeFromDistributionError returns status code according to registry errcode
+// code is loosely based on errcode.ServeJSON() in docker/distribution
+func statusCodeFromDistributionError(err error) int {
+	switch errs := err.(type) {
+	case errcode.Errors:
+		if len(errs) < 1 {
+			return http.StatusInternalServerError
+		}
+		if _, ok := errs[0].(errcode.ErrorCoder); ok {
+			return statusCodeFromDistributionError(errs[0])
+		}
+	case errcode.ErrorCoder:
+		return errs.ErrorCode().Descriptor().HTTPStatusCode
+	}
+	return http.StatusInternalServerError
+}
+
+// statusCodeFromContainerdError returns status code for containerd errors when
+// consumed directly (not through gRPC)
+func statusCodeFromContainerdError(err error) int {
+	switch {
+	case containerderrors.IsInvalidArgument(err):
+		return http.StatusBadRequest
+	case containerderrors.IsNotFound(err):
+		return http.StatusNotFound
+	case containerderrors.IsAlreadyExists(err):
+		return http.StatusConflict
+	case containerderrors.IsFailedPrecondition(err):
+		return http.StatusPreconditionFailed
+	case containerderrors.IsUnavailable(err):
+		return http.StatusServiceUnavailable
+	case containerderrors.IsNotImplemented(err):
+		return http.StatusNotImplemented
+	default:
+		return http.StatusInternalServerError
+	}
+}

api/server/httputils/errors_deprecated.go

@@ -1,9 +0,0 @@
-package httputils // import "github.com/docker/docker/api/server/httputils"
-import "github.com/docker/docker/errdefs"
-
-// GetHTTPErrorStatusCode retrieves status code from error message.
-//
-// Deprecated: use errdefs.GetHTTPErrorStatusCode
-func GetHTTPErrorStatusCode(err error) int {
-	return errdefs.GetHTTPErrorStatusCode(err)
-}

api/server/httputils/httputils.go

@@ -7,13 +7,9 @@ import (
 	"net/http"
 	"strings"
 
-	"github.com/docker/docker/api/types"
-	"github.com/docker/docker/api/types/versions"
 	"github.com/docker/docker/errdefs"
-	"github.com/gorilla/mux"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	"google.golang.org/grpc/status"
 )
 
 // APIVersionKey is the client's requested API version.
@@ -92,28 +88,6 @@ func VersionFromContext(ctx context.Context) string {
 	return ""
 }
 
-// MakeErrorHandler makes an HTTP handler that decodes a Docker error and
-// returns it in the response.
-func MakeErrorHandler(err error) http.HandlerFunc {
-	return func(w http.ResponseWriter, r *http.Request) {
-		statusCode := errdefs.GetHTTPErrorStatusCode(err)
-		vars := mux.Vars(r)
-		if apiVersionSupportsJSONErrors(vars["version"]) {
-			response := &types.ErrorResponse{
-				Message: err.Error(),
-			}
-			_ = WriteJSON(w, statusCode, response)
-		} else {
-			http.Error(w, status.Convert(err).Message(), statusCode)
-		}
-	}
-}
-
-func apiVersionSupportsJSONErrors(version string) bool {
-	const firstAPIVersionWithJSONErrors = "1.23"
-	return version == "" || versions.GreaterThan(version, firstAPIVersionWithJSONErrors)
-}
-
 // matchesContentType validates the content type against the expected one
 func matchesContentType(contentType, expectedType string) bool {
 	mimetype, _, err := mime.ParseMediaType(contentType)

api/server/router/container/container_routes.go

@@ -10,6 +10,7 @@ import (
 	"syscall"
 
 	"github.com/containerd/containerd/platforms"
+	"github.com/docker/docker/api/server/httpstatus"
 	"github.com/docker/docker/api/server/httputils"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/backend"
@@ -626,7 +627,7 @@ func (s *containerRouter) postContainersAttach(ctx context.Context, w http.Respo
 		// Remember to close stream if error happens
 		conn, _, errHijack := hijacker.Hijack()
 		if errHijack == nil {
-			statusCode := errdefs.GetHTTPErrorStatusCode(err)
+			statusCode := httpstatus.FromError(err)
 			statusText := http.StatusText(statusCode)
 			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())
 			httputils.CloseStreams(conn)

api/server/server.go

@@ -7,12 +7,12 @@ import (
 	"net/http"
 	"strings"
 
+	"github.com/docker/docker/api/server/httpstatus"
 	"github.com/docker/docker/api/server/httputils"
 	"github.com/docker/docker/api/server/middleware"
 	"github.com/docker/docker/api/server/router"
 	"github.com/docker/docker/api/server/router/debug"
 	"github.com/docker/docker/dockerversion"
-	"github.com/docker/docker/errdefs"
 	"github.com/gorilla/mux"
 	"github.com/sirupsen/logrus"
 )
@@ -139,11 +139,11 @@ func (s *Server) makeHTTPHandler(handler httputils.APIFunc) http.HandlerFunc {
 		}
 
 		if err := handlerFunc(ctx, w, r, vars); err != nil {
-			statusCode := errdefs.GetHTTPErrorStatusCode(err)
+			statusCode := httpstatus.FromError(err)
 			if statusCode >= 500 {
 				logrus.Errorf("Handler for %s %s returned error: %v", r.Method, r.URL.Path, err)
 			}
-			httputils.MakeErrorHandler(err)(w, r)
+			makeErrorHandler(err)(w, r)
 		}
 	}
 }
@@ -184,7 +184,7 @@ func (s *Server) createMux() *mux.Router {
 		m.Path("/debug" + r.Path()).Handler(f)
 	}
 
-	notFoundHandler := httputils.MakeErrorHandler(pageNotFoundError{})
+	notFoundHandler := makeErrorHandler(pageNotFoundError{})
 	m.HandleFunc(versionMatcher+"/{path:.*}", notFoundHandler)
 	m.NotFoundHandler = notFoundHandler
 	m.MethodNotAllowedHandler = notFoundHandler

errdefs/http_helpers.go

@@ -1,78 +1,11 @@
 package errdefs // import "github.com/docker/docker/errdefs"
 
 import (
-	"fmt"
 	"net/http"
 
-	containerderrors "github.com/containerd/containerd/errdefs"
-	"github.com/docker/distribution/registry/api/errcode"
 	"github.com/sirupsen/logrus"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
-// GetHTTPErrorStatusCode retrieves status code from error message.
-func GetHTTPErrorStatusCode(err error) int {
-	if err == nil {
-		logrus.WithFields(logrus.Fields{"error": err}).Error("unexpected HTTP error handling")
-		return http.StatusInternalServerError
-	}
-
-	var statusCode int
-
-	// 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 IsNotFound(err):
-		statusCode = http.StatusNotFound
-	case IsInvalidParameter(err):
-		statusCode = http.StatusBadRequest
-	case IsConflict(err):
-		statusCode = http.StatusConflict
-	case IsUnauthorized(err):
-		statusCode = http.StatusUnauthorized
-	case IsUnavailable(err):
-		statusCode = http.StatusServiceUnavailable
-	case IsForbidden(err):
-		statusCode = http.StatusForbidden
-	case IsNotModified(err):
-		statusCode = http.StatusNotModified
-	case IsNotImplemented(err):
-		statusCode = http.StatusNotImplemented
-	case IsSystem(err) || IsUnknown(err) || IsDataLoss(err) || IsDeadline(err) || IsCancelled(err):
-		statusCode = http.StatusInternalServerError
-	default:
-		statusCode = statusCodeFromGRPCError(err)
-		if statusCode != http.StatusInternalServerError {
-			return statusCode
-		}
-		statusCode = statusCodeFromContainerdError(err)
-		if statusCode != http.StatusInternalServerError {
-			return statusCode
-		}
-		statusCode = statusCodeFromDistributionError(err)
-		if statusCode != http.StatusInternalServerError {
-			return statusCode
-		}
-		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 {
-		statusCode = http.StatusInternalServerError
-	}
-
-	return statusCode
-}
-
 // FromStatusCode creates an errdef error, based on the provided HTTP status-code
 func FromStatusCode(err error, statusCode int) error {
 	if err == nil {
@@ -100,10 +33,10 @@ func FromStatusCode(err error, statusCode int) error {
 			err = System(err)
 		}
 	default:
-		logrus.WithFields(logrus.Fields{
+		logrus.WithError(err).WithFields(logrus.Fields{
 			"module":      "api",
-			"status_code": fmt.Sprintf("%d", statusCode),
-		}).Debugf("FIXME: Got an status-code for which error does not match any expected type!!!: %d", statusCode)
+			"status_code": statusCode,
+		}).Debug("FIXME: Got an status-code for which error does not match any expected type!!!")
 
 		switch {
 		case statusCode >= 200 && statusCode < 400:
@@ -118,74 +51,3 @@ func FromStatusCode(err error, statusCode int) error {
 	}
 	return err
 }
-
-// statusCodeFromGRPCError returns status code according to gRPC error
-func statusCodeFromGRPCError(err error) int {
-	switch status.Code(err) {
-	case codes.InvalidArgument: // code 3
-		return http.StatusBadRequest
-	case codes.NotFound: // code 5
-		return http.StatusNotFound
-	case codes.AlreadyExists: // code 6
-		return http.StatusConflict
-	case codes.PermissionDenied: // code 7
-		return http.StatusForbidden
-	case codes.FailedPrecondition: // code 9
-		return http.StatusBadRequest
-	case codes.Unauthenticated: // code 16
-		return http.StatusUnauthorized
-	case codes.OutOfRange: // code 11
-		return http.StatusBadRequest
-	case codes.Unimplemented: // code 12
-		return http.StatusNotImplemented
-	case codes.Unavailable: // code 14
-		return http.StatusServiceUnavailable
-	default:
-		// codes.Canceled(1)
-		// codes.Unknown(2)
-		// codes.DeadlineExceeded(4)
-		// codes.ResourceExhausted(8)
-		// codes.Aborted(10)
-		// codes.Internal(13)
-		// codes.DataLoss(15)
-		return http.StatusInternalServerError
-	}
-}
-
-// statusCodeFromDistributionError returns status code according to registry errcode
-// code is loosely based on errcode.ServeJSON() in docker/distribution
-func statusCodeFromDistributionError(err error) int {
-	switch errs := err.(type) {
-	case errcode.Errors:
-		if len(errs) < 1 {
-			return http.StatusInternalServerError
-		}
-		if _, ok := errs[0].(errcode.ErrorCoder); ok {
-			return statusCodeFromDistributionError(errs[0])
-		}
-	case errcode.ErrorCoder:
-		return errs.ErrorCode().Descriptor().HTTPStatusCode
-	}
-	return http.StatusInternalServerError
-}
-
-// statusCodeFromContainerdError returns status code for containerd errors when
-// consumed directly (not through gRPC)
-func statusCodeFromContainerdError(err error) int {
-	switch {
-	case containerderrors.IsInvalidArgument(err):
-		return http.StatusBadRequest
-	case containerderrors.IsNotFound(err):
-		return http.StatusNotFound
-	case containerderrors.IsAlreadyExists(err):
-		return http.StatusConflict
-	case containerderrors.IsFailedPrecondition(err):
-		return http.StatusPreconditionFailed
-	case containerderrors.IsUnavailable(err):
-		return http.StatusServiceUnavailable
-	case containerderrors.IsNotImplemented(err):
-		return http.StatusNotImplemented
-	default:
-		return http.StatusInternalServerError
-	}
-}