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

fs: bad error message in fs.openSync #2871

Closed
ronkorving opened this issue Sep 15, 2015 · 6 comments
Closed

fs: bad error message in fs.openSync #2871

ronkorving opened this issue Sep 15, 2015 · 6 comments
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@ronkorving
Copy link
Contributor

Calling:

fs.openSync(path, { flags: 'w' });

Throws: TypeError: flags must be an int from native land.
While on the C++ side, that may be correct, in JavaScript land, the 2nd argument is not an int but a string.

In other words, yes, me passing an object was bad, but the error informed me to do the wrong thing compared to what the docs say.

@reqshark
Copy link

fs flags are a string, not object, like going require('fs').readFile(dude, 'utf8', cb)

console.log(require('fs').openSync('what.js', 'w'))

@reqshark
Copy link

but yea the error definitely says int when you give it objects

@brendanashworth brendanashworth added the fs Issues and PRs related to the fs subsystem / file system. label Sep 15, 2015
@yjhjstz
Copy link

yjhjstz commented Sep 15, 2015

should we throw new Error('Unknown file open flag: ' + flag); in stringToFlags?

@ronkorving
Copy link
Contributor Author

I think it should throw a TypeError if not string and not integer (Number.isInteger is now at our disposal). However, this is also used in the asynchronous fs.open, so we need to be careful with exceptions.

In fact, I just realized that's already broken in fs.open and fs.readFile, since stringToFlags already throws. See Line 251 and Line 541.

@ronkorving
Copy link
Contributor Author

I'll conjure up a PR to fix this.

ronkorving pushed a commit to ronkorving/node that referenced this issue Sep 16, 2015
Before, it would show "TypeError: flags must be an int", which
in native Node is technically correct. But in JS they may be
(and usually are) strings.

Fixes nodejs#2871
@jasnell
Copy link
Member

jasnell commented Mar 9, 2016

Fixed by #5590

@jasnell jasnell closed this as completed Mar 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants