Node: Fix or Mark apis in the documentation that cannot be try/catch/promised

[haraldrudell]

Certain library objects that extend EventEmitter emits error on callback that are not caught by a try/catch or Promise. If it is not possible to address this in code somehow, those objects and methods should be marked in the documentation as requiring an error listener.

IMPACT:
Node.js throws uncaught exception in edge-case situations

Example 1: failing dns request

node --eval "new Promise(r => new (require('http').Server)().listen(0, '#$%'))"
events.js:160
      throw er; // Unhandled 'error' event
      ^

Error: getaddrinfo ENOTFOUND #$%
    at errnoException (dns.js:28:10)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:79:26)

Example 2: failed spawn

node --eval "new Promise(r => require('child_process').spawn('error factory'))"
events.js:160
      throw er; // Unhandled 'error' event
      ^

Error: spawn error factory ENOENT
    at exports._errnoException (util.js:1026:11)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:182:32)
    at onErrorNT (internal/child_process.js:348:16)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)

uname --all && node --version
Linux c89 4.4.0-34-generic #53-Ubuntu SMP Wed Jul 27 16:06:39 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
v6.4.0

[Fishrock123]

"Fixing" it is out of the question. Many people rely on this behavior and the APIs are designed with it in mind.

That is to say, it is expected behavior in the context of the API. I think we could document it for now.

Moving towards an API that is easier to wrap without similar patters would require larger discussion and is unlikely to become a reality in the short term.

[haraldrudell]

Is there a discussion somewhere of promisifying libuv?
To make that transition smooth, it could be a babel-like project.

[Fishrock123]

@haraldrudell libuv is written in C. :)

Writing it in JS is not exactly possible, you're interacting with lots of syscalls at that level, just like a browser VM.

[haraldrudell]

I am interested in what the plan is and what the majority and I should be doing.

1. Node.js api has around 30,000 lines of JavaScript at /usr/lib/nodejs
2. Possibly every second api is a callback api
3. When people discover async, which has been available to daredevils since September, 2015, the direct usage of callback apis will decrease by approximately 100%. However, all code to date rely on the present api to not change for all eternity.
4. Right now, every person is in every one of their source files promisifying the apis they use, sometimes leading to mysterious problems like the ones listed above. It seems like more JavaScript, written once to almost every callback api, will be the least work.
5. How do I find out what the plan for a more efficient and organized solution is?

[benjamingr]

The plan for a more organized solution for event emitters would probably be https://github.com/tc39/proposal-observable and https://github.com/tc39/proposal-async-iteration .

I'm also strongly -1 on changing this behavior. error events are not only documented - they're useful with long stack traces and their behavior is relied upon.

[haraldrudell]

To avoid those pesky error emits on callback that needs custom code, one could wrap http.get likeso:

import {get} from 'http'
export class http {
  static async get(url) {
    return new Promise((resolve, reject) => {
      get(url, resolve).on('error', reject) // error-listener for socket errors like ECONNRESET
    })
  }
}

http.get('http://google.com')
  .then(response => console.log(`status code: ${response.statusCode}`))
  .catch(e => console.error('error:', e))

This is what I mean with an all-promisified library. Basically, every Node.js produced object should have an error listener that rejects the promise.

Or the api could change altogether.

[joyeecheung]

When people discover async, which has been available to daredevils since September, 2015, the direct usage of callback apis will decrease by approximately 100%

As someone who has been using co(like async/await desugaring done in user land) for quite some time before async/await started to get implemented, I don't really think this will be the case. There are still many folks who prefer the simplicity and flexibility of callbacks to the overhead and restriction brought by ES promises/async/await, and I can understand their counter arguments. There is always trade-off.

Providing a promisified version of the APIs is still open(stalled) in https://github.com/nodejs/node/pull/5020 . If you would like to help that direction to move, maybe it's more appropriate for https://github.com/nodejs/promises ? (Not sure if that working group is still active). There are a lot of previous discussions about this kind of API change(warning: tons of text): https://github.com/nodejs/NG/issues/25 https://github.com/nodejs/node/issues/11

At the moment adding side notes in the documentation about the caveats of promisifying them in userland could be a better way to do this. You can sending some PRs when you bump into emitters like this(see the contributing guide, the API docs are markdown files under /doc/api with one-to-one naming).

[haraldrudell]

async functions and promises are not the same.

Promisifying the Node.js api, for the first time, offers safe code. This bug is about that wholesale promisify does not work for the Node.js api because of the api’s primitive handling of errors. Promisifying requires intimate knowledge of each api.

http.get is an excellent example:
a. it is not obvious that this api returns an object emitting uncatchable exceptions.
b. It also illustrates the bug prone constructs required to handle errors right in the documentation here: https://nodejs.org/api/http.html#http_http_get_options_callback

• parsing JSON is 6 lines in obsolete-style callback ECMAScript
• inside a promise it’s a single line

[joyeecheung]

I think it's more about the way the promisify layer is implemented, so it is more approachable via documentation changes. When using generators before async/await came along people usually need to install packages that wrap a part of Node.js API into generators(with this kind of knowledge integrated), so IMHO for async/await(or simply just Promises) it's still the same. It is too hard at this point to just change the old APIs, promisifying them as more of a semver-minor thing is easier, and async/await support can benefit from promisification(less code to write), no?

Promisifying the Node.js api, for the first time, offers safe code

I'm not sure why it is safer than generators that wraps around the Node.js API with async/await-like sugar on top? Generators are not that preferred these days because they are more like a temporary hack before promises and async/await get implemented, but you can try-catch and don't need to worry about swallowed errors just the same.

[benjamingr]

Please refer to https://github.com/nodejs/CTC/issues/12

[Trott]

This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.

[benjamingr]

@Trott moreover, this was actually fixed about 2 months ago by @addaleax when we landed https://github.com/nodejs/CTC/issues/12