Consistent check for non-existing objects (#190)

commands/cmd_clonestore.go

@@ -361,7 +361,7 @@ func cloneOneVersion(
 		return cloneVersionIndex(currentVersionIndex), nil
 	}
 
-	if !errors.Is(err, os.ErrNotExist) {
+	if !longtaillib.IsNotExist(err) {
 		return cloneVersionIndex(currentVersionIndex), errors.Wrap(err, fname)
 	}
 

longtailstorelib/blobStore_test.go

@@ -4,13 +4,12 @@ import (
 	"context"
 	"fmt"
 	"log"
-	"os"
 	"sort"
 	"strings"
 	"testing"
 	"time"
 
-	"github.com/pkg/errors"
+	"github.com/DanEngelbrecht/golongtail/longtaillib"
 )
 
 func TestCreateStoreAndClient(t *testing.T) {
@@ -38,7 +37,7 @@ func TestListObjectsInEmptyStore(t *testing.T) {
 	}
 	obj, _ := client.NewObject("should-not-exist")
 	data, err := obj.Read()
-	if !errors.Is(err, os.ErrNotExist) {
+	if !longtaillib.IsNotExist(err) {
 		t.Errorf("TestListObjectsInEmptyStore() obj.Read()) %s", err)
 	}
 	if data != nil {

longtailstorelib/fsstore.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/DanEngelbrecht/golongtail/longtaillib"
 	"github.com/pkg/errors"
 )
 
@@ -101,11 +102,10 @@ func (blobClient *fsBlobClient) String() string {
 func (blobObject *fsBlobObject) Exists() (bool, error) {
 	const fname = "fsBlobObject.Exists"
 	_, err := os.Stat(blobObject.path)
-	if errors.Is(err, os.ErrNotExist) {
+	if longtaillib.IsNotExist(err) {
 		return false, nil
 	}
 	if err != nil {
-		err = errors.Wrap(err, fmt.Sprintf("Failed to check for existance `%s`", blobObject.path))
 		return false, errors.Wrap(err, fname)
 	}
 	return true, nil
@@ -138,7 +138,7 @@ func (blobObject *fsBlobObject) getMetaGeneration() (int64, error) {
 	const fname = "fsBlobObject.getMetaGeneration"
 	metapath := blobObject.path + ".gen"
 	data, err := ioutil.ReadFile(metapath)
-	if errors.Is(err, os.ErrNotExist) {
+	if longtaillib.IsNotExist(err) {
 		return 0, nil
 	}
 	if err != nil {
@@ -165,7 +165,7 @@ func (blobObject *fsBlobObject) deleteGeneration() error {
 	const fname = "fsBlobObject.deleteGeneration"
 	metapath := blobObject.path + ".gen"
 	_, err := os.Stat(metapath)
-	if errors.Is(err, os.ErrNotExist) {
+	if longtaillib.IsNotExist(err) {
 		return nil
 	}
 	err = os.Remove(metapath)
@@ -299,7 +299,7 @@ func (blobObject *fsBlobObject) Delete() error {
 		if err != nil {
 			return errors.Wrap(err, fname)
 		}
-	} else if errors.Is(err, os.ErrNotExist) {
+	} else if longtaillib.IsNotExist(err) {
 		return nil
 	}
 	return err

longtailstorelib/fsstore_test.go

@@ -3,13 +3,13 @@ package longtailstorelib
 import (
 	"fmt"
 	"io/ioutil"
-	"os"
 	"strings"
 	"sync"
 	"testing"
 
-	"github.com/pkg/errors"
 	"golang.org/x/net/context"
+
+	"github.com/DanEngelbrecht/golongtail/longtaillib"
 )
 
 func TestFSBlobStore(t *testing.T) {
@@ -53,7 +53,7 @@ func TestListObjectsInEmptyFSStore(t *testing.T) {
 	}
 	obj, _ := client.NewObject("should-not-exist")
 	data, err := obj.Read()
-	if !errors.Is(err, os.ErrNotExist) {
+	if !longtaillib.IsNotExist(err) {
 		t.Errorf("TestListObjectsInEmptyFSStore() obj.Read()) %s", err)
 	}
 	if data != nil {

longtailstorelib/gcsBlobStore_test.go

@@ -3,13 +3,13 @@ package longtailstorelib
 import (
 	"fmt"
 	"net/url"
-	"os"
 	"strings"
 	"sync"
 	"testing"
 
-	"github.com/pkg/errors"
 	"golang.org/x/net/context"
+
+	"github.com/DanEngelbrecht/golongtail/longtaillib"
 )
 
 func TestGCSBlobStore(t *testing.T) {
@@ -131,7 +131,7 @@ func TestListObjectsInEmptyGCSStore(t *testing.T) {
 	}
 	obj, _ := client.NewObject("should-not-exist")
 	data, err := obj.Read()
-	if !errors.Is(err, os.ErrNotExist) {
+	if !longtaillib.IsNotExist(err) {
 		t.Errorf("TestListObjectsInEmptyGCSStore() obj.Read()) %s", err)
 	}
 	if data != nil {

longtailstorelib/go.mod

@@ -10,6 +10,8 @@ require (
 	github.com/pkg/errors v0.9.1
 	golang.org/x/net v0.0.0-20210917221730-978cfadd31cf
 	google.golang.org/api v0.57.0
+
+	github.com/DanEngelbrecht/golongtail/longtaillib v0.0.0-00010101000000-000000000000
 )
 
 require (

longtailstorelib/s3Store.go

@@ -131,13 +131,18 @@ func (blobObject *s3BlobObject) LockWriteVersion() (bool, error) {
 }
 
 func (blobObject *s3BlobObject) Exists() (bool, error) {
+	const fname = "s3BlobObject.Exists()"
 	input := &s3.GetObjectAclInput{
 		Bucket: aws.String(blobObject.client.store.bucketName),
 		Key:    aws.String(blobObject.path),
 	}
 	_, err := blobObject.client.client.GetObjectAcl(blobObject.client.ctx, input)
 	if err != nil {
-		return false, nil
+		var nsk *types.NoSuchKey
+		if errors.As(err, &nsk) {
+			return false, nil
+		}
+		return false, errors.Wrap(err, fname)
 	}
 	return true, nil
 }

longtailstorelib/s3Store_test.go

@@ -2,11 +2,11 @@ package longtailstorelib
 
 import (
 	"net/url"
-	"os"
 	"testing"
 
-	"github.com/pkg/errors"
 	"golang.org/x/net/context"
+
+	"github.com/DanEngelbrecht/golongtail/longtaillib"
 )
 
 func TestS3BlobStore(t *testing.T) {
@@ -128,7 +128,7 @@ func TestListObjectsInEmptyS3Store(t *testing.T) {
 	}
 	obj, _ := client.NewObject("should-not-exist")
 	data, err := obj.Read()
-	if !errors.Is(err, os.ErrNotExist) {
+	if !longtaillib.IsNotExist(err) {
 		t.Errorf("TestListObjectsInEmptyS3Store() obj.Read()) %s", err)
 	}
 	if data != nil {

longtailutils/longtailutils.go

@@ -3,7 +3,6 @@ package longtailutils
 import (
 	"context"
 	"fmt"
-	"os"
 	"strings"
 	"sync"
 	"time"
@@ -359,7 +358,7 @@ func DeleteByURI(uri string) error {
 		return errors.Wrap(err, fname)
 	}
 	err = object.Delete()
-	if err != nil && !errors.Is(err, os.ErrNotExist) {
+	if err != nil && !longtaillib.IsNotExist(err) {
 		return errors.Wrap(err, fname)
 	}
 	return nil
@@ -393,7 +392,7 @@ func ReadBlobWithRetry(
 	retryDelay := []time.Duration{0, 100 * time.Millisecond, 250 * time.Millisecond, 500 * time.Millisecond, 1 * time.Second, 2 * time.Second}
 	blobData, err := objHandle.Read()
 	for err != nil {
-		if errors.Is(err, os.ErrNotExist) {
+		if longtaillib.IsNotExist(err) {
 			return nil, retryCount, errors.Wrap(err, fname)
 		}
 		if retryCount == len(retryDelay) {

remotestore/remotestore.go

@@ -1113,7 +1113,7 @@ func tryAddRemoteStoreIndexWithLocking(
 		if err != nil {
 			return false, longtaillib.Longtail_StoreIndex{}, errors.Wrap(err, fname)
 		}
-		if errors.Is(err, os.ErrNotExist) {
+		if longtaillib.IsNotExist(err) {
 			return false, longtaillib.Longtail_StoreIndex{}, nil
 		}
 
@@ -1605,7 +1605,7 @@ func readStoreStoreIndexFromPath(
 		return longtaillib.Longtail_StoreIndex{}, errors.Wrapf(err, fname)
 	}
 	if len(blobData) == 0 {
-		err = errors.Wrap(longtaillib.NotExistErr(), fmt.Sprintf("%s contains no data", key))
+		err = errors.Wrap(os.ErrNotExist, fmt.Sprintf("%s contains no data", key))
 		return longtaillib.Longtail_StoreIndex{}, errors.Wrap(err, fname)
 	}
 	storeIndex, err := longtaillib.ReadStoreIndexFromBuffer(blobData)
@@ -1657,10 +1657,14 @@ func mergeStoreIndexItems(
 	storeIndex := longtaillib.Longtail_StoreIndex{}
 	for _, item := range items {
 		tmpStoreIndex, err := readStoreStoreIndexFromPath(ctx, item, client)
+
 		if err != nil || (!tmpStoreIndex.IsValid()) {
-			// The file we expected is no longer there, tell caller that we need to try again
 			storeIndex.Dispose()
-			return longtaillib.Longtail_StoreIndex{}, nil, nil
+			if longtaillib.IsNotExist(err) {
+				// The file we expected is no longer there, tell caller that we need to try again
+				return longtaillib.Longtail_StoreIndex{}, nil, nil
+			}
+			return longtaillib.Longtail_StoreIndex{}, nil, errors.Wrap(err, fname)
 		}
 		if !storeIndex.IsValid() {
 			storeIndex = tmpStoreIndex
@@ -1774,7 +1778,7 @@ func readRemoteStoreIndex(
 					err = errors.Wrap(err, fmt.Sprintf("Cant parse optional store index from `%s`", optionalStoreIndexPath))
 					log.WithError(err).Info("Failed parsing optional store index")
 				}
-			} else if !errors.Is(err, os.ErrNotExist) {
+			} else if !longtaillib.IsNotExist(err) {
 				log.WithError(err).Info("Failed reading optional store index")
 			}
 		}

remotestore/remotestore_test.go

@@ -760,21 +760,18 @@ func testStoreIndexSync(blobStore longtailstorelib.BlobStore, t *testing.T) {
 
 				blocksIndex, err := longtaillib.CreateStoreIndexFromBlocks(blocks)
 				if err != nil {
-					wg.Done()
 					t.Errorf("longtaillib.CreateStoreIndexFromBlocks() failed with %s", err)
 				}
 				newStoreIndex, err := addToRemoteStoreIndex(context.Background(), client, blocksIndex)
 				blocksIndex.Dispose()
 				if err != nil {
-					wg.Done()
 					t.Errorf("addToRemoteStoreIndex() failed with %s", err)
 				}
 				newStoreIndex.Dispose()
 			}
 
 			readStoreIndex, _, err := readStoreStoreIndexWithItems(context.Background(), client)
 			if err != nil {
-				wg.Done()
 				t.Errorf("readStoreStoreIndexWithItems() failed with %s", err)
 			}
 			readStoreIndex.Dispose()
@@ -787,12 +784,10 @@ func testStoreIndexSync(blobStore longtailstorelib.BlobStore, t *testing.T) {
 				}
 				generatedBlocksIndex, err = longtaillib.CreateStoreIndexFromBlocks(blocks)
 				if err != nil {
-					wg.Done()
 					t.Errorf("longtaillib.CreateStoreIndexFromBlocks() failed with %s", err)
 				}
 				newStoreIndex, err := addToRemoteStoreIndex(context.Background(), client, generatedBlocksIndex)
 				if err != nil {
-					wg.Done()
 					t.Errorf("addToRemoteStoreIndex() failed with %s", err)
 				}
 				newStoreIndex.Dispose()