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

q.denodeify doesn't work for fs.exists #593

Closed
umarashfaq opened this Issue Oct 17, 2014 · 5 comments

Comments

Projects
None yet
3 participants
@umarashfaq

umarashfaq commented Oct 17, 2014

Unlike other fs method, fs.exists doesn't have error as first parameter in callback. Appending "then" to the denodeified method invocation ends up in the error callback with error description as "true", whenever file exists.

@arikon

This comment has been minimized.

arikon commented Oct 17, 2014

@umarashfaq So, what is your proposal?

@domenic

This comment has been minimized.

Collaborator

domenic commented Oct 17, 2014

Yep, q.denodeify only works on functions that follow the Node callback convention. Many functions do not, including fs.exists.

@domenic domenic closed this Oct 17, 2014

@umarashfaq

This comment has been minimized.

umarashfaq commented Oct 17, 2014

@arikon I'm hoping following code to work:

var fs_exists = q.denodeify(fs.exists);

fs_exists('/some/file/path')
.then(function( exists ){
    console.log('file exists: %s', exists);
}, function( err ) {
   console.log('An error occurred: %s', err);
});

But it doesn't really work the way I expected it to. I thought whether the file exists or not, control would flow into success callback, unless an error occurs, in which case, control would flow into failure callback. Which is not the case. Here is what happens:

When the file exists, controls flows into error callback and when it doesn't exist, the success callback is invoked. To make it work, I have to do something like this:

var fs_exists = q.denodeify(fs.exists);

fs_exists('/some/file/path')
.then(function( exists ){
    console.log('file doesn\'t exist');
}, function( err ) {
   console.log('file exists');
});

Let me know if I'm missing something or if it is deliberate and not a flaw. Otherwise if you think behavior I proposed above (first of the two snippets) looks viable, I can send you a pull request.

@domenic

This comment has been minimized.

Collaborator

domenic commented Oct 17, 2014

In general you should not use fs.exists anyway. You should either (a) just perform the operation you want to perform, and handle ENOENT errors if they occur; or (b) use fs.stat.

(a) is better because it prevents race conditions where the file stops existing between your call to fs.exists and the operation you're performing. (b) is still subject to race conditions, but fs.stat uses the Node callback convention.

@umarashfaq

This comment has been minimized.

umarashfaq commented Oct 17, 2014

@domenic Thanks for explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment