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

Attempting to open a non-existant file hard-crashes Node #16

Closed
ghost opened this issue May 6, 2014 · 6 comments
Closed

Attempting to open a non-existant file hard-crashes Node #16

ghost opened this issue May 6, 2014 · 6 comments

Comments

@ghost
Copy link

ghost commented May 6, 2014

They way the internal open function is implemented, it simply throws any error it receives when attempting to open a file. This is a major problem since that error is in a OS-level callback and therefore can't be handled by the caller. Basically, a missing file can crash your entire app.

Here's a brief code example which reproduces the problem:

var lineReader = require("line-reader");

try {
    lineReader.eachLine('missing file', function(line, isLast) {
        console.log("got a line: " + line);
    }).then(function () {
        console.log("all done!");
    });
} catch (e) {
    console.log("got an exception: " + e);
}
@ghost
Copy link
Author

ghost commented May 6, 2014

This seems to be a specific example of #7.

@scaryguy
Copy link

Any update or work around on this?

@scharf
Copy link

scharf commented Jan 28, 2015

The API is not following the node standard, because it does not use error as first argument in the callback.

@jedwards1211
Copy link
Contributor

I was thinking about this, I think there should just be an (error, result) callback after the (line, last) callback in the arguments to eachLine(), plus the other options should be supplied as a hash that comes before this last callback. This callback would be called at the very end of everything. That way the function could be auto-promisified by libraries like bluebird (and the .then() handler could be removed since it doesn't follow any promise standards either). If error was provided as the first argument to the per-line callback, then it couldn't be auto-promisified.

I looks to me like Nick Ewing doesn't intend to actively maintain this...but he has merged PRs somewhat recently.

@nickewing
Copy link
Owner

@jedwards1211 You're right, I haven't had time to do fixes myself and I haven't been working with node recently. I am very happy to review and accept PRs however!

@jedwards1211
Copy link
Contributor

Yeah, understandable! Well if I find time soon I'll see if I can make a PR for this.

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