KindNotFound for all storages (#560)

* all exc mongo

* module, version associated with each storage error

* remove confusion

* removed formatted error in favor of raw flavor

* once more

pkg/storage/azurecdn/storage.go

@@ -65,7 +65,7 @@ func (s *Storage) Save(ctx context.Context, module, version string, mod []byte,
 	//
 	// Do that only after module source+metadata is uploaded
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	return nil
 }

pkg/storage/fs/checker.go

@@ -16,7 +16,7 @@ func (v *storageImpl) Exists(ctx context.Context, module, version string) (bool,
 	versionedPath := v.versionLocation(module, version)
 	exists, err := afero.Exists(v.filesystem, filepath.Join(versionedPath, "go.mod"))
 	if err != nil {
-		return false, errors.E(op, err)
+		return false, errors.E(op, errors.M(module), errors.V(version), err)
 	}
 
 	return exists, nil

pkg/storage/fs/deleter.go

@@ -15,7 +15,7 @@ func (v *storageImpl) Delete(ctx context.Context, module, version string) error
 	versionedPath := v.versionLocation(module, version)
 	exists, err := v.Exists(ctx, module, version)
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	if !exists {
 		return errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound)

pkg/storage/fs/lister.go

@@ -2,6 +2,7 @@ package fs
 
 import (
 	"context"
+	"os"
 
 	"github.com/gomods/athens/pkg/errors"
 	opentracing "github.com/opentracing/opentracing-go"
@@ -15,7 +16,12 @@ func (l *storageImpl) List(ctx context.Context, module string) ([]string, error)
 	loc := l.moduleLocation(module)
 	fileInfos, err := afero.ReadDir(l.filesystem, loc)
 	if err != nil {
-		return nil, errors.E(op, err)
+		kind := errors.KindUnexpected
+		if os.IsNotExist(err) {
+			kind = errors.KindNotFound
+		}
+
+		return nil, errors.E(op, errors.M(module), err, kind)
 	}
 	ret := []string{}
 	for _, fileInfo := range fileInfos {

pkg/storage/fs/saver.go

@@ -11,36 +11,36 @@ import (
 	"github.com/spf13/afero"
 )
 
-func (s *storageImpl) Save(ctx context.Context, module, vsn string, mod []byte, zip io.Reader, info []byte) error {
+func (s *storageImpl) Save(ctx context.Context, module, version string, mod []byte, zip io.Reader, info []byte) error {
 	const op errors.Op = "fs.Save"
 	sp, ctx := opentracing.StartSpanFromContext(ctx, "storage.fs.Save")
 	defer sp.Finish()
-	dir := s.versionLocation(module, vsn)
+	dir := s.versionLocation(module, version)
 	// TODO: 777 is not the best filemode, use something better
 
 	// make the versioned directory to hold the go.mod and the zipfile
 	if err := s.filesystem.MkdirAll(dir, os.ModeDir|os.ModePerm); err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 
 	// write the go.mod file
 	if err := afero.WriteFile(s.filesystem, filepath.Join(dir, "go.mod"), mod, os.ModePerm); err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 
 	// write the zipfile
 	f, err := s.filesystem.OpenFile(filepath.Join(dir, "source.zip"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm)
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	defer f.Close()
 	_, err = io.Copy(f, zip)
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 
 	// write the info file
-	err = afero.WriteFile(s.filesystem, filepath.Join(dir, vsn+".info"), info, os.ModePerm)
+	err = afero.WriteFile(s.filesystem, filepath.Join(dir, version+".info"), info, os.ModePerm)
 	if err != nil {
 		return errors.E(op, err)
 	}

pkg/storage/gcp/deleter.go

@@ -18,7 +18,7 @@ func (s *Storage) Delete(ctx context.Context, module, version string) error {
 	defer sp.Finish()
 	exists, err := s.bucket.Exists(ctx, config.PackageVersionedName(module, version, "mod"))
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	if !exists {
 		return errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound)

pkg/storage/gcp/getter.go

@@ -18,7 +18,7 @@ func (s *Storage) Info(ctx context.Context, module, version string) ([]byte, err
 	defer sp.Finish()
 	exists, err := s.Exists(ctx, module, version)
 	if err != nil {
-		return nil, errors.E(op, err)
+		return nil, errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	if !exists {
 		return nil, errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound)
@@ -26,12 +26,12 @@ func (s *Storage) Info(ctx context.Context, module, version string) ([]byte, err
 
 	infoReader, err := s.bucket.Open(ctx, config.PackageVersionedName(module, version, "info"))
 	if err != nil {
-		return nil, errors.E(op, err)
+		return nil, errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	infoBytes, err := ioutil.ReadAll(infoReader)
 	infoReader.Close()
 	if err != nil {
-		return nil, errors.E(op, fmt.Errorf("could not read bytes of info file: %s", err))
+		return nil, errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	return infoBytes, nil
 }
@@ -43,7 +43,7 @@ func (s *Storage) GoMod(ctx context.Context, module, version string) ([]byte, er
 	defer sp.Finish()
 	exists, err := s.Exists(ctx, module, version)
 	if err != nil {
-		return nil, errors.E(op, err)
+		return nil, errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	if !exists {
 		return nil, errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound)
@@ -51,12 +51,12 @@ func (s *Storage) GoMod(ctx context.Context, module, version string) ([]byte, er
 
 	modReader, err := s.bucket.Open(ctx, config.PackageVersionedName(module, version, "mod"))
 	if err != nil {
-		return nil, errors.E(op, err)
+		return nil, errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	modBytes, err := ioutil.ReadAll(modReader)
 	modReader.Close()
 	if err != nil {
-		return nil, errors.E(op, fmt.Errorf("could not get new reader for mod file: %s", err))
+		return nil, errors.E(op, fmt.Errorf("could not get new reader for mod file: %s", err), errors.M(module), errors.V(version))
 	}
 
 	return modBytes, nil
@@ -69,7 +69,7 @@ func (s *Storage) Zip(ctx context.Context, module, version string) (io.ReadClose
 	defer sp.Finish()
 	exists, err := s.Exists(ctx, module, version)
 	if err != nil {
-		return nil, errors.E(op, err)
+		return nil, errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	if !exists {
 		return nil, errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound)
@@ -77,7 +77,7 @@ func (s *Storage) Zip(ctx context.Context, module, version string) (io.ReadClose
 
 	zipReader, err := s.bucket.Open(ctx, config.PackageVersionedName(module, version, "zip"))
 	if err != nil {
-		return nil, errors.E(op, err)
+		return nil, errors.E(op, err, errors.M(module), errors.V(version))
 	}
 
 	return zipReader, nil

pkg/storage/gcp/lister.go

@@ -16,7 +16,7 @@ func (s *Storage) List(ctx context.Context, module string) ([]string, error) {
 	defer sp.Finish()
 	paths, err := s.bucket.List(ctx, module)
 	if err != nil {
-		return nil, err
+		return nil, errors.E(op, err, errors.M(module))
 	}
 	versions := extractVersions(paths)
 	if len(versions) < 1 {

pkg/storage/gcp/saver.go

@@ -25,7 +25,7 @@ func (s *Storage) Save(ctx context.Context, module, version string, mod []byte,
 	defer sp.Finish()
 	exists, err := s.Exists(ctx, module, version)
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	if exists {
 		return errors.E(op, "already exists", errors.M(module), errors.V(version), errors.KindAlreadyExists)
@@ -33,7 +33,7 @@ func (s *Storage) Save(ctx context.Context, module, version string, mod []byte,
 
 	err = moduploader.Upload(ctx, module, version, bytes.NewReader(info), bytes.NewReader(mod), zip, s.upload)
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 
 	// TODO: take out lease on the /list file and add the version to it

pkg/storage/minio/checker.go

@@ -26,7 +26,7 @@ func (v *storageImpl) Exists(ctx context.Context, module, version string) (bool,
 	}
 
 	if err != nil {
-		return false, errors.E(op, err)
+		return false, errors.E(op, err, errors.M(module), errors.V(version))
 	}
 
 	return true, nil

pkg/storage/minio/deleter.go

@@ -14,7 +14,7 @@ func (v *storageImpl) Delete(ctx context.Context, module, version string) error
 	defer sp.Finish()
 	exists, err := v.Exists(ctx, module, version)
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 
 	if !exists {
@@ -25,18 +25,18 @@ func (v *storageImpl) Delete(ctx context.Context, module, version string) error
 
 	modPath := fmt.Sprintf("%s/go.mod", versionedPath)
 	if err := v.minioClient.RemoveObject(v.bucketName, modPath); err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 
 	zipPath := fmt.Sprintf("%s/source.zip", versionedPath)
 	if err := v.minioClient.RemoveObject(v.bucketName, zipPath); err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 
 	infoPath := fmt.Sprintf("%s/%s.info", versionedPath, version)
 	err = v.minioClient.RemoveObject(v.bucketName, infoPath)
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	return nil
 }

pkg/storage/minio/getter.go

@@ -49,7 +49,13 @@ func (v *storageImpl) Zip(ctx context.Context, module, vsn string) (io.ReadClose
 	const op errors.Op = "minio.Zip"
 	sp, ctx := opentracing.StartSpanFromContext(ctx, "storage.minio.Zip")
 	defer sp.Finish()
+
 	zipPath := fmt.Sprintf("%s/source.zip", v.versionLocation(module, vsn))
+	_, err := v.minioClient.StatObject(v.bucketName, zipPath, minio.StatObjectOptions{})
+	if err != nil {
+		return nil, errors.E(op, err, errors.KindNotFound, errors.M(module), errors.V(vsn))
+	}
+
 	zipReader, err := v.minioClient.GetObject(v.bucketName, zipPath, minio.GetObjectOptions{})
 	if err != nil {
 		return nil, errors.E(op, err)

pkg/storage/minio/lister.go

@@ -21,7 +21,7 @@ func (l *storageImpl) List(ctx context.Context, module string) ([]string, error)
 	objectCh := l.minioClient.ListObjectsV2(l.bucketName, searchPrefix, false, doneCh)
 	for object := range objectCh {
 		if object.Err != nil {
-			return nil, errors.E(op, object.Err)
+			return nil, errors.E(op, object.Err, errors.M(module))
 		}
 		parts := strings.Split(object.Key, "/")
 		ver := parts[len(parts)-2]
@@ -29,7 +29,12 @@ func (l *storageImpl) List(ctx context.Context, module string) ([]string, error)
 			dict[ver] = struct{}{}
 		}
 	}
+
 	ret := []string{}
+	if len(dict) == 0 {
+		return ret, errors.E(op, errors.M(module), errors.KindNotFound)
+	}
+
 	for ver := range dict {
 		ret = append(ret, ver)
 	}

pkg/storage/mongo/checker.go

@@ -4,14 +4,19 @@ import (
 	"context"
 
 	"github.com/globalsign/mgo/bson"
+	"github.com/gomods/athens/pkg/errors"
 	opentracing "github.com/opentracing/opentracing-go"
 )
 
 // Exists checks for a specific version of a module
 func (s *ModuleStore) Exists(ctx context.Context, module, vsn string) (bool, error) {
+	var op errors.Op = "storage.mongo.Exists"
 	sp, ctx := opentracing.StartSpanFromContext(ctx, "storage.mongo.Exists")
 	defer sp.Finish()
 	c := s.s.DB(s.d).C(s.c)
 	count, err := c.Find(bson.M{"module": module, "version": vsn}).Count()
-	return err == nil && count > 0, err
+	if err != nil {
+		return false, errors.E(op, errors.M(module), errors.V(vsn), err)
+	}
+	return count > 0, nil
 }

pkg/storage/mongo/deleter.go

@@ -15,7 +15,7 @@ func (s *ModuleStore) Delete(ctx context.Context, module, version string) error
 	defer sp.Finish()
 	exists, err := s.Exists(ctx, module, version)
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	if !exists {
 		return errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound)
@@ -25,11 +25,11 @@ func (s *ModuleStore) Delete(ctx context.Context, module, version string) error
 	c := db.C(s.c)
 	err = db.GridFS("fs").Remove(s.gridFileName(module, version))
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	err = c.Remove(bson.M{"module": module, "version": version})
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	return nil
 }

pkg/storage/mongo/getter.go

@@ -63,7 +63,7 @@ func (s *ModuleStore) Zip(ctx context.Context, module, vsn string) (io.ReadClose
 		if err == mgo.ErrNotFound {
 			kind = errors.KindNotFound
 		}
-		return nil, errors.E(op, err, kind)
+		return nil, errors.E(op, err, kind, errors.M(module), errors.V(vsn))
 	}
 
 	return f, nil

pkg/storage/mongo/lister.go

@@ -27,6 +27,10 @@ func (s *ModuleStore) List(ctx context.Context, module string) ([]string, error)
 	}
 
 	versions := make([]string, len(result))
+	if len(result) == 0 {
+		return versions, errors.E(op, errors.M(module), errors.KindNotFound)
+	}
+
 	for i, r := range result {
 		versions[i] = r.Version
 	}

pkg/storage/mongo/saver.go

@@ -19,13 +19,13 @@ func (s *ModuleStore) Save(ctx context.Context, module, version string, mod []by
 	fs := s.s.DB(s.d).GridFS("fs")
 	f, err := fs.Create(zipName)
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	defer f.Close()
 
 	_, err = io.Copy(f, zip) // check number of bytes written?
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 
 	m := &storage.Module{
@@ -38,7 +38,7 @@ func (s *ModuleStore) Save(ctx context.Context, module, version string, mod []by
 	c := s.s.DB(s.d).C(s.c)
 	err = c.Insert(m)
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 
 	return nil

pkg/storage/s3/storage.go

@@ -87,7 +87,7 @@ func (s *Storage) Save(ctx context.Context, module, version string, mod []byte,
 	//
 	// Do that only after module source+metadata is uploaded
 	if err != nil {
-		return errors.E(op, err)
+		return errors.E(op, err, errors.M(module), errors.V(version))
 	}
 	return nil
 }

pkg/storage/storage_tests/module_storage/storage_test.go

@@ -36,7 +36,7 @@ type TestSuites struct {
 func (d *TestSuites) SetupTest() {
 	ra := d.Require()
 
-	//
+	// file system
 	fsTests, err := fs.NewTestSuite(d.Model)
 	ra.NoError(err)
 	d.storages = append(d.storages, fsTests)
@@ -71,6 +71,7 @@ func TestModuleStorages(t *testing.T) {
 func (d *TestSuites) TestStorages() {
 	for _, store := range d.storages {
 		d.testNotFound(store)
+		d.testKindNotFound(store)
 		d.testGetSaveListRoundTrip(store)
 		d.testList(store)
 		d.testDelete(store)
@@ -86,6 +87,34 @@ func (d *TestSuites) testNotFound(ts storage.TestSuite) {
 	d.Require().Equal(true, errors.IsNotFoundErr(err), "Invalid error type for %s: %#v", ts.StorageHumanReadableName(), err)
 }
 
+func (d *TestSuites) testKindNotFound(ts storage.TestSuite) {
+	s := ts.Storage()
+	mod, ver := "xxx", "yyy"
+	ctx := context.Background()
+	r := d.Require()
+	hrn := ts.StorageHumanReadableName()
+
+	err := s.Delete(ctx, mod, ver)
+	r.Error(err, hrn)
+	r.Equal(errors.KindNotFound, errors.Kind(err), hrn)
+
+	_, err = s.GoMod(ctx, mod, ver)
+	r.Error(err, hrn)
+	r.Equal(errors.KindNotFound, errors.Kind(err), hrn)
+
+	_, err = s.Info(ctx, mod, ver)
+	r.Error(err, hrn)
+	r.Equal(errors.KindNotFound, errors.Kind(err), hrn)
+
+	_, err = s.List(ctx, mod)
+	r.Error(err, hrn)
+	r.Equal(errors.KindNotFound, errors.Kind(err), hrn)
+
+	_, err = s.Zip(ctx, mod, ver)
+	r.Error(err, hrn)
+	r.Equal(errors.KindNotFound, errors.Kind(err), hrn)
+}
+
 func (d *TestSuites) testList(ts storage.TestSuite) {
 	r := d.Require()
 	hrn := ts.StorageHumanReadableName()