Repository: peak/s5cmd
Pull request #463 (2022-07-07 14:33:52)

Change cp error not being descriptive
With this fix, when "given object not found" error message will also include absolute path of the file.

Fixes https://github.com/peak/s5cmd/issues/461

sonmezonur: @boraberke Thanks, LGTM! Could you describe this change in CHANGELOG?

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0e51837..9212427 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 ## not released yet
 
+#### Bugfixes
+- Changed cp error message to be more precise. "given object not found" error message now will also include absolute path of the file. ([#463](https://github.com/peak/s5cmd/pull/463))
+
 #### Improvements
 - Disable AWS SDK logger if log level is not "trace"
 
diff --git a/command/cp.go b/command/cp.go
index aa78e41..9072367 100644
--- a/command/cp.go
+++ b/command/cp.go
@@ -2,6 +2,7 @@ package command
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -749,8 +750,12 @@ func prepareLocalDestination(
 	}
 
 	obj, err := client.Stat(ctx, dsturl)
-	if err != nil && err != storage.ErrGivenObjectNotFound {
-		return nil, err
+
+	if err != nil {
+		var objNotFound *storage.ErrGivenObjectNotFound
+		if !errors.As(err, &objNotFound) {
+			return nil, err
+		}
 	}
 
 	if isBatch && !flatten {
@@ -760,8 +765,8 @@ func prepareLocalDestination(
 			return nil, err
 		}
 	}
-
-	if err == storage.ErrGivenObjectNotFound {
+	var objNotFound *storage.ErrGivenObjectNotFound
+	if errors.As(err, &objNotFound) {
 		err := client.MkdirAll(dsturl.Dir())
 		if err != nil {
 			return nil, err
@@ -782,7 +787,8 @@ func prepareLocalDestination(
 // found, error and returning object would be nil.
 func getObject(ctx context.Context, url *url.URL, client storage.Storage) (*storage.Object, error) {
 	obj, err := client.Stat(ctx, url)
-	if err == storage.ErrGivenObjectNotFound {
+	var objNotFound *storage.ErrGivenObjectNotFound
+	if errors.As(err, &objNotFound) {
 		return nil, nil
 	}
 
diff --git a/command/expand.go b/command/expand.go
index 838ca35..f58c295 100644
--- a/command/expand.go
+++ b/command/expand.go
@@ -2,6 +2,7 @@ package command
 
 import (
 	"context"
+	"errors"
 	"sync"
 
 	"github.com/peak/s5cmd/atomic"
@@ -68,7 +69,8 @@ func expandSources(
 
 				objch, err := expandSource(ctx, client, followSymlinks, origSrc)
 				if err != nil {
-					if err != storage.ErrGivenObjectNotFound {
+					var objNotFound *storage.ErrGivenObjectNotFound
+					if !errors.As(err, &objNotFound) {
 						ch <- &storage.Object{Err: err}
 					}
 					return
diff --git a/storage/fs.go b/storage/fs.go
index 37ce48b..628244a 100644
--- a/storage/fs.go
+++ b/storage/fs.go
@@ -22,7 +22,7 @@ func (f *Filesystem) Stat(ctx context.Context, url *url.URL) (*Object, error) {
 	st, err := os.Stat(url.Absolute())
 	if err != nil {
 		if os.IsNotExist(err) {
-			return nil, ErrGivenObjectNotFound
+			return nil, &ErrGivenObjectNotFound{ObjectAbsPath: url.Absolute()}
 		}
 		return nil, err
 	}
diff --git a/storage/s3.go b/storage/s3.go
index 9625176..09dc695 100644
--- a/storage/s3.go
+++ b/storage/s3.go
@@ -118,7 +118,7 @@ func (s *S3) Stat(ctx context.Context, url *url.URL) (*Object, error) {
 	})
 	if err != nil {
 		if errHasCode(err, "NotFound") {
-			return nil, ErrGivenObjectNotFound
+			return nil, &ErrGivenObjectNotFound{ObjectAbsPath: url.Absolute()}
 		}
 		return nil, err
 	}
diff --git a/storage/storage.go b/storage/storage.go
index 8df6b46..1b56780 100644
--- a/storage/storage.go
+++ b/storage/storage.go
@@ -14,13 +14,20 @@ import (
 )
 
 var (
-	// ErrGivenObjectNotFound indicates a specified object is not found.
-	ErrGivenObjectNotFound = fmt.Errorf("given object not found")
 
 	// ErrNoObjectFound indicates there are no objects found from a given directory.
 	ErrNoObjectFound = fmt.Errorf("no object found")
 )
 
+// ErrGivenObjectNotFound indicates a specified object is not found.
+type ErrGivenObjectNotFound struct {
+	ObjectAbsPath string
+}
+
+func (e *ErrGivenObjectNotFound) Error() string {
+	return fmt.Sprintf("given object %v not found", e.ObjectAbsPath)
+}
+
 // Storage is an interface for storage operations that is common
 // to local filesystem and remote object storage.
 type Storage interface {


Test patch:
diff --git a/e2e/cp_test.go b/e2e/cp_test.go
index fe3a60a..5d1540f 100644
--- a/e2e/cp_test.go
+++ b/e2e/cp_test.go
@@ -2802,6 +2802,37 @@ func TestCopyWithFollowSymlink(t *testing.T) {
 	assert.Assert(t, ensureS3Object(s3client, bucket, "prefix/c/link2", fileContent))
 }
 
+func TestCopyErrorWhenGivenObjectIsNotFoundUsingWildcard(t *testing.T) {
+	t.Parallel()
+
+	s3client, s5cmd, cleanup := setup(t)
+	defer cleanup()
+
+	const bucket = "bucket"
+	createBucket(t, s3client, bucket)
+
+	folderLayout := []fs.PathOp{
+		// we intentionally did not create a/f1.txt to
+		// trigger given object not found error.
+		fs.WithDir("b"),
+		fs.WithSymlink("b/link1", "a/f1.txt"),
+	}
+
+	workdir := fs.NewDir(t, t.Name(), folderLayout...)
+	defer workdir.Remove()
+
+	dst := fmt.Sprintf("s3://%v/prefix/", bucket)
+
+	cmd := s5cmd("cp", "*", dst)
+	result := icmd.RunCmd(cmd, withWorkingDir(workdir))
+
+	result.Assert(t, icmd.Expected{ExitCode: 1})
+
+	assertLines(t, result.Stderr(), map[int]compareFunc{
+		0: equals(`ERROR "cp * %v": given object b/link1 not found`, dst),
+	}, sortInput(true))
+}
+
 // cp --no-follow-symlinks * s3://bucket/prefix/
 func TestCopyWithNoFollowSymlink(t *testing.T) {
 	t.Parallel()
diff --git a/e2e/run_test.go b/e2e/run_test.go
index b695222..e78ba2a 100644
--- a/e2e/run_test.go
+++ b/e2e/run_test.go
@@ -98,7 +98,7 @@ func TestRunFromStdinWithErrors(t *testing.T) {
 
 	assertLines(t, result.Stderr(), map[int]compareFunc{
 		0: contains(`ERROR "cp s3://%v/nonexistentobject nonexistentobject": NoSuchKey: status code: 404`, bucket),
-		1: equals(`ERROR "ls s3/": given object not found`),
+		1: equals(`ERROR "ls s3/": given object s3/ not found`),
 	}, sortInput(true))
 }