Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change cp error not being descriptive #463

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
16 changes: 11 additions & 5 deletions command/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package command

import (
"context"
"errors"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -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 givenObjNotFoundErr *storage.ErrGivenObjectNotFound
igungor marked this conversation as resolved.
Show resolved Hide resolved
if !errors.As(err, &givenObjNotFoundErr) {
return nil, err
}
}

if isBatch && !flatten {
Expand All @@ -760,8 +765,8 @@ func prepareLocalDestination(
return nil, err
}
}

if err == storage.ErrGivenObjectNotFound {
var givenObjNotFoundErr *storage.ErrGivenObjectNotFound
if errors.As(err, &givenObjNotFoundErr) {
err := client.MkdirAll(dsturl.Dir())
if err != nil {
return nil, err
Expand All @@ -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 givenObjNotFoundErr *storage.ErrGivenObjectNotFound
if errors.As(err, &givenObjNotFoundErr) {
return nil, nil
}

Expand Down
4 changes: 3 additions & 1 deletion command/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package command

import (
"context"
"errors"
"sync"

"github.com/peak/s5cmd/atomic"
Expand Down Expand Up @@ -68,7 +69,8 @@ func expandSources(

objch, err := expandSource(ctx, client, followSymlinks, origSrc)
if err != nil {
if err != storage.ErrGivenObjectNotFound {
var givenObjNotFoundErr *storage.ErrGivenObjectNotFound
if !errors.As(err, &givenObjNotFoundErr) {
ch <- &storage.Object{Err: err}
}
return
Expand Down
31 changes: 31 additions & 0 deletions e2e/cp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
boraberke marked this conversation as resolved.
Show resolved Hide resolved
//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()
Expand Down
2 changes: 1 addition & 1 deletion e2e/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down
2 changes: 1 addition & 1 deletion storage/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion storage/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
11 changes: 9 additions & 2 deletions storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
igungor marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down