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

Copy clobber flag never err even when true #272

Closed
ryancmlee opened this issue Sep 10, 2016 · 9 comments
Closed

Copy clobber flag never err even when true #272

ryancmlee opened this issue Sep 10, 2016 · 9 comments
Assignees

Comments

@ryancmlee
Copy link

ryancmlee commented Sep 10, 2016

In the following code err is never not null:

fs.copy(sourcePath, targetPath, { clobber: true  }, (err) =>
{
    if (err)
        debugger
    fs.copy(sourcePath, targetPath, { clobber: true}, (err) =>
    {
        if (err)
            debugger
        fs.copy(sourcePath, targetPath, { clobber: false}, (err) =>
        {
            if (err)
                debugger
            fs.copy(sourcePath, targetPath, { clobber: false }, (err) =>
            {
                if (err)
                    debugger
            })
        })
    })
})

Maybe this is intended but then at least add the option for open flags so I can at least put wx and have it err.code = "EEXIST"

Thanks,
Ryan

@ryancmlee ryancmlee changed the title copy clobber flag has no effect Copy clobber flag never err even when true Sep 10, 2016
@qqilihq
Copy link

qqilihq commented Sep 27, 2016

I'm asking myself as well:

Is there any way at all to determine, if the file content was copied, when clobber is set to false?

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 25, 2016

@ryancmlee @qqilihq Good points. @jprichardson Thoughts?

@jprichardson
Copy link
Owner

Yep, this is a bug. If clobber is false and the destination exists, there should be an Error.

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 25, 2016

OK, will investigate.

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 25, 2016

The issue is https://github.com/jprichardson/node-fs-extra/blob/master/lib/copy/ncp.js#L92. @jprichardson Should I attempt to create a fake EEXIST error here or what?

@jprichardson
Copy link
Owner

@RyanZim yep, I think that'd be fine.

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 25, 2016

@jprichardson Creating a fake SystemError isn't easy, and I don't think I can replicate it completely. Advice?

@jprichardson
Copy link
Owner

@RyanZim what about...

const err = new Error(file + ' already exists.')
err.code = EEXIST

?

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 25, 2016

I guess that will cover most cases; still will have a JS stack trace, IDK.

Should also set err.errno = -17 & err.path = file.

@RyanZim RyanZim self-assigned this Oct 25, 2016
RyanZim added a commit that referenced this issue Oct 26, 2016
- Fix #233 - First argument passed to callback should be an Error
  object, not an array.
- Fix #272 - Should error when options.clobber is false and the
  destination exists.
- Remove options.limit, it is no longer needed with graceful-fs.
RyanZim added a commit that referenced this issue Oct 26, 2016
- Fix #233 - First argument passed to callback should be an Error
  object, not an array.
- Fix #272 - Should error when options.clobber is false and the
  destination exists.
- Remove options.limit, it is no longer needed with graceful-fs.
RyanZim added a commit that referenced this issue Oct 27, 2016
- Fix #233 - First argument passed to callback should be an Error
  object, not an array.
- Fix #272 - Should error when options.clobber is false and the
  destination exists.
- Remove options.limit, it is no longer needed with graceful-fs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants