From 7840a5bc8f4991b1a9f1b7c9304b0212d21073f4 Mon Sep 17 00:00:00 2001 From: amitshukla Date: Fri, 2 Oct 2015 16:19:06 -0700 Subject: [PATCH 1/2] Fix for issue 664: https://github.com/docker/distribution/issues/664 Errors thrown by storage drivers don't have the name of the driver, causing user confusion about whether the error is coming from Docker or from a storage driver. This change adds the storage driver name to each error message. This required changing ErrUnsupportedDriver to a type, leading to code changes whenever ErrUnsupportedDriver is used. The tests check whether the driver name appears in the error message. Signed-off-by: Amit Shukla --- docs/storage/blobserver.go | 7 +- docs/storage/driver/base/base.go | 71 ++++++++++++++------ docs/storage/driver/filesystem/driver.go | 2 +- docs/storage/driver/inmemory/driver.go | 2 +- docs/storage/driver/oss/oss.go | 2 +- docs/storage/driver/rados/rados.go | 2 +- docs/storage/driver/s3/s3.go | 2 +- docs/storage/driver/storagedriver.go | 26 ++++--- docs/storage/driver/swift/swift.go | 6 +- docs/storage/driver/testsuites/testsuites.go | 23 ++++++- 10 files changed, 99 insertions(+), 44 deletions(-) diff --git a/docs/storage/blobserver.go b/docs/storage/blobserver.go index 24aeba69..2d89ecd8 100644 --- a/docs/storage/blobserver.go +++ b/docs/storage/blobserver.go @@ -36,16 +36,15 @@ func (bs *blobServer) ServeBlob(ctx context.Context, w http.ResponseWriter, r *h redirectURL, err := bs.driver.URLFor(ctx, path, map[string]interface{}{"method": r.Method}) - switch err { - case nil: + if err == nil { if bs.redirect { // Redirect to storage URL. http.Redirect(w, r, redirectURL, http.StatusTemporaryRedirect) return err } + } - fallthrough - case driver.ErrUnsupportedMethod: + if _, ok := err.(*driver.ErrUnsupportedMethod); ok { // Fallback to serving the content directly. br, err := newFileReader(ctx, bs.driver, path, desc.Size) if err != nil { diff --git a/docs/storage/driver/base/base.go b/docs/storage/driver/base/base.go index 60af06b8..2333bba7 100644 --- a/docs/storage/driver/base/base.go +++ b/docs/storage/driver/base/base.go @@ -50,16 +50,40 @@ type Base struct { storagedriver.StorageDriver } +// Format errors received from the storage driver +func (base *Base) setDriverName(e error) error { + if e != nil { + if actualErr, ok := e.(storagedriver.ErrUnsupportedMethod); ok { + actualErr.DriverName = base.StorageDriver.Name() + return actualErr + } + if actualErr, ok := e.(storagedriver.PathNotFoundError); ok { + actualErr.DriverName = base.StorageDriver.Name() + return actualErr + } + if actualErr, ok := e.(storagedriver.InvalidPathError); ok { + actualErr.DriverName = base.StorageDriver.Name() + return actualErr + } + if actualErr, ok := e.(storagedriver.InvalidOffsetError); ok { + actualErr.DriverName = base.StorageDriver.Name() + return actualErr + } + } + return e +} + // GetContent wraps GetContent of underlying storage driver. func (base *Base) GetContent(ctx context.Context, path string) ([]byte, error) { ctx, done := context.WithTrace(ctx) defer done("%s.GetContent(%q)", base.Name(), path) if !storagedriver.PathRegexp.MatchString(path) { - return nil, storagedriver.InvalidPathError{Path: path} + return nil, storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.GetContent(ctx, path) + b, e := base.StorageDriver.GetContent(ctx, path) + return b, base.setDriverName(e) } // PutContent wraps PutContent of underlying storage driver. @@ -68,10 +92,10 @@ func (base *Base) PutContent(ctx context.Context, path string, content []byte) e defer done("%s.PutContent(%q)", base.Name(), path) if !storagedriver.PathRegexp.MatchString(path) { - return storagedriver.InvalidPathError{Path: path} + return storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.PutContent(ctx, path, content) + return base.setDriverName(base.StorageDriver.PutContent(ctx, path, content)) } // ReadStream wraps ReadStream of underlying storage driver. @@ -80,14 +104,15 @@ func (base *Base) ReadStream(ctx context.Context, path string, offset int64) (io defer done("%s.ReadStream(%q, %d)", base.Name(), path, offset) if offset < 0 { - return nil, storagedriver.InvalidOffsetError{Path: path, Offset: offset} + return nil, storagedriver.InvalidOffsetError{Path: path, Offset: offset, DriverName: base.StorageDriver.Name()} } if !storagedriver.PathRegexp.MatchString(path) { - return nil, storagedriver.InvalidPathError{Path: path} + return nil, storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.ReadStream(ctx, path, offset) + rc, e := base.StorageDriver.ReadStream(ctx, path, offset) + return rc, base.setDriverName(e) } // WriteStream wraps WriteStream of underlying storage driver. @@ -96,14 +121,15 @@ func (base *Base) WriteStream(ctx context.Context, path string, offset int64, re defer done("%s.WriteStream(%q, %d)", base.Name(), path, offset) if offset < 0 { - return 0, storagedriver.InvalidOffsetError{Path: path, Offset: offset} + return 0, storagedriver.InvalidOffsetError{Path: path, Offset: offset, DriverName: base.StorageDriver.Name()} } if !storagedriver.PathRegexp.MatchString(path) { - return 0, storagedriver.InvalidPathError{Path: path} + return 0, storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.WriteStream(ctx, path, offset, reader) + i64, e := base.StorageDriver.WriteStream(ctx, path, offset, reader) + return i64, base.setDriverName(e) } // Stat wraps Stat of underlying storage driver. @@ -112,10 +138,11 @@ func (base *Base) Stat(ctx context.Context, path string) (storagedriver.FileInfo defer done("%s.Stat(%q)", base.Name(), path) if !storagedriver.PathRegexp.MatchString(path) { - return nil, storagedriver.InvalidPathError{Path: path} + return nil, storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.Stat(ctx, path) + fi, e := base.StorageDriver.Stat(ctx, path) + return fi, base.setDriverName(e) } // List wraps List of underlying storage driver. @@ -124,10 +151,11 @@ func (base *Base) List(ctx context.Context, path string) ([]string, error) { defer done("%s.List(%q)", base.Name(), path) if !storagedriver.PathRegexp.MatchString(path) && path != "/" { - return nil, storagedriver.InvalidPathError{Path: path} + return nil, storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.List(ctx, path) + str, e := base.StorageDriver.List(ctx, path) + return str, base.setDriverName(e) } // Move wraps Move of underlying storage driver. @@ -136,12 +164,12 @@ func (base *Base) Move(ctx context.Context, sourcePath string, destPath string) defer done("%s.Move(%q, %q", base.Name(), sourcePath, destPath) if !storagedriver.PathRegexp.MatchString(sourcePath) { - return storagedriver.InvalidPathError{Path: sourcePath} + return storagedriver.InvalidPathError{Path: sourcePath, DriverName: base.StorageDriver.Name()} } else if !storagedriver.PathRegexp.MatchString(destPath) { - return storagedriver.InvalidPathError{Path: destPath} + return storagedriver.InvalidPathError{Path: destPath, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.Move(ctx, sourcePath, destPath) + return base.setDriverName(base.StorageDriver.Move(ctx, sourcePath, destPath)) } // Delete wraps Delete of underlying storage driver. @@ -150,10 +178,10 @@ func (base *Base) Delete(ctx context.Context, path string) error { defer done("%s.Delete(%q)", base.Name(), path) if !storagedriver.PathRegexp.MatchString(path) { - return storagedriver.InvalidPathError{Path: path} + return storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.Delete(ctx, path) + return base.setDriverName(base.StorageDriver.Delete(ctx, path)) } // URLFor wraps URLFor of underlying storage driver. @@ -162,8 +190,9 @@ func (base *Base) URLFor(ctx context.Context, path string, options map[string]in defer done("%s.URLFor(%q)", base.Name(), path) if !storagedriver.PathRegexp.MatchString(path) { - return "", storagedriver.InvalidPathError{Path: path} + return "", storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.URLFor(ctx, path, options) + str, e := base.StorageDriver.URLFor(ctx, path, options) + return str, base.setDriverName(e) } diff --git a/docs/storage/driver/filesystem/driver.go b/docs/storage/driver/filesystem/driver.go index d5d8708c..20ccfce7 100644 --- a/docs/storage/driver/filesystem/driver.go +++ b/docs/storage/driver/filesystem/driver.go @@ -248,7 +248,7 @@ func (d *driver) Delete(ctx context.Context, subPath string) error { // URLFor returns a URL which may be used to retrieve the content stored at the given path. // May return an UnsupportedMethodErr in certain StorageDriver implementations. func (d *driver) URLFor(ctx context.Context, path string, options map[string]interface{}) (string, error) { - return "", storagedriver.ErrUnsupportedMethod + return "", new(storagedriver.ErrUnsupportedMethod) } // fullPath returns the absolute path of a key within the Driver's storage. diff --git a/docs/storage/driver/inmemory/driver.go b/docs/storage/driver/inmemory/driver.go index 2d121e1c..2dad0ec8 100644 --- a/docs/storage/driver/inmemory/driver.go +++ b/docs/storage/driver/inmemory/driver.go @@ -258,5 +258,5 @@ func (d *driver) Delete(ctx context.Context, path string) error { // URLFor returns a URL which may be used to retrieve the content stored at the given path. // May return an UnsupportedMethodErr in certain StorageDriver implementations. func (d *driver) URLFor(ctx context.Context, path string, options map[string]interface{}) (string, error) { - return "", storagedriver.ErrUnsupportedMethod + return "", new(storagedriver.ErrUnsupportedMethod) } diff --git a/docs/storage/driver/oss/oss.go b/docs/storage/driver/oss/oss.go index cec32026..99bca366 100644 --- a/docs/storage/driver/oss/oss.go +++ b/docs/storage/driver/oss/oss.go @@ -748,7 +748,7 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int if ok { methodString, ok = method.(string) if !ok || (methodString != "GET" && methodString != "HEAD") { - return "", storagedriver.ErrUnsupportedMethod + return "", new(storagedriver.ErrUnsupportedMethod) } } diff --git a/docs/storage/driver/rados/rados.go b/docs/storage/driver/rados/rados.go index b2e6590d..fa73c8d2 100644 --- a/docs/storage/driver/rados/rados.go +++ b/docs/storage/driver/rados/rados.go @@ -496,7 +496,7 @@ func (d *driver) Delete(ctx context.Context, objectPath string) error { // URLFor returns a URL which may be used to retrieve the content stored at the given path. // May return an UnsupportedMethodErr in certain StorageDriver implementations. func (d *driver) URLFor(ctx context.Context, path string, options map[string]interface{}) (string, error) { - return "", storagedriver.ErrUnsupportedMethod + return "", new(storagedriver.ErrUnsupportedMethod) } // Generate a blob identifier diff --git a/docs/storage/driver/s3/s3.go b/docs/storage/driver/s3/s3.go index 46dbcd7f..0a9d80c0 100644 --- a/docs/storage/driver/s3/s3.go +++ b/docs/storage/driver/s3/s3.go @@ -759,7 +759,7 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int if ok { methodString, ok = method.(string) if !ok || (methodString != "GET" && methodString != "HEAD") { - return "", storagedriver.ErrUnsupportedMethod + return "", new(storagedriver.ErrUnsupportedMethod) } } diff --git a/docs/storage/driver/storagedriver.go b/docs/storage/driver/storagedriver.go index bade099f..996381c6 100644 --- a/docs/storage/driver/storagedriver.go +++ b/docs/storage/driver/storagedriver.go @@ -1,7 +1,6 @@ package driver import ( - "errors" "fmt" "io" "regexp" @@ -93,33 +92,42 @@ type StorageDriver interface { var PathRegexp = regexp.MustCompile(`^(/[A-Za-z0-9._-]+)+$`) // ErrUnsupportedMethod may be returned in the case where a StorageDriver implementation does not support an optional method. -var ErrUnsupportedMethod = errors.New("unsupported method") +type ErrUnsupportedMethod struct { + DriverName string +} + +func (err ErrUnsupportedMethod) Error() string { + return fmt.Sprintf("[%s] unsupported method", err.DriverName) +} // PathNotFoundError is returned when operating on a nonexistent path. type PathNotFoundError struct { - Path string + Path string + DriverName string } func (err PathNotFoundError) Error() string { - return fmt.Sprintf("Path not found: %s", err.Path) + return fmt.Sprintf("[%s] Path not found: %s", err.DriverName, err.Path) } // InvalidPathError is returned when the provided path is malformed. type InvalidPathError struct { - Path string + Path string + DriverName string } func (err InvalidPathError) Error() string { - return fmt.Sprintf("Invalid path: %s", err.Path) + return fmt.Sprintf("[%s] Invalid path: %s", err.DriverName, err.Path) } // InvalidOffsetError is returned when attempting to read or write from an // invalid offset. type InvalidOffsetError struct { - Path string - Offset int64 + Path string + Offset int64 + DriverName string } func (err InvalidOffsetError) Error() string { - return fmt.Sprintf("Invalid offset: %d for path: %s", err.Offset, err.Path) + return fmt.Sprintf("[%s] Invalid offset: %d for path: %s", err.DriverName, err.Offset, err.Path) } diff --git a/docs/storage/driver/swift/swift.go b/docs/storage/driver/swift/swift.go index 3b2cdc53..bd330925 100644 --- a/docs/storage/driver/swift/swift.go +++ b/docs/storage/driver/swift/swift.go @@ -658,14 +658,14 @@ func (d *driver) Delete(ctx context.Context, path string) error { // URLFor returns a URL which may be used to retrieve the content stored at the given path. func (d *driver) URLFor(ctx context.Context, path string, options map[string]interface{}) (string, error) { if d.SecretKey == "" { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } methodString := "GET" method, ok := options["method"] if ok { if methodString, ok = method.(string); !ok { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } } @@ -684,7 +684,7 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int } if !supported { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } expiresTime := time.Now().Add(20 * time.Minute) diff --git a/docs/storage/driver/testsuites/testsuites.go b/docs/storage/driver/testsuites/testsuites.go index 1772560b..f8117285 100644 --- a/docs/storage/driver/testsuites/testsuites.go +++ b/docs/storage/driver/testsuites/testsuites.go @@ -10,6 +10,7 @@ import ( "os" "path" "sort" + "strings" "sync" "testing" "time" @@ -145,10 +146,12 @@ func (suite *DriverSuite) TestInvalidPaths(c *check.C) { defer suite.StorageDriver.Delete(suite.ctx, firstPart(filename)) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.InvalidPathError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) _, err = suite.StorageDriver.GetContent(suite.ctx, filename) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.InvalidPathError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } } @@ -205,6 +208,7 @@ func (suite *DriverSuite) TestReadNonexistent(c *check.C) { _, err := suite.StorageDriver.GetContent(suite.ctx, filename) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestWriteReadStreams1 tests a simple write-read streaming workflow. @@ -321,6 +325,7 @@ func (suite *DriverSuite) TestReadStreamWithOffset(c *check.C) { c.Assert(err.(storagedriver.InvalidOffsetError).Offset, check.Equals, int64(-1)) c.Assert(err.(storagedriver.InvalidOffsetError).Path, check.Equals, filename) c.Assert(reader, check.IsNil) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) // Read past the end of the content and make sure we get a reader that // returns 0 bytes and io.EOF @@ -443,6 +448,7 @@ func (suite *DriverSuite) testContinueStreamAppend(c *check.C, chunkSize int64) c.Assert(err, check.FitsTypeOf, storagedriver.InvalidOffsetError{}) c.Assert(err.(storagedriver.InvalidOffsetError).Path, check.Equals, filename) c.Assert(err.(storagedriver.InvalidOffsetError).Offset, check.Equals, int64(-1)) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestReadNonexistentStream tests that reading a stream for a nonexistent path @@ -453,10 +459,12 @@ func (suite *DriverSuite) TestReadNonexistentStream(c *check.C) { _, err := suite.StorageDriver.ReadStream(suite.ctx, filename, 0) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) _, err = suite.StorageDriver.ReadStream(suite.ctx, filename, 64) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestList checks the returned list of keys after populating a directory tree. @@ -517,6 +525,7 @@ func (suite *DriverSuite) TestMove(c *check.C) { _, err = suite.StorageDriver.GetContent(suite.ctx, sourcePath) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestMoveOverwrite checks that a moved object no longer exists at the source @@ -546,6 +555,7 @@ func (suite *DriverSuite) TestMoveOverwrite(c *check.C) { _, err = suite.StorageDriver.GetContent(suite.ctx, sourcePath) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestMoveNonexistent checks that moving a nonexistent key fails and does not @@ -563,6 +573,7 @@ func (suite *DriverSuite) TestMoveNonexistent(c *check.C) { err = suite.StorageDriver.Move(suite.ctx, sourcePath, destPath) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) received, err := suite.StorageDriver.GetContent(suite.ctx, destPath) c.Assert(err, check.IsNil) @@ -600,6 +611,7 @@ func (suite *DriverSuite) TestDelete(c *check.C) { _, err = suite.StorageDriver.GetContent(suite.ctx, filename) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestURLFor checks that the URLFor method functions properly, but only if it @@ -614,7 +626,7 @@ func (suite *DriverSuite) TestURLFor(c *check.C) { c.Assert(err, check.IsNil) url, err := suite.StorageDriver.URLFor(suite.ctx, filename, nil) - if err == storagedriver.ErrUnsupportedMethod { + if _, ok := err.(*storagedriver.ErrUnsupportedMethod); ok { return } c.Assert(err, check.IsNil) @@ -628,7 +640,7 @@ func (suite *DriverSuite) TestURLFor(c *check.C) { c.Assert(read, check.DeepEquals, contents) url, err = suite.StorageDriver.URLFor(suite.ctx, filename, map[string]interface{}{"method": "HEAD"}) - if err == storagedriver.ErrUnsupportedMethod { + if _, ok := err.(*storagedriver.ErrUnsupportedMethod); ok { return } c.Assert(err, check.IsNil) @@ -644,6 +656,7 @@ func (suite *DriverSuite) TestDeleteNonexistent(c *check.C) { err := suite.StorageDriver.Delete(suite.ctx, filename) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestDeleteFolder checks that deleting a folder removes all child elements. @@ -671,6 +684,7 @@ func (suite *DriverSuite) TestDeleteFolder(c *check.C) { _, err = suite.StorageDriver.GetContent(suite.ctx, path.Join(dirname, filename1)) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) _, err = suite.StorageDriver.GetContent(suite.ctx, path.Join(dirname, filename2)) c.Assert(err, check.IsNil) @@ -684,14 +698,17 @@ func (suite *DriverSuite) TestDeleteFolder(c *check.C) { _, err = suite.StorageDriver.GetContent(suite.ctx, path.Join(dirname, filename1)) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) _, err = suite.StorageDriver.GetContent(suite.ctx, path.Join(dirname, filename2)) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) _, err = suite.StorageDriver.GetContent(suite.ctx, path.Join(dirname, filename3)) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestStatCall runs verifies the implementation of the storagedriver's Stat call. @@ -707,11 +724,13 @@ func (suite *DriverSuite) TestStatCall(c *check.C) { fi, err := suite.StorageDriver.Stat(suite.ctx, dirPath) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) c.Assert(fi, check.IsNil) fi, err = suite.StorageDriver.Stat(suite.ctx, filePath) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) c.Assert(fi, check.IsNil) err = suite.StorageDriver.PutContent(suite.ctx, filePath, content) From e79324edd8794711003e4602917b36adc175a4a0 Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Mon, 2 Nov 2015 13:23:53 -0800 Subject: [PATCH 2/2] Add a generic error type to capture non-typed errors Signed-off-by: Richard Scothern --- docs/storage/blobserver.go | 6 ++-- docs/storage/driver/base/base.go | 38 +++++++++++--------- docs/storage/driver/filesystem/driver.go | 2 +- docs/storage/driver/gcs/gcs.go | 4 +-- docs/storage/driver/inmemory/driver.go | 2 +- docs/storage/driver/oss/oss.go | 2 +- docs/storage/driver/rados/rados.go | 2 +- docs/storage/driver/s3/s3.go | 2 +- docs/storage/driver/storagedriver.go | 11 ++++++ docs/storage/driver/testsuites/testsuites.go | 4 +-- 10 files changed, 44 insertions(+), 29 deletions(-) diff --git a/docs/storage/blobserver.go b/docs/storage/blobserver.go index 2d89ecd8..45f81f53 100644 --- a/docs/storage/blobserver.go +++ b/docs/storage/blobserver.go @@ -36,15 +36,15 @@ func (bs *blobServer) ServeBlob(ctx context.Context, w http.ResponseWriter, r *h redirectURL, err := bs.driver.URLFor(ctx, path, map[string]interface{}{"method": r.Method}) - if err == nil { + switch err.(type) { + case nil: if bs.redirect { // Redirect to storage URL. http.Redirect(w, r, redirectURL, http.StatusTemporaryRedirect) return err } - } - if _, ok := err.(*driver.ErrUnsupportedMethod); ok { + case driver.ErrUnsupportedMethod: // Fallback to serving the content directly. br, err := newFileReader(ctx, bs.driver, path, desc.Size) if err != nil { diff --git a/docs/storage/driver/base/base.go b/docs/storage/driver/base/base.go index 2333bba7..c816d2d6 100644 --- a/docs/storage/driver/base/base.go +++ b/docs/storage/driver/base/base.go @@ -52,25 +52,29 @@ type Base struct { // Format errors received from the storage driver func (base *Base) setDriverName(e error) error { - if e != nil { - if actualErr, ok := e.(storagedriver.ErrUnsupportedMethod); ok { - actualErr.DriverName = base.StorageDriver.Name() - return actualErr - } - if actualErr, ok := e.(storagedriver.PathNotFoundError); ok { - actualErr.DriverName = base.StorageDriver.Name() - return actualErr - } - if actualErr, ok := e.(storagedriver.InvalidPathError); ok { - actualErr.DriverName = base.StorageDriver.Name() - return actualErr - } - if actualErr, ok := e.(storagedriver.InvalidOffsetError); ok { - actualErr.DriverName = base.StorageDriver.Name() - return actualErr + switch actual := e.(type) { + case nil: + return nil + case storagedriver.ErrUnsupportedMethod: + actual.DriverName = base.StorageDriver.Name() + return actual + case storagedriver.PathNotFoundError: + actual.DriverName = base.StorageDriver.Name() + return actual + case storagedriver.InvalidPathError: + actual.DriverName = base.StorageDriver.Name() + return actual + case storagedriver.InvalidOffsetError: + actual.DriverName = base.StorageDriver.Name() + return actual + default: + storageError := storagedriver.Error{ + DriverName: base.StorageDriver.Name(), + Enclosed: e, } + + return storageError } - return e } // GetContent wraps GetContent of underlying storage driver. diff --git a/docs/storage/driver/filesystem/driver.go b/docs/storage/driver/filesystem/driver.go index 20ccfce7..7dece0b3 100644 --- a/docs/storage/driver/filesystem/driver.go +++ b/docs/storage/driver/filesystem/driver.go @@ -248,7 +248,7 @@ func (d *driver) Delete(ctx context.Context, subPath string) error { // URLFor returns a URL which may be used to retrieve the content stored at the given path. // May return an UnsupportedMethodErr in certain StorageDriver implementations. func (d *driver) URLFor(ctx context.Context, path string, options map[string]interface{}) (string, error) { - return "", new(storagedriver.ErrUnsupportedMethod) + return "", storagedriver.ErrUnsupportedMethod{} } // fullPath returns the absolute path of a key within the Driver's storage. diff --git a/docs/storage/driver/gcs/gcs.go b/docs/storage/driver/gcs/gcs.go index 8dc96675..4cef972c 100644 --- a/docs/storage/driver/gcs/gcs.go +++ b/docs/storage/driver/gcs/gcs.go @@ -575,7 +575,7 @@ func (d *driver) Delete(context ctx.Context, path string) error { // Returns ErrUnsupportedMethod if this driver has no privateKey func (d *driver) URLFor(context ctx.Context, path string, options map[string]interface{}) (string, error) { if d.privateKey == nil { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } name := d.pathToKey(path) @@ -584,7 +584,7 @@ func (d *driver) URLFor(context ctx.Context, path string, options map[string]int if ok { methodString, ok = method.(string) if !ok || (methodString != "GET" && methodString != "HEAD") { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } } diff --git a/docs/storage/driver/inmemory/driver.go b/docs/storage/driver/inmemory/driver.go index 2dad0ec8..b5735c0a 100644 --- a/docs/storage/driver/inmemory/driver.go +++ b/docs/storage/driver/inmemory/driver.go @@ -258,5 +258,5 @@ func (d *driver) Delete(ctx context.Context, path string) error { // URLFor returns a URL which may be used to retrieve the content stored at the given path. // May return an UnsupportedMethodErr in certain StorageDriver implementations. func (d *driver) URLFor(ctx context.Context, path string, options map[string]interface{}) (string, error) { - return "", new(storagedriver.ErrUnsupportedMethod) + return "", storagedriver.ErrUnsupportedMethod{} } diff --git a/docs/storage/driver/oss/oss.go b/docs/storage/driver/oss/oss.go index 99bca366..c16b9949 100644 --- a/docs/storage/driver/oss/oss.go +++ b/docs/storage/driver/oss/oss.go @@ -748,7 +748,7 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int if ok { methodString, ok = method.(string) if !ok || (methodString != "GET" && methodString != "HEAD") { - return "", new(storagedriver.ErrUnsupportedMethod) + return "", storagedriver.ErrUnsupportedMethod{} } } diff --git a/docs/storage/driver/rados/rados.go b/docs/storage/driver/rados/rados.go index fa73c8d2..29bc3247 100644 --- a/docs/storage/driver/rados/rados.go +++ b/docs/storage/driver/rados/rados.go @@ -496,7 +496,7 @@ func (d *driver) Delete(ctx context.Context, objectPath string) error { // URLFor returns a URL which may be used to retrieve the content stored at the given path. // May return an UnsupportedMethodErr in certain StorageDriver implementations. func (d *driver) URLFor(ctx context.Context, path string, options map[string]interface{}) (string, error) { - return "", new(storagedriver.ErrUnsupportedMethod) + return "", storagedriver.ErrUnsupportedMethod{} } // Generate a blob identifier diff --git a/docs/storage/driver/s3/s3.go b/docs/storage/driver/s3/s3.go index 0a9d80c0..7672fbdb 100644 --- a/docs/storage/driver/s3/s3.go +++ b/docs/storage/driver/s3/s3.go @@ -759,7 +759,7 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int if ok { methodString, ok = method.(string) if !ok || (methodString != "GET" && methodString != "HEAD") { - return "", new(storagedriver.ErrUnsupportedMethod) + return "", storagedriver.ErrUnsupportedMethod{} } } diff --git a/docs/storage/driver/storagedriver.go b/docs/storage/driver/storagedriver.go index 996381c6..cd1c883b 100644 --- a/docs/storage/driver/storagedriver.go +++ b/docs/storage/driver/storagedriver.go @@ -131,3 +131,14 @@ type InvalidOffsetError struct { func (err InvalidOffsetError) Error() string { return fmt.Sprintf("[%s] Invalid offset: %d for path: %s", err.DriverName, err.Offset, err.Path) } + +// Error is a catch-all error type which captures an error string and +// the driver type on which it occured. +type Error struct { + DriverName string + Enclosed error +} + +func (err Error) Error() string { + return fmt.Sprintf("[%s] %s", err.DriverName, err.Enclosed) +} diff --git a/docs/storage/driver/testsuites/testsuites.go b/docs/storage/driver/testsuites/testsuites.go index f8117285..f99df8d9 100644 --- a/docs/storage/driver/testsuites/testsuites.go +++ b/docs/storage/driver/testsuites/testsuites.go @@ -626,7 +626,7 @@ func (suite *DriverSuite) TestURLFor(c *check.C) { c.Assert(err, check.IsNil) url, err := suite.StorageDriver.URLFor(suite.ctx, filename, nil) - if _, ok := err.(*storagedriver.ErrUnsupportedMethod); ok { + if _, ok := err.(storagedriver.ErrUnsupportedMethod); ok { return } c.Assert(err, check.IsNil) @@ -640,7 +640,7 @@ func (suite *DriverSuite) TestURLFor(c *check.C) { c.Assert(read, check.DeepEquals, contents) url, err = suite.StorageDriver.URLFor(suite.ctx, filename, map[string]interface{}{"method": "HEAD"}) - if _, ok := err.(*storagedriver.ErrUnsupportedMethod); ok { + if _, ok := err.(storagedriver.ErrUnsupportedMethod); ok { return } c.Assert(err, check.IsNil)