iojs/io.js — pull request #173 by bajtos (2014-12-16 12:12:34+00:00)
Title: Support promises in `fs` module

This the first experimental step towards fully promise-enabled core API, see #11. The solution presented in this pull request has the following goals:

- A promise API that works with ES6 generators (e.g. via [co](https://www.npmjs.com/package/co)) and that will work with ES7 async/await.
- The implementation is fully backwards compatible (with a small edge case - see below).
- The performance of existing callback-based code is not affected.
- The performance of promise-based code is reasonably good considering the the current state of Promise implementation in V8.

The promise API is disabled by default, one has to run `node --promisify-core` to enable it.

Note: I am intentionally focusing on the API where the conversion to promises is straightforward. Areas like streams and event-emitters, where the desired API is not clear yet, are completely out of scope of this work.

**Tasks**
- [x] Initial spike to discuss the overall implementation (`fs.stat`, `fs.readFile`)
- [ ] Modify remaining functions: `access`, `exists`, `close`, `open`, `read`, `write`, `rename`, `truncate`, `ftruncate`, `rmdir`, `fdatasync`, `fsync`, `mkdir`, `readdir`, `fstat`, `lstat`, `readLink`, `symlink`, `link`, `unlink`, `fchmod`, `chmod`, `fchown`, `chown`, `utimes`, `futimes`, `writeFile`, `appendFile`, `realPath`
- [ ] Final code cleanup, remove possibly temporal entities like `makeCallback` vs. `makeCallbackOrPromise`.

Out of scope: `fs.watch`, `fs.unwatchFile`.

**Breaking change**

At the moment, when an async function is invoked without a callback, it will:

- silently discard the result on success
- throw an error on failure, this error goes to "unhandled error"
handler (domain or process)

The PR changes the error behaviour: the rejected promise will be silently discarded, because the current version of V8 does not provide any way for detecting rejected promises that don't have any `catch` handler.

In the future, if V8 and/or the Promise standard provide a way for detecting unhandled rejections and Node integrates that mechanism, then this behaviour will be automatically reverted back.

**Performance**

Callback-based code is not affected by the change (even when `--promisify-core` is turned on), promise-based code is approx. 2x slower.

Benchmark results from my machine (rMBP; 2.7 GHz Intel Core i7; 16 GB 1600 MHz DDR3) are below, you are very welcome to run the benchmark yourself too. The rate "0.0000" means that a configuration was skipped as not available.

```
$ out/Release/node benchmark/fs/stat.js
fs/stat.js dur=5 concurrent=1 variant=promise: 0.0000
fs/stat.js dur=5 concurrent=1 variant=callback: 14944
fs/stat.js dur=5 concurrent=10 variant=promise: 0.0000
fs/stat.js dur=5 concurrent=10 variant=callback: 38148
fs/stat.js dur=5 concurrent=100 variant=promise: 0.0000
fs/stat.js dur=5 concurrent=100 variant=callback: 38725

$ out/Release/node --promisify-core benchmark/fs/stat.js
fs/stat.js dur=5 concurrent=1 variant=promise: 14651
fs/stat.js dur=5 concurrent=1 variant=callback: 16885
fs/stat.js dur=5 concurrent=10 variant=promise: 19011
fs/stat.js dur=5 concurrent=10 variant=callback: 38020
fs/stat.js dur=5 concurrent=100 variant=promise: 19353
fs/stat.js dur=5 concurrent=100 variant=callback: 41552
```

**Additional information**

Before I started the implementation, I run a benchmark to compare different ways of adding promises to `fs.stat`. See the following gist for more details: https://gist.github.com/bajtos/f8fca4fad36ada8e21dc

An example of using the new API with `co` and `yield`:

```
var co = require('co');
var fs = require('fs');

co(sourceAndStat);

function* sourceAndStat() {
var stat = yield fs.stat(__filename);
console.log('FILE SIZE: %s bytes\n', stat.size);
var source = yield fs.readFile(__filename, 'utf-8');
console.log(source);
}
```

---

@piscisaureus @bnoordhuis @trevnorris PTAL and let me know your opinion. I'd like to iron out the overall design before I start modifying the remaining 30 functions and writing unit-tests for them.

Things to consider:
- Is the option name `--promisify-core` revealing the intention? Should it be documented via `node --help` as proposed here, or should it be an undocumented flag?
- Testing strategy: How much tests do we need for the promise version of the API? Do we need to cover all edge cases, or is it enough to cover the most important paths (success, failure, optional arguments)? Is it better to mix promise tests with regular tests (as in `test-fs-stat.js`) or move them to a standalone file (`test-fs-readfile-promise.js`)?