cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

settle on rejected promises causes unclosed "potentially unhandled rejection" #423

Closed jodal closed 9 years ago

jodal commented 9 years ago

I'm not sure I've misunderstood something, or if this is weakness in the unhandled rejection checker: If I settle an array which contains rejected promises, these cause "potentially unhandled rejection" without a matching warning later with the same ID. The same behavior is observed with .then() and .done().

Example code:

var when = require('when');

var array = [when.reject(new Error(1)), 2, when(3), when.reject(new Error(4))];
var settled = when.settle(array);

settled.then(function (descriptors) {
  descriptors.forEach(function (d) {
    console.log(d);
  });
});

Output:

$ node test.js
{ state: 'rejected', reason: [Error: 1] }
{ state: 'fulfilled', value: 2 }
{ state: 'fulfilled', value: 3 }
{ state: 'rejected', reason: [Error: 4] }
Potentially unhandled rejection [1] Error: 1
    at Object.<anonymous> (/home/jodal/dev/mopidy.js/test.js:3:26)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:902:3
Potentially unhandled rejection [2] Error: 4
    at Object.<anonymous> (/home/jodal/dev/mopidy.js/test.js:3:65)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:902:3
jodal commented 9 years ago

Forgot to add, I'm using When.js 3.7.0, Node.js 0.10.25.

Real code in question is https://github.com/mopidy/mopidy.js/blob/master/test/mopidy-test.js#L225-L248.

briancavalier commented 9 years ago

@jodal Ah, yes, you're right. Thanks for the detailed report. The problem is that settle is taking a shortcut for already-settled promises--a shortcut that doesn't mark the rejected promise as having been observed. It should be an easy fix ... stay tuned :)

briancavalier commented 9 years ago

@jodal Fixed in master. Feel free to give it a try. I'll get a 3.7.1 release together shortly.

jodal commented 9 years ago

Thanks for the quick fix! I tested, and it looks good :-)

briancavalier commented 9 years ago

@jodal No prob, thanks for reporting it. 3.7.1 was released with this fix.

nickdima commented 9 years ago

I still seem to have this problem when one of the rejected promises comes from a function exported by a node module that uses it's own when. Both the parent module and the child module require the latest version 3.7.2. I resolved the issue by adding when as a peer dependency for the child module but I still think it should be looked at...

briancavalier commented 9 years ago

@nickdima Hmm, are you sure they are both using 3.7.2? I could see this happening if there is an older version of settle being called somewhere.

There is another possible explanation. Many promise impls (including when.js) distrust other implementations: they use fast/private code path when dealing with their own promises, and slower/safer paths when dealing with foreign promises. This is true across different versions of the same library or even two copies of the same version, since most use an instanceof check to identify their own promises.

One thing when.js does with foreign promises is to never call then in the current microtick, to protect the current call stack from side effects that might happen in the foreign promise's then. In your situation, maybe that extra microtick is just long enough to trigger an unhandled rejection report.

Question: Do you then see a subsequent "Handled previous rejection" message logged?

sobolevn commented 8 years ago

I have faced the same issue. Maybe I am missing something, since I'm not so familiar with promises. Here's the snippet:

const url = require('url');
const when = require('when');

function validate(ast, file, preferred, done) {
  const links = ['http://google.com', 'http://example.com', 'http://i-will-raise-rejection.promise'];
  const promises = [];

  for (let i in links) {
    const link = links[i];
    const options = {
      method: 'GET',
      timeout: 5000,
      uri: link,
      resolveWithFullResponse: true,
    };

    const promise = rp(options);
    promises.push(promise);
    promise
      .then(r => handleResponse(r, file, link))
      .catch(() => handleError(file, link));
  }

  // here I want to wait for every request to finish, before running the callback:
  when.settle(promises).then(() => done());
}

That works as expected, except this warning:

Potentially unhandled rejection [1] RequestError: Error: ETIMEDOUT at new RequestError (/Users/sobolev/Documents/github/remark-lint-is-link-active/node_modules/request-promise/lib/errors.js:14:15) at Request.RP$callback as _callback at self.callback (/Users/sobolev/Documents/github/remark-lint-is-link-active/node_modules/request/request.js:200:22) at emitOne (events.js:90:13) at Request.emit (events.js:182:7) at null._onTimeout (/Users/sobolev/Documents/github/remark-lint-is-link-active/node_modules/request/request.js:775:12) at Timer.listOnTimeout (timers.js:92:15) Potentially unhandled rejection [3] RequestError: Error: ETIMEDOUT at new RequestError (/Users/sobolev/Documents/github/remark-lint-is-link-active/node_modules/request-promise/lib/errors.js:14:15) at Request.RP$callback as _callback at self.callback (/Users/sobolev/Documents/github/remark-lint-is-link-active/node_modules/request/request.js:200:22) at emitOne (events.js:90:13) at Request.emit (events.js:182:7) at null._onTimeout (/Users/sobolev/Documents/github/remark-lint-is-link-active/node_modules/request/request.js:775:12) at Timer.listOnTimeout (timers.js:92:15)

I am using:

"request-promise": "^3.0.0",
"when": "^3.7.7"

I guess it also has to do something with: https://gist.github.com/benjamingr/0237932cee84712951a2