From 86ffe515df18746ae5e5796d7efae065cc2e4c33 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 16 Jul 2015 11:35:02 -0700 Subject: [PATCH] Export ServeJSON for serving error codes This changeset provides a common http handler for serving errcodes. This should unify http responses across webservices in the face of errors. Several type assertions have been added, as well, to ensure the error interface is implemented. Signed-off-by: Stephen J Day --- registry/api/errcode/errors.go | 6 +++++ registry/api/errcode/handler.go | 44 +++++++++++++++++++++++++++++++++ registry/handlers/app.go | 26 ++++++++++--------- registry/handlers/helpers.go | 33 ------------------------- 4 files changed, 65 insertions(+), 44 deletions(-) create mode 100644 registry/api/errcode/handler.go diff --git a/registry/api/errcode/errors.go b/registry/api/errcode/errors.go index d221cb67..acdeb022 100644 --- a/registry/api/errcode/errors.go +++ b/registry/api/errcode/errors.go @@ -16,6 +16,8 @@ type ErrorCoder interface { // and the integer format may change and should *never* be exported. type ErrorCode int +var _ error = ErrorCode(0) + // ErrorCode just returns itself func (ec ErrorCode) ErrorCode() ErrorCode { return ec @@ -93,6 +95,8 @@ type Error struct { // variable substitution right before showing the message to the user } +var _ error = Error{} + // ErrorCode returns the ID/Value of this Error func (e Error) ErrorCode() ErrorCode { return e.Code @@ -163,6 +167,8 @@ func ParseErrorCode(value string) ErrorCode { // for use within the application. type Errors []error +var _ error = Errors{} + func (errs Errors) Error() string { switch len(errs) { case 0: diff --git a/registry/api/errcode/handler.go b/registry/api/errcode/handler.go new file mode 100644 index 00000000..49a64a86 --- /dev/null +++ b/registry/api/errcode/handler.go @@ -0,0 +1,44 @@ +package errcode + +import ( + "encoding/json" + "net/http" +) + +// ServeJSON attempts to serve the errcode in a JSON envelope. It marshals err +// and sets the content-type header to 'application/json'. It will handle +// ErrorCoder and Errors, and if necessary will create an envelope. +func ServeJSON(w http.ResponseWriter, err error) error { + w.Header().Set("Content-Type", "application/json; charset=utf-8") + var sc int + + switch errs := err.(type) { + case Errors: + if len(errs) < 1 { + break + } + + if err, ok := errs[0].(ErrorCoder); ok { + sc = err.ErrorCode().Descriptor().HTTPStatusCode + } + case ErrorCoder: + sc = errs.ErrorCode().Descriptor().HTTPStatusCode + err = Errors{err} // create an envelope. + default: + // We just have an unhandled error type, so just place in an envelope + // and move along. + err = Errors{err} + } + + if sc == 0 { + sc = http.StatusInternalServerError + } + + w.WriteHeader(sc) + + if err := json.NewEncoder(w).Encode(err); err != nil { + return err + } + + return nil +} diff --git a/registry/handlers/app.go b/registry/handlers/app.go index d3985067..c8c52362 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -379,7 +379,9 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { context.Errors = append(context.Errors, v2.ErrorCodeNameInvalid.WithDetail(err)) } - serveJSON(w, context.Errors) + if err := errcode.ServeJSON(w, context.Errors); err != nil { + ctxu.GetLogger(context).Errorf("error serving error json: %v (from %v)", err, context.Errors) + } return } @@ -393,7 +395,9 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { ctxu.GetLogger(context).Errorf("error initializing repository middleware: %v", err) context.Errors = append(context.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) - serveJSON(w, context.Errors) + if err := errcode.ServeJSON(w, context.Errors); err != nil { + ctxu.GetLogger(context).Errorf("error serving error json: %v (from %v)", err, context.Errors) + } return } } @@ -405,7 +409,9 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { if context.Errors.Len() > 0 { app.logError(context, context.Errors) - serveJSON(w, context.Errors) + if err := errcode.ServeJSON(w, context.Errors); err != nil { + ctxu.GetLogger(context).Errorf("error serving error json: %v (from %v)", err, context.Errors) + } } }) } @@ -482,11 +488,9 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont // base route is accessed. This section prevents us from making // that mistake elsewhere in the code, allowing any operation to // proceed. - - var errs errcode.Errors - errs = append(errs, v2.ErrorCodeUnauthorized) - - serveJSON(w, errs) + if err := errcode.ServeJSON(w, v2.ErrorCodeUnauthorized); err != nil { + ctxu.GetLogger(context).Errorf("error serving error json: %v (from %v)", err, context.Errors) + } return fmt.Errorf("forbidden: no repository name") } } @@ -498,9 +502,9 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont // Add the appropriate WWW-Auth header err.ServeHTTP(w, r) - var errs errcode.Errors - errs = append(errs, v2.ErrorCodeUnauthorized.WithDetail(accessRecords)) - serveJSON(w, errs) + if err := errcode.ServeJSON(w, v2.ErrorCodeUnauthorized.WithDetail(accessRecords)); err != nil { + ctxu.GetLogger(context).Errorf("error serving error json: %v (from %v)", err, context.Errors) + } default: // This condition is a potential security problem either in // the configuration or whatever is backing the access diff --git a/registry/handlers/helpers.go b/registry/handlers/helpers.go index c72c5784..e2d220c2 100644 --- a/registry/handlers/helpers.go +++ b/registry/handlers/helpers.go @@ -1,43 +1,10 @@ package handlers import ( - "encoding/json" "io" "net/http" - - "github.com/docker/distribution/registry/api/errcode" ) -// serveJSON marshals v and sets the content-type header to -// 'application/json'. If a different status code is required, call -// ResponseWriter.WriteHeader before this function. -func serveJSON(w http.ResponseWriter, v interface{}) error { - w.Header().Set("Content-Type", "application/json; charset=utf-8") - sc := http.StatusInternalServerError - - if errs, ok := v.(errcode.Errors); ok && len(errs) > 0 { - if err, ok := errs[0].(errcode.ErrorCoder); ok { - if sc2 := err.ErrorCode().Descriptor().HTTPStatusCode; sc2 != 0 { - sc = sc2 - } - } - } else if err, ok := v.(errcode.ErrorCoder); ok { - if sc2 := err.ErrorCode().Descriptor().HTTPStatusCode; sc2 != 0 { - sc = sc2 - } - } - - w.WriteHeader(sc) - - enc := json.NewEncoder(w) - - if err := enc.Encode(v); err != nil { - return err - } - - return nil -} - // closeResources closes all the provided resources after running the target // handler. func closeResources(handler http.Handler, closers ...io.Closer) http.Handler {