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.createWriteStream is not failing when folder doesn't exist #24688

Closed
ORESoftware opened this issue Nov 28, 2018 · 10 comments
Closed

fs.createWriteStream is not failing when folder doesn't exist #24688

ORESoftware opened this issue Nov 28, 2018 · 10 comments

Comments

@ORESoftware
Copy link
Contributor

ORESoftware commented Nov 28, 2018

I have this code:

const fs = require('fs');
const path = require('path');
const file = path.resolve(outputDir + '/foo.js');
const w = fs.createWriteStream(file);
w.write('foop');
w.end('\n');

for some bizarre reason, even if outputDir does not exist, it will not emit an error, and the file won't get created. If outputDir does exist, the file will get created but nothing will get written to it.

I can only replicate this in one project, on both node version 10 and 11, on Ubuntu.

@ORESoftware
Copy link
Contributor Author

I also checked this:

w.write('zooooom', function(){
  console.log('flushed');
});

the flushed callback never fires either.

@ORESoftware
Copy link
Contributor Author

ORESoftware commented Nov 28, 2018

I tried fs.appendFile, same exact phenomenon. If the folder doesn't exist, no error. And if the folder does exist, the file gets created but doesn't get written to. I tried writing to multiple different destination folders/files, same thing. Weird.

However fs.appendFileSync works! My only guess would be if some env var was affecting async i/o, but that seems unlikely, I have no idea.

@TomCoded
Copy link
Contributor

I was unable to replicate the behavior where you call fs.createWriteStream() on a filename in a nonexistent directory and do not get any error.

Does the problem persist if you move the failing code segment to a new directory? If you isolate it from the rest of the project it is in? What are you using for outputDir? Does file have the value you are expecting after calling path.resolve()?

In the appendFile case, does the user running node have write permissions on the newly created file?

@ORESoftware
Copy link
Contributor Author

ORESoftware commented Nov 29, 2018

@TomCoded I figured it out, bit of a headache tho. I was calling

process.exit(0)

and that was preventing the streams from finishing writing. I wasn't calling in the same tick, but the next tick, still not enough time to finish writing or create files or throw errors I guess.
This explains why fs.appendFileSync was working but fs.appendFile was having the same issue.

Say I create an arbitrary number of writable streams. I can let the event loop empty and let node exit on it's own, or I could try to listen for streams to finish writing, something like this:

const promiseList = [];

const createStrmPromise = function(file){
   const strm = fs.createWriteStream(file);
    promiseList.push(new Promise((res, rej) => {
          strm.once('end', res).once('error',rej);
    }));
   return strm;
}

then before I exit, just do something like:

Promise.all(promiseList)
.then(() => process.exit());

@ORESoftware
Copy link
Contributor Author

ORESoftware commented Nov 29, 2018

My only suggestion - can node.js warn if there are still tasks on the event loop when process.exit() is called?

@ORESoftware ORESoftware reopened this Nov 29, 2018
@apapirovski
Copy link
Member

My only suggestion - can node.js warn if there are still tasks on the event loop when process.exit() is called?

That would be rather strange as the user is clearly opting in to quit the process when calling process.exit(0).

If you're concerned about the event loop clearing then just let it exit naturally?

@ORESoftware
Copy link
Contributor Author

ORESoftware commented Nov 29, 2018

@apapirovski ultimately the issue is about libraries. If you have a library and some user code is incorporated - you want to force exit even if the user did some dumb sh*t, but you don't want to force exit the library code.

Let me give you another scenario - say you want to exit with 1, but you want to wait until the event loop empties?

I guess you can do this?

process.once('exit', code => {
   process.exitCode = 1;
});

So the second scenario is solvable, but the scenario where user code is mixed with library code is not as clear to me - how does the library shutdown user code without force shutting down the library calls?

@apapirovski
Copy link
Member

Correct. That would be the documented way to do it.

In terms of libraries — you'll just have to make sure to setup the appropriate callbacks for your writes so that you track the lifecycle of those. The end event can fire before the actual write makes it on the wire unfortunately.

@TomCoded
Copy link
Contributor

On the warning, docs on process.on('exit') and process.exit() make it clear that events get discarded, so IMHO it would be throwing a warning over a common and expected code pattern. (i.e. programs calling process.exit() to terminate early regardless of what is in the event loop).

It should almost never be necessary to call process.exit() if the event loop is empty (since node will terminate automatically), so the warning would be emitted in most scenarios where process.exit() needs to be called.

Which will admittedly make it a little harder to debug your scenario than having a warning, but it is the expected behavior.

Your Promise.all() thought and @apapirovski on appropriate callbacks strike me as right in terms of the library having to keep track of the execution paths you want to make sure are completed before you force an exit.

@ORESoftware
Copy link
Contributor Author

ORESoftware commented Nov 29, 2018

@apapirovski well I was saying - the library can setup the callbacks, but let's say the user forgets to fire a callback etc. Anyway, I think is a non-issue so I will close, but @apapirovski @TomCoded thanks for the feedback.

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