moos / wordpos

Part-of-speech utilities for node.js based on the WordNet database.
478 stars 41 forks source link

Mixing promises and callback code in wordpos #16

Closed bensalilijames closed 7 years ago

bensalilijames commented 7 years ago

Firstly, thanks for the great library. The ~5x speed improvement of v1 is really helping us out a lot!

I wondered if I could get your advice on using straightforward callbacks in wordpos. I would like to just be able to do something like this:

const myFunction = (callback) => {
  wordpos.lookup(..., (results) => {
    callback(results);
  });
};

But sometimes my callback will throw an error, causing a Promise within wordpos to be rejected, and my callback is called again from wordpos, throwing another error, and then finally node hangs with a UnhandledPromiseRejectionWarning, which breaks our test suite as it was expecting a normal throw, but got nothing.

I would like the throw somewhere down the callback chain to bubble up and throw at the top-level. Is this just not possible with this library, given the use of Promises in it? I won't be able to convert all our code to use Promises as it is a monumental effort.

At the moment, I'm getting around this by doing something like:

let lookupDone = false;
let lookupResults = [];

wordpos.lookup(word).then((results) => {
  lookupDone = true;
  lookupResults = results;
});

const wait = function wait() {
  if (!lookupDone) {
    setTimeout(wait, 10);
  } else {
    // do stuff with results
  }
}
wait();

But obviously this is highly not ideal. I may simply be not understanding Promises right. Thanks in advance!

bensalilijames commented 7 years ago

Success! I simply added:

process.on('unhandledRejection', (err) => {
  throw err;
});

And node throws at the top-level like I was expecting. Sorry for the (kind of) spam!

moos commented 7 years ago

Glad you found a solution (issues that solve themselves are my favorite! :)

Your callback should never be called after an exception. But it is possible for reject being called after an exception in your callback. From the readme:

Also, beware that exceptions in the callback will result in the Promise being rejected and caught by catch(), if provided.

You can .catch() that exception so it doesn't bubble. Cheers.