From 56349665b758d500eac09798c369b546125b439b Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Wed, 3 Jun 2015 06:52:39 -0700 Subject: [PATCH] Round 4 Signed-off-by: Doug Davis --- docs/api/errcode/errors.go | 242 +++++++++++++++++--------------- docs/api/errcode/errors_test.go | 12 +- docs/api/errcode/register.go | 86 ++++++++++++ docs/api/v2/errors.go | 30 ++-- docs/client/blob_writer_test.go | 5 +- docs/handlers/api_test.go | 12 +- docs/handlers/app.go | 33 +++-- docs/handlers/app_test.go | 8 +- docs/handlers/blob.go | 10 +- docs/handlers/blobupload.go | 50 +++---- docs/handlers/helpers.go | 11 +- docs/handlers/images.go | 26 ++-- docs/handlers/tags.go | 6 +- 13 files changed, 334 insertions(+), 197 deletions(-) create mode 100644 docs/api/errcode/register.go diff --git a/docs/api/errcode/errors.go b/docs/api/errcode/errors.go index 4285dedc..cf186cfb 100644 --- a/docs/api/errcode/errors.go +++ b/docs/api/errcode/errors.go @@ -1,106 +1,29 @@ package errcode import ( + "encoding/json" "fmt" - "net/http" "strings" - "sync" ) +// ErrorCoder is the base interface for ErrorCode and Error allowing +// users of each to just call ErrorCode to get the real ID of each +type ErrorCoder interface { + ErrorCode() ErrorCode +} + // ErrorCode represents the error type. The errors are serialized via strings // and the integer format may change and should *never* be exported. type ErrorCode int -// ErrorDescriptor provides relevant information about a given error code. -type ErrorDescriptor struct { - // Code is the error code that this descriptor describes. - Code ErrorCode - - // Value provides a unique, string key, often captilized with - // underscores, to identify the error code. This value is used as the - // keyed value when serializing api errors. - Value string - - // Message is a short, human readable decription of the error condition - // included in API responses. - Message string - - // Description provides a complete account of the errors purpose, suitable - // for use in documentation. - Description string - - // HTTPStatusCode provides the http status code that is associated with - // this error condition. - HTTPStatusCode int +// ErrorCode just returns itself +func (ec ErrorCode) ErrorCode() ErrorCode { + return ec } -var ( - errorCodeToDescriptors = map[ErrorCode]ErrorDescriptor{} - idToDescriptors = map[string]ErrorDescriptor{} - groupToDescriptors = map[string][]ErrorDescriptor{} -) - -// ErrorCodeUnknown is a generic error that can be used as a last -// resort if there is no situation-specific error message that can be used -var ErrorCodeUnknown = Register("registry.api.errcode", ErrorDescriptor{ - Value: "UNKNOWN", - Message: "unknown error", - Description: `Generic error returned when the error does not have an - API classification.`, - HTTPStatusCode: http.StatusInternalServerError, -}) - -var nextCode = 1000 -var registerLock sync.Mutex - -// Register will make the passed-in error known to the environment and -// return a new ErrorCode -func Register(group string, descriptor ErrorDescriptor) ErrorCode { - registerLock.Lock() - defer registerLock.Unlock() - code := ErrorCode(nextCode) - - descriptor.Code = code - - if _, ok := idToDescriptors[descriptor.Value]; ok { - panic(fmt.Sprintf("ErrorValue %s is already registered", descriptor.Value)) - } - if _, ok := errorCodeToDescriptors[descriptor.Code]; ok { - panic(fmt.Sprintf("ErrorCode %d is already registered", descriptor.Code)) - } - - groupToDescriptors[group] = append(groupToDescriptors[group], descriptor) - errorCodeToDescriptors[code] = descriptor - idToDescriptors[descriptor.Value] = descriptor - - nextCode++ - return code -} - -// ParseErrorCode returns the value by the string error code. -// `ErrorCodeUnknown` will be returned if the error is not known. -func ParseErrorCode(value string) ErrorCode { - ed, ok := idToDescriptors[value] - if ok { - return ed.Code - } - - return ErrorCodeUnknown -} - -// GetGroupNames returns the list of Error group names that are registered -func GetGroupNames() []string { - keys := []string{} - - for k := range groupToDescriptors { - keys = append(keys, k) - } - return keys -} - -// GetErrorCodeGroup returns the named group of error descriptors -func GetErrorCodeGroup(name string) []ErrorDescriptor { - return groupToDescriptors[name] +// Error returns the ID/Value +func (ec ErrorCode) Error() string { + return ec.Descriptor().Value } // Descriptor returns the descriptor for the error code. @@ -143,12 +66,30 @@ func (ec *ErrorCode) UnmarshalText(text []byte) error { return nil } +// WithDetail creates a new Error struct based on the passed-in info and +// set the Detail property appropriately +func (ec ErrorCode) WithDetail(detail interface{}) Error { + if err, ok := detail.(error); ok { + detail = err.Error() + } + + return Error{ + Code: ec, + Detail: detail, + } +} + // Error provides a wrapper around ErrorCode with extra Details provided. type Error struct { Code ErrorCode `json:"code"` Detail interface{} `json:"detail,omitempty"` } +// ErrorCode returns the ID/Value of this Error +func (e Error) ErrorCode() ErrorCode { + return e.Code +} + // Error returns a human readable representation of the error. func (e Error) Error() string { return fmt.Sprintf("%s: %s", @@ -161,30 +102,43 @@ func (e Error) Message() string { return e.Code.Message() } +// ErrorDescriptor provides relevant information about a given error code. +type ErrorDescriptor struct { + // Code is the error code that this descriptor describes. + Code ErrorCode + + // Value provides a unique, string key, often captilized with + // underscores, to identify the error code. This value is used as the + // keyed value when serializing api errors. + Value string + + // Message is a short, human readable decription of the error condition + // included in API responses. + Message string + + // Description provides a complete account of the errors purpose, suitable + // for use in documentation. + Description string + + // HTTPStatusCode provides the http status code that is associated with + // this error condition. + HTTPStatusCode int +} + +// ParseErrorCode returns the value by the string error code. +// `ErrorCodeUnknown` will be returned if the error is not known. +func ParseErrorCode(value string) ErrorCode { + ed, ok := idToDescriptors[value] + if ok { + return ed.Code + } + + return ErrorCodeUnknown +} + // Errors provides the envelope for multiple errors and a few sugar methods // for use within the application. -type Errors []Error - -// NewError creates a new Error struct based on the passed-in info -func NewError(code ErrorCode, details ...interface{}) Error { - if len(details) > 1 { - panic("please specify zero or one detail items for this error") - } - - var detail interface{} - if len(details) > 0 { - detail = details[0] - } - - if err, ok := detail.(error); ok { - detail = err.Error() - } - - return Error{ - Code: code, - Detail: detail, - } -} +type Errors []error func (errs Errors) Error() string { switch len(errs) { @@ -205,3 +159,67 @@ func (errs Errors) Error() string { func (errs Errors) Len() int { return len(errs) } + +// jsonError extends Error with 'Message' so that we can include the +// error text, just in case the receiver of the JSON doesn't have this +// particular ErrorCode registered +type jsonError struct { + Code ErrorCode `json:"code"` + Message string `json:"message"` + Detail interface{} `json:"detail,omitempty"` +} + +// MarshalJSON converts slice of error, ErrorCode or Error into a +// slice of Error - then serializes +func (errs Errors) MarshalJSON() ([]byte, error) { + var tmpErrs []jsonError + + for _, daErr := range errs { + var err Error + + switch daErr.(type) { + case ErrorCode: + err = daErr.(ErrorCode).WithDetail(nil) + case Error: + err = daErr.(Error) + default: + err = ErrorCodeUnknown.WithDetail(daErr) + + } + + tmpErrs = append(tmpErrs, jsonError{ + Code: err.Code, + Message: err.Message(), + Detail: err.Detail, + }) + } + + return json.Marshal(tmpErrs) +} + +// UnmarshalJSON deserializes []Error and then converts it into slice of +// Error or ErrorCode +func (errs *Errors) UnmarshalJSON(data []byte) error { + var tmpErrs []jsonError + + if err := json.Unmarshal(data, &tmpErrs); err != nil { + return err + } + + var newErrs Errors + for _, daErr := range tmpErrs { + if daErr.Detail == nil { + // Error's w/o details get converted to ErrorCode + newErrs = append(newErrs, daErr.Code) + } else { + // Error's w/ details are untouched + newErrs = append(newErrs, Error{ + Code: daErr.Code, + Detail: daErr.Detail, + }) + } + } + + *errs = newErrs + return nil +} diff --git a/docs/api/errcode/errors_test.go b/docs/api/errcode/errors_test.go index aaf0d73b..d89c0253 100644 --- a/docs/api/errcode/errors_test.go +++ b/docs/api/errcode/errors_test.go @@ -79,8 +79,8 @@ var ErrorCodeTest2 = Register("v2.errors", ErrorDescriptor{ func TestErrorsManagement(t *testing.T) { var errs Errors - errs = append(errs, NewError(ErrorCodeTest1)) - errs = append(errs, NewError(ErrorCodeTest2, + errs = append(errs, ErrorCodeTest1) + errs = append(errs, ErrorCodeTest2.WithDetail( map[string]interface{}{"digest": "sometestblobsumdoesntmatter"})) p, err := json.Marshal(errs) @@ -89,10 +89,10 @@ func TestErrorsManagement(t *testing.T) { t.Fatalf("error marashaling errors: %v", err) } - expectedJSON := "[{\"code\":\"TEST1\"},{\"code\":\"TEST2\",\"detail\":{\"digest\":\"sometestblobsumdoesntmatter\"}}]" + expectedJSON := "[{\"code\":\"TEST1\",\"message\":\"test error 1\"},{\"code\":\"TEST2\",\"message\":\"test error 2\",\"detail\":{\"digest\":\"sometestblobsumdoesntmatter\"}}]" if string(p) != expectedJSON { - t.Fatalf("unexpected json: %q != %q", string(p), expectedJSON) + t.Fatalf("unexpected json:\ngot:\n%q\n\nexpected:\n%q", string(p), expectedJSON) } // Now test the reverse @@ -106,8 +106,8 @@ func TestErrorsManagement(t *testing.T) { } // Test again with a single value this time - errs = Errors{NewError(ErrorCodeUnknown)} - expectedJSON = "[{\"code\":\"UNKNOWN\"}]" + errs = Errors{ErrorCodeUnknown} + expectedJSON = "[{\"code\":\"UNKNOWN\",\"message\":\"unknown error\"}]" p, err = json.Marshal(errs) if err != nil { diff --git a/docs/api/errcode/register.go b/docs/api/errcode/register.go new file mode 100644 index 00000000..42f911b3 --- /dev/null +++ b/docs/api/errcode/register.go @@ -0,0 +1,86 @@ +package errcode + +import ( + "fmt" + "net/http" + "sort" + "sync" +) + +var ( + errorCodeToDescriptors = map[ErrorCode]ErrorDescriptor{} + idToDescriptors = map[string]ErrorDescriptor{} + groupToDescriptors = map[string][]ErrorDescriptor{} +) + +// ErrorCodeUnknown is a generic error that can be used as a last +// resort if there is no situation-specific error message that can be used +var ErrorCodeUnknown = Register("errcode", ErrorDescriptor{ + Value: "UNKNOWN", + Message: "unknown error", + Description: `Generic error returned when the error does not have an + API classification.`, + HTTPStatusCode: http.StatusInternalServerError, +}) + +var nextCode = 1000 +var registerLock sync.Mutex + +// Register will make the passed-in error known to the environment and +// return a new ErrorCode +func Register(group string, descriptor ErrorDescriptor) ErrorCode { + registerLock.Lock() + defer registerLock.Unlock() + + descriptor.Code = ErrorCode(nextCode) + + if _, ok := idToDescriptors[descriptor.Value]; ok { + panic(fmt.Sprintf("ErrorValue %q is already registered", descriptor.Value)) + } + if _, ok := errorCodeToDescriptors[descriptor.Code]; ok { + panic(fmt.Sprintf("ErrorCode %v is already registered", descriptor.Code)) + } + + groupToDescriptors[group] = append(groupToDescriptors[group], descriptor) + errorCodeToDescriptors[descriptor.Code] = descriptor + idToDescriptors[descriptor.Value] = descriptor + + nextCode++ + return descriptor.Code +} + +type byValue []ErrorDescriptor + +func (a byValue) Len() int { return len(a) } +func (a byValue) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a byValue) Less(i, j int) bool { return a[i].Value < a[j].Value } + +// GetGroupNames returns the list of Error group names that are registered +func GetGroupNames() []string { + keys := []string{} + + for k := range groupToDescriptors { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} + +// GetErrorCodeGroup returns the named group of error descriptors +func GetErrorCodeGroup(name string) []ErrorDescriptor { + desc := groupToDescriptors[name] + sort.Sort(byValue(desc)) + return desc +} + +// GetErrorAllDescriptors returns a slice of all ErrorDescriptors that are +// registered, irrespective of what group they're in +func GetErrorAllDescriptors() []ErrorDescriptor { + result := []ErrorDescriptor{} + + for _, group := range GetGroupNames() { + result = append(result, GetErrorCodeGroup(group)...) + } + sort.Sort(byValue(result)) + return result +} diff --git a/docs/api/v2/errors.go b/docs/api/v2/errors.go index c12cbc1c..14684560 100644 --- a/docs/api/v2/errors.go +++ b/docs/api/v2/errors.go @@ -6,9 +6,11 @@ import ( "github.com/docker/distribution/registry/api/errcode" ) +const errGroup = "registry.api.v2" + var ( // ErrorCodeUnsupported is returned when an operation is not supported. - ErrorCodeUnsupported = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeUnsupported = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "UNSUPPORTED", Message: "The operation is unsupported.", Description: `The operation was unsupported due to a missing @@ -16,7 +18,7 @@ var ( }) // ErrorCodeUnauthorized is returned if a request is not authorized. - ErrorCodeUnauthorized = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeUnauthorized = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "UNAUTHORIZED", Message: "access to the requested resource is not authorized", Description: `The access controller denied access for the operation on @@ -27,7 +29,7 @@ var ( // ErrorCodeDigestInvalid is returned when uploading a blob if the // provided digest does not match the blob contents. - ErrorCodeDigestInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeDigestInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "DIGEST_INVALID", Message: "provided digest did not match uploaded content", Description: `When a blob is uploaded, the registry will check that @@ -39,7 +41,7 @@ var ( }) // ErrorCodeSizeInvalid is returned when uploading a blob if the provided - ErrorCodeSizeInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeSizeInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "SIZE_INVALID", Message: "provided length did not match content length", Description: `When a layer is uploaded, the provided size will be @@ -50,7 +52,7 @@ var ( // ErrorCodeNameInvalid is returned when the name in the manifest does not // match the provided name. - ErrorCodeNameInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeNameInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "NAME_INVALID", Message: "invalid repository name", Description: `Invalid repository name encountered either during @@ -60,7 +62,7 @@ var ( // ErrorCodeTagInvalid is returned when the tag in the manifest does not // match the provided tag. - ErrorCodeTagInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeTagInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "TAG_INVALID", Message: "manifest tag did not match URI", Description: `During a manifest upload, if the tag in the manifest @@ -69,7 +71,7 @@ var ( }) // ErrorCodeNameUnknown when the repository name is not known. - ErrorCodeNameUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeNameUnknown = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "NAME_UNKNOWN", Message: "repository name not known to registry", Description: `This is returned if the name used during an operation is @@ -78,7 +80,7 @@ var ( }) // ErrorCodeManifestUnknown returned when image manifest is unknown. - ErrorCodeManifestUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeManifestUnknown = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "MANIFEST_UNKNOWN", Message: "manifest unknown", Description: `This error is returned when the manifest, identified by @@ -89,7 +91,7 @@ var ( // ErrorCodeManifestInvalid returned when an image manifest is invalid, // typically during a PUT operation. This error encompasses all errors // encountered during manifest validation that aren't signature errors. - ErrorCodeManifestInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeManifestInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "MANIFEST_INVALID", Message: "manifest invalid", Description: `During upload, manifests undergo several checks ensuring @@ -101,7 +103,7 @@ var ( // ErrorCodeManifestUnverified is returned when the manifest fails // signature verfication. - ErrorCodeManifestUnverified = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeManifestUnverified = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "MANIFEST_UNVERIFIED", Message: "manifest failed signature verification", Description: `During manifest upload, if the manifest fails signature @@ -111,7 +113,7 @@ var ( // ErrorCodeManifestBlobUnknown is returned when a manifest blob is // unknown to the registry. - ErrorCodeManifestBlobUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeManifestBlobUnknown = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "MANIFEST_BLOB_UNKNOWN", Message: "blob unknown to registry", Description: `This error may be returned when a manifest blob is @@ -122,7 +124,7 @@ var ( // ErrorCodeBlobUnknown is returned when a blob is unknown to the // registry. This can happen when the manifest references a nonexistent // layer or the result is not found by a blob fetch. - ErrorCodeBlobUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeBlobUnknown = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "BLOB_UNKNOWN", Message: "blob unknown to registry", Description: `This error may be returned when a blob is unknown to the @@ -133,7 +135,7 @@ var ( }) // ErrorCodeBlobUploadUnknown is returned when an upload is unknown. - ErrorCodeBlobUploadUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeBlobUploadUnknown = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "BLOB_UPLOAD_UNKNOWN", Message: "blob upload unknown to registry", Description: `If a blob upload has been cancelled or was never @@ -142,7 +144,7 @@ var ( }) // ErrorCodeBlobUploadInvalid is returned when an upload is invalid. - ErrorCodeBlobUploadInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeBlobUploadInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "BLOB_UPLOAD_INVALID", Message: "blob upload invalid", Description: `The blob upload encountered an error and can no diff --git a/docs/client/blob_writer_test.go b/docs/client/blob_writer_test.go index 74545b06..eeb9f53d 100644 --- a/docs/client/blob_writer_test.go +++ b/docs/client/blob_writer_test.go @@ -164,7 +164,10 @@ func TestUploadReadFrom(t *testing.T) { } else if len(uploadErr) != 1 { t.Fatalf("Unexpected number of errors: %d, expected 1", len(uploadErr)) } else { - v2Err := uploadErr[0] + v2Err, ok := uploadErr[0].(errcode.Error) + if !ok { + t.Fatalf("Not an 'Error' type: %#v", uploadErr[0]) + } if v2Err.Code != v2.ErrorCodeBlobUploadInvalid { t.Fatalf("Unexpected error code: %s, expected %d", v2Err.Code.String(), v2.ErrorCodeBlobUploadInvalid) } diff --git a/docs/handlers/api_test.go b/docs/handlers/api_test.go index 146fcf4c..9952d68e 100644 --- a/docs/handlers/api_test.go +++ b/docs/handlers/api_test.go @@ -780,11 +780,15 @@ func checkBodyHasErrorCodes(t *testing.T, msg string, resp *http.Response, error counts[code] = 0 } - for _, err := range errs { - if _, ok := expected[err.Code]; !ok { - t.Fatalf("unexpected error code %v encountered during %s: %s ", err.Code, msg, string(p)) + for _, e := range errs { + err, ok := e.(errcode.ErrorCoder) + if !ok { + t.Fatalf("not an ErrorCoder: %#v", e) } - counts[err.Code]++ + if _, ok := expected[err.ErrorCode()]; !ok { + t.Fatalf("unexpected error code %v encountered during %s: %s ", err.ErrorCode(), msg, string(p)) + } + counts[err.ErrorCode()]++ } // Ensure that counts of expected errors were all non-zero diff --git a/docs/handlers/app.go b/docs/handlers/app.go index 0ef7d4ca..83b231af 100644 --- a/docs/handlers/app.go +++ b/docs/handlers/app.go @@ -346,9 +346,9 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { switch err := err.(type) { case distribution.ErrRepositoryUnknown: - context.Errors = append(context.Errors, errcode.NewError(v2.ErrorCodeNameUnknown, err)) + context.Errors = append(context.Errors, v2.ErrorCodeNameUnknown.WithDetail(err)) case distribution.ErrRepositoryNameInvalid: - context.Errors = append(context.Errors, errcode.NewError(v2.ErrorCodeNameInvalid, err)) + context.Errors = append(context.Errors, v2.ErrorCodeNameInvalid.WithDetail(err)) } serveJSON(w, context.Errors) @@ -363,7 +363,7 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { context.Repository, err = applyRepoMiddleware(context.Repository, app.Config.Middleware["repository"]) if err != nil { ctxu.GetLogger(context).Errorf("error initializing repository middleware: %v", err) - context.Errors = append(context.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + context.Errors = append(context.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) serveJSON(w, context.Errors) return @@ -383,10 +383,25 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { } func (app *App) logError(context context.Context, errors errcode.Errors) { - for _, e := range errors { - c := ctxu.WithValue(context, "err.code", e.Code) - c = ctxu.WithValue(c, "err.message", e.Code.Message()) - c = ctxu.WithValue(c, "err.detail", e.Detail) + for _, e1 := range errors { + var c ctxu.Context + + switch e1.(type) { + case errcode.Error: + e, _ := e1.(errcode.Error) + c = ctxu.WithValue(context, "err.code", e.Code) + c = ctxu.WithValue(c, "err.message", e.Code.Message()) + c = ctxu.WithValue(c, "err.detail", e.Detail) + case errcode.ErrorCode: + e, _ := e1.(errcode.ErrorCode) + c = ctxu.WithValue(context, "err.code", e) + c = ctxu.WithValue(c, "err.message", e.Message()) + default: + // just normal go 'error' + c = ctxu.WithValue(context, "err.code", errcode.ErrorCodeUnknown) + c = ctxu.WithValue(c, "err.message", e1.Error()) + } + c = ctxu.WithLogger(c, ctxu.GetLogger(c, "err.code", "err.message", @@ -441,7 +456,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont // proceed. var errs errcode.Errors - errs = append(errs, errcode.NewError(v2.ErrorCodeUnauthorized)) + errs = append(errs, v2.ErrorCodeUnauthorized) serveJSON(w, errs) return fmt.Errorf("forbidden: no repository name") @@ -465,7 +480,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont err.ServeHTTP(w, r) var errs errcode.Errors - errs = append(errs, errcode.NewError(v2.ErrorCodeUnauthorized, accessRecords)) + errs = append(errs, v2.ErrorCodeUnauthorized.WithDetail(accessRecords)) serveJSON(w, errs) default: // This condition is a potential security problem either in diff --git a/docs/handlers/app_test.go b/docs/handlers/app_test.go index d98ae400..98ecaefd 100644 --- a/docs/handlers/app_test.go +++ b/docs/handlers/app_test.go @@ -201,8 +201,12 @@ func TestNewApp(t *testing.T) { t.Fatalf("error decoding error response: %v", err) } - if errs[0].Code != v2.ErrorCodeUnauthorized { - t.Fatalf("unexpected error code: %v != %v", errs[0].Code, v2.ErrorCodeUnauthorized) + err2, ok := errs[0].(errcode.ErrorCoder) + if !ok { + t.Fatalf("not an ErrorCoder: %#v", errs[0]) + } + if err2.ErrorCode() != v2.ErrorCodeUnauthorized { + t.Fatalf("unexpected error code: %v != %v", err2.ErrorCode(), v2.ErrorCodeUnauthorized) } } diff --git a/docs/handlers/blob.go b/docs/handlers/blob.go index fa9f576a..e33bd3c0 100644 --- a/docs/handlers/blob.go +++ b/docs/handlers/blob.go @@ -18,12 +18,12 @@ func blobDispatcher(ctx *Context, r *http.Request) http.Handler { if err == errDigestNotAvailable { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx.Errors = append(ctx.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) + ctx.Errors = append(ctx.Errors, v2.ErrorCodeDigestInvalid.WithDetail(err)) }) } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx.Errors = append(ctx.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) + ctx.Errors = append(ctx.Errors, v2.ErrorCodeDigestInvalid.WithDetail(err)) }) } @@ -53,16 +53,16 @@ func (bh *blobHandler) GetBlob(w http.ResponseWriter, r *http.Request) { desc, err := blobs.Stat(bh, bh.Digest) if err != nil { if err == distribution.ErrBlobUnknown { - bh.Errors = append(bh.Errors, errcode.NewError(v2.ErrorCodeBlobUnknown, bh.Digest)) + bh.Errors = append(bh.Errors, v2.ErrorCodeBlobUnknown.WithDetail(bh.Digest)) } else { - bh.Errors = append(bh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + bh.Errors = append(bh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) } return } if err := blobs.ServeBlob(bh, w, r, desc.Digest); err != nil { context.GetLogger(bh).Debugf("unexpected error getting blob HTTP handler: %v", err) - bh.Errors = append(bh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + bh.Errors = append(bh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } } diff --git a/docs/handlers/blobupload.go b/docs/handlers/blobupload.go index 7e8c3962..8dc417ba 100644 --- a/docs/handlers/blobupload.go +++ b/docs/handlers/blobupload.go @@ -37,7 +37,7 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { if err != nil { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("error resolving upload: %v", err) - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) }) } buh.State = state @@ -45,14 +45,14 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { if state.Name != ctx.Repository.Name() { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("mismatched repository name in upload state: %q != %q", state.Name, buh.Repository.Name()) - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) }) } if state.UUID != buh.UUID { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("mismatched uuid in upload state: %q != %q", state.UUID, buh.UUID) - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) }) } @@ -62,12 +62,12 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { ctxu.GetLogger(ctx).Errorf("error resolving upload: %v", err) if err == distribution.ErrBlobUploadUnknown { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown.WithDetail(err)) }) } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) }) } buh.Upload = upload @@ -81,14 +81,14 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { defer upload.Close() ctxu.GetLogger(ctx).Infof("error seeking blob upload: %v", err) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) upload.Cancel(buh) }) } else if nn != buh.State.Offset { defer upload.Close() ctxu.GetLogger(ctx).Infof("seek to wrong offest: %d != %d", nn, buh.State.Offset) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) upload.Cancel(buh) }) } @@ -119,7 +119,7 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req blobs := buh.Repository.Blobs(buh) upload, err := blobs.Create(buh) if err != nil { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -127,7 +127,7 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req defer buh.Upload.Close() if err := buh.blobUploadResponse(w, r, true); err != nil { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -138,7 +138,7 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req // GetUploadStatus returns the status of a given upload, identified by id. func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown) return } @@ -146,7 +146,7 @@ func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Req // resumable upload is supported. This will enable returning a non-zero // range for clients to begin uploading at an offset. if err := buh.blobUploadResponse(w, r, true); err != nil { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -157,13 +157,13 @@ func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Req // PatchBlobData writes data to an upload. func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown) return } ct := r.Header.Get("Content-Type") if ct != "" && ct != "application/octet-stream" { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, fmt.Errorf("Bad Content-Type"))) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(fmt.Errorf("Bad Content-Type"))) // TODO(dmcgowan): encode error return } @@ -173,12 +173,12 @@ func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Reque // Copy the data if _, err := io.Copy(buh.Upload, r.Body); err != nil { ctxu.GetLogger(buh).Errorf("unknown error copying into upload: %v", err) - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } if err := buh.blobUploadResponse(w, r, false); err != nil { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -192,7 +192,7 @@ func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Reque // url of the blob. func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown) return } @@ -200,21 +200,21 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht if dgstStr == "" { // no digest? return error, but allow retry. - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, "digest missing")) + buh.Errors = append(buh.Errors, v2.ErrorCodeDigestInvalid.WithDetail("digest missing")) return } dgst, err := digest.ParseDigest(dgstStr) if err != nil { // no digest? return error, but allow retry. - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, "digest parsing failed")) + buh.Errors = append(buh.Errors, v2.ErrorCodeDigestInvalid.WithDetail("digest parsing failed")) return } // Read in the data, if any. if _, err := io.Copy(buh.Upload, r.Body); err != nil { ctxu.GetLogger(buh).Errorf("unknown error copying into upload: %v", err) - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -229,14 +229,14 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht if err != nil { switch err := err.(type) { case distribution.ErrBlobInvalidDigest: - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeDigestInvalid.WithDetail(err)) default: switch err { case distribution.ErrBlobInvalidLength, distribution.ErrBlobDigestUnsupported: - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) default: ctxu.GetLogger(buh).Errorf("unknown error completing upload: %#v", err) - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) } } @@ -253,7 +253,7 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht // Build our canonical blob url blobURL, err := buh.urlBuilder.BuildBlobURL(buh.Repository.Name(), desc.Digest) if err != nil { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -266,14 +266,14 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht // CancelBlobUpload cancels an in-progress upload of a blob. func (buh *blobUploadHandler) CancelBlobUpload(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown) return } w.Header().Set("Docker-Upload-UUID", buh.UUID) if err := buh.Upload.Cancel(buh); err != nil { ctxu.GetLogger(buh).Errorf("error encountered canceling upload: %v", err) - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) } w.WriteHeader(http.StatusNoContent) diff --git a/docs/handlers/helpers.go b/docs/handlers/helpers.go index 656d2066..c72c5784 100644 --- a/docs/handlers/helpers.go +++ b/docs/handlers/helpers.go @@ -16,9 +16,14 @@ func serveJSON(w http.ResponseWriter, v interface{}) error { sc := http.StatusInternalServerError if errs, ok := v.(errcode.Errors); ok && len(errs) > 0 { - sc = errs[0].Code.Descriptor().HTTPStatusCode - if sc == 0 { - sc = http.StatusInternalServerError + 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 } } diff --git a/docs/handlers/images.go b/docs/handlers/images.go index 9d025c78..41fbabc4 100644 --- a/docs/handlers/images.go +++ b/docs/handlers/images.go @@ -64,7 +64,7 @@ func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http } if err != nil { - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeManifestUnknown, err)) + imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown.WithDetail(err)) return } @@ -72,7 +72,7 @@ func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http if imh.Digest == "" { dgst, err := digestManifest(imh, sm) if err != nil { - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) + imh.Errors = append(imh.Errors, v2.ErrorCodeDigestInvalid.WithDetail(err)) return } @@ -93,13 +93,13 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http var manifest manifest.SignedManifest if err := dec.Decode(&manifest); err != nil { - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeManifestInvalid, err)) + imh.Errors = append(imh.Errors, v2.ErrorCodeManifestInvalid.WithDetail(err)) return } dgst, err := digestManifest(imh, &manifest) if err != nil { - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) + imh.Errors = append(imh.Errors, v2.ErrorCodeDigestInvalid.WithDetail(err)) return } @@ -107,7 +107,7 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http if imh.Tag != "" { if manifest.Tag != imh.Tag { ctxu.GetLogger(imh).Errorf("invalid tag on manifest payload: %q != %q", manifest.Tag, imh.Tag) - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeTagInvalid)) + imh.Errors = append(imh.Errors, v2.ErrorCodeTagInvalid) return } @@ -115,11 +115,11 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http } else if imh.Digest != "" { if dgst != imh.Digest { ctxu.GetLogger(imh).Errorf("payload digest does match: %q != %q", dgst, imh.Digest) - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid)) + imh.Errors = append(imh.Errors, v2.ErrorCodeDigestInvalid) return } } else { - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeTagInvalid, "no tag or digest specified")) + imh.Errors = append(imh.Errors, v2.ErrorCodeTagInvalid.WithDetail("no tag or digest specified")) return } @@ -131,19 +131,19 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http for _, verificationError := range err { switch verificationError := verificationError.(type) { case distribution.ErrManifestBlobUnknown: - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeBlobUnknown, verificationError.Digest)) + imh.Errors = append(imh.Errors, v2.ErrorCodeBlobUnknown.WithDetail(verificationError.Digest)) case distribution.ErrManifestUnverified: - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeManifestUnverified)) + imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnverified) default: if verificationError == digest.ErrDigestInvalidFormat { - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid)) + imh.Errors = append(imh.Errors, v2.ErrorCodeDigestInvalid) } else { - imh.Errors = append(imh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, verificationError)) + imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown, verificationError) } } } default: - imh.Errors = append(imh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) } return @@ -172,7 +172,7 @@ func (imh *imageManifestHandler) DeleteImageManifest(w http.ResponseWriter, r *h // tag index entries a serious problem in eventually consistent storage. // Once we work out schema version 2, the full deletion system will be // worked out and we can add support back. - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeUnsupported)) + imh.Errors = append(imh.Errors, v2.ErrorCodeUnsupported) } // digestManifest takes a digest of the given manifest. This belongs somewhere diff --git a/docs/handlers/tags.go b/docs/handlers/tags.go index e1846cf9..00f9760e 100644 --- a/docs/handlers/tags.go +++ b/docs/handlers/tags.go @@ -40,9 +40,9 @@ func (th *tagsHandler) GetTags(w http.ResponseWriter, r *http.Request) { if err != nil { switch err := err.(type) { case distribution.ErrRepositoryUnknown: - th.Errors = append(th.Errors, errcode.NewError(v2.ErrorCodeNameUnknown, map[string]string{"name": th.Repository.Name()})) + th.Errors = append(th.Errors, v2.ErrorCodeNameUnknown.WithDetail(map[string]string{"name": th.Repository.Name()})) default: - th.Errors = append(th.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + th.Errors = append(th.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) } return } @@ -54,7 +54,7 @@ func (th *tagsHandler) GetTags(w http.ResponseWriter, r *http.Request) { Name: th.Repository.Name(), Tags: tags, }); err != nil { - th.Errors = append(th.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + th.Errors = append(th.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } }