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.readFile() does not handle exceptions properly with extensions #634

Closed
gcampax opened this issue Nov 8, 2015 · 5 comments
Closed

fs.readFile() does not handle exceptions properly with extensions #634

gcampax opened this issue Nov 8, 2015 · 5 comments

Comments

@gcampax
Copy link
Contributor

gcampax commented Nov 8, 2015

If a fs extension (such as the one introduced by jxcore-android/jxcore-cordova) throws an exception in readfilesync, mostly likely NOENT, readFile() crashes instead of calling the callback with the exception, which lets the caller handle it.

@obastemur
Copy link
Member

/cc @ktrzeciaknubisa (high priority)

@ktrzeciaknubisa
Copy link
Member

Personally I'm aware of this and I was even warning about this before. In fact, as far as we talk about jxcore-cordova, this was even fixed by commit and then reverted back. Here is some discussion.

But I believe this is dependent on how the extension method itself is implemented. Not much to do with extensions internals (fs.js). Am I mistaken @gcampax ?

@gcampax
Copy link
Contributor Author

gcampax commented Nov 17, 2015

Well, if your extension interface is defined as "read the file or throw an error" - because that's what the sync methods do, then it's up to the async wrapper to turn "throw an error" into "call the callback with an error"

@ktrzeciaknubisa
Copy link
Member

You're right @gcampax . The above commit takes care of it internally. However to have this patch in jxcore-android/jxcore-cordova, you need to wait for new binaries release (or compile by yourself).

Thanks!

@gcampax
Copy link
Contributor Author

gcampax commented Nov 27, 2015

Oh I'm compiling everything from source so it's not a problem.

Thanks for addressing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants