fs.createWriteStream: add util.promisify support.

[coreyfarrell]

• Version: v8.9.0
• Platform: Linux lt1.cfware.com 4.13.10-200.fc26.x86_64 deps: update openssl to 1.0.1j #1 SMP Fri Oct 27 15:34:40 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem: fs

#!/usr/bin/env node
'use strict';

const util = require('util');
const fs = require('fs');
const createWriteStream = util.promisify(fs.createWriteStream);
const {spawn} = require('child_process');

async function main() {
  const log = await createWriteStream('output.log');
  spawn('ls', ['-lh'], { stdio: ['ignore', log, log]});
  return 'Done';
}

main().then(console.log).catch(console.error);

This causes 'output.log' to be created but empty, node.js exits silently.

I've created a "polyfill" which seems to work and do what I would expect:

const fs = require('fs');
const util = require('util');
fs.createWriteStream[util.promisify.custom] = (path, options) => {
  const stream = fs.createWriteStream(path, options);
  return new Promise((resolve, reject) => {
    stream.on('error', reject);
    stream.on('open', resolve);
  });
};

I'm not yet prepared to submit a pull request for this (it will be my first to node). I have code in lib/fs.js that I think correctly uses the internal version of the promises, I haven't had a chance to build. I'm not sure what automatic tests would be needed for this feature, I've only tested this manually. I'm reading the contributor guide but still wanted to submit the bug report now.

I also haven't had a chance to check out other fs API's like createReadStream, probably appropriate to have similar promisify support.

[benjamingr]

I don't think this makes sense since I'd expect createWriteStream to return a write stream - and I'd expect to add handlers to it synchronously.

If anything, you can propose promisifying once on EventEmitter or something of the sort?

[benjamingr]

Thanks a lot for bringing this up by the way.

[coreyfarrell]

@benjamingr I'm not sure I understand what you mean by promisifying once?

Also my idea behind promisifying createWriteStream is that the returned stream is the "ready" on return. Maybe this is a special case for spawn that it needs the file descriptors from the stream rather than the stream? The example code is very close to the inspiration for this report (open a file, pipe output of spawn to it).

Looking at the fs docs more maybe I should be using fs.open to get an FD & pass that to spawn instead of the writable stream?

[coreyfarrell]

@mscdex Should I post a separate issue for the bug part of this? Using the promisified version of a function that doesn't support promisify should throw a JS error, not silently exit node.

[eduardbme]

according to documentation util.promisify

Takes a function following the common Node.js callback style...

createWriteStream does not correspond to this specification, and honestly speaking I don't see any reason to use util.promisify on this function.

[coreyfarrell]

That's fine I've moved on, I used fs.open instead. But node should not crash if someone tries promisifying fs.createWriteStream.

[benjamingr]

@coreyfarrell we've updated the docs on what util.promisify does in any case in case that helps.

I'm going to close this since you're no longer pursuing, if you feel strongly against this let me know and I'll reopen.

Thanks for taking the time and making the effort by the way. Any feedback about how we can make a better Node promises story is very appreciated.

[coreyfarrell]

@benjamingr I do still think it is a bug that my example code exits silently. I no longer feel that it's worth while for the stream methods to work with util.promisify, but I think if someone (incorrectly) tries doing this an exception should be thrown or some other diagnostics. Run-time JS coding errors should not cause node.js to exit silently.

[rafaelrinaldi]

Just a heads up that I also expected promisify to work on writeable streams. Exiting silently was a bit confusing and unexpected as a module user.
I've followed @coreyfarrell's suggestion and am now using fs.open instead.