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 <amit.shukla@docker.com>
This commit is contained in:
amitshukla 2015-10-02 16:19:06 -07:00 committed by Richard Scothern
parent bd958d8b88
commit 7840a5bc8f
10 changed files with 99 additions and 44 deletions

View file

@ -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 {

View file

@ -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)
}

View file

@ -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.

View file

@ -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)
}

View file

@ -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)
}
}

View file

@ -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

View file

@ -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)
}
}

View file

@ -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)
}

View file

@ -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)

View file

@ -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)