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

Graceful FS by default #167

Closed
domenic opened this issue Dec 15, 2014 · 4 comments
Closed

Graceful FS by default #167

domenic opened this issue Dec 15, 2014 · 4 comments

Comments

@domenic
Copy link
Contributor

domenic commented Dec 15, 2014

The current fs module is a footgun waiting to shoot you in the foot as soon as you start opening too many files. As projects grow, they inevitably need to transition to @isaacs's graceful-fs. We've seen this happen with npm and others.

It would be great if Node's filesystem manipulations were reliable by default, and people didn't have to discover this. In other words: it probably should be part of the boilerplate for every new Node program to install and require graceful-fs. However, this isn't really happening, because it's only something you run into "in production" or "at scale." It would be better to roll it into core.

As a note, it would be interesting to hear about whether other standard libraries are graceful or fail, to see how Node compares. Evidence on either side could help clarify whether this proposal is a good idea.

@lxe
Copy link

lxe commented Dec 16, 2014

This could be a bit of a slippery slope... I actually agree that a lot of community modules should be a part of the standard library, but I don't think the community shares my sentiment.

@reqshark
Copy link

-1

@isaacs
Copy link
Contributor

isaacs commented Dec 16, 2014

Big -1 on this.

Maybe some of the benefits of graceful-fs polyfills should be included (like nerfing fs.lchmod and friends on platforms that lack them).

However, replacing "fails with EMFILE" with "hangs until fs.close" is a terrible and surprising default. If your module can be expected to close some files eventually, and if it's better to wait for that to happen rather than fail up front, then graceful-fs is great. But given the impacts of the "bad old days" when graceful-fs used to monkey-patch the builtin, and the havoc that caused when people would use glob in their modules, I think it's best to not do that by default.

I'm gonna close this for now, in anticipation of other TC members agreeing with me, but I'm happy to have it re-opened and slated for discussion if it turns out I'm incorrect on that assumption. If we limit the discussion to just the graceful-fs polyfills, and not the EMFILE monkeypatching, then I'm open to it, but that should be a separate issue.

@isaacs isaacs closed this as completed Dec 16, 2014
@matthew-dean
Copy link

IMHO EMFILEis the biggest problem with the fs module. It's one of the most common errors people encounter (https://www.google.ca/#q=node+emfile), and one of the reasons people often switch to graceful-fs.

The docs have said for a long time "To remedy a low limit, run ulimit -n 2048 in the same shell that will run the io.js process." That's great, if you can have permission to run ulimit, and if you're in a shell, and if your operating system will do anything at all if you run that command, which some versions of OS X don't. In short, it's a non-solution to the problem.

Sure, there are other ways to avoid EMFILE errors, but it's not obvious from reading the documentation. Graceful-fs doesn't force the developer to know advanced methods of batching opens and limiting file descriptors in order to use async FS methods. If you don't have a PhD in computer science, the FS module is kind of difficult at the moment.

I don't think the solution is adopting graceful-fs in full. But surely it's obvious that it's addressing real and common problems that developers are often encountering using the FS module for node / io that could be better solved by the core module itself.

boingoing pushed a commit to boingoing/node that referenced this issue Apr 6, 2017
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

5 participants