What is wrong with promise resolving?

Question

Any ideas? Why does node say 'filename is undefined'? Thanks. Contract, policy ans invoice functions resolve with no data, just resolve().

var dc = function(data) {

return new Promise(function(resolve, reject) {

    var filename = 'Test';

    var contract = function() { ... }

    var policy = function() { ... }

    var invoice = function() { ... }

    contract().then(invoice().then(policy().then(function() {
        console.log(filename); // Test
        resolve(filename); // UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): ReferenceError: filename is not defined
    })))
})

}

Each of there functions return a promise. I need to resolve the common promise after all internal promises are resolved. — Seva, Dec 22 '16 at 13:32
Did [my answer](https://stackoverflow.com/questions/41284107/what-is-wrong-with-promise-resolving/41284674#41284674) below answer your question? If so then you may consider [accepting it](http://meta.stackexchange.com/questions/5234/how-does-accepting-an-answer-work/5235#5235) so that other people who search for this topic could see that it has an accepted answer. — rsp, Dec 27 '16 at 18:31

score 2 · Accepted Answer · edited May 23 '17 at 12:34

First of all, you cannot write:

contract().then(invoice() ... )

(that would work if the invoice() function returned another function to act as a then handler)

You have to write:

contract().then(function (value) { invoice() ... })

Or:

contract().then(value => invoice() ... )

Or maybe this if one function should handle the result of other function:

contract().then(invoice).then(policy).then(function (result) { ... });

What you have to pass as an argument to then is a function, not a result of calling a function (which is probably a promise in your example).

I don't know if that's the only problem with your approach but it is certainly one of the problems. Of course it may work but probably not how you expect.

2017 Update

If you use ES2017 async/await that's available in Node since v7.0 then instead of:

contract().then(invoice).then(policy).then((result) => { ... });

you can use:

let a = await contract();
let b = await invoice(a);
let c = await policy(b);
// here your `result` is in `c`

or even this:

let result = await policy(await invoice(await contract()));

Note that you can only use it in functions declared with the async keyword. This works on Node since version 7. For older versions of Node you can use a similar thing with a slightly different syntax using generator-based coroutines, or you can use Babel to transpile your code if that's what you prefer of if that what you already do.

This is quite a new feature but there are a lot of questions on Stack Overflow about it. See:

jmilloy · Answer 2 · 2016-12-22T15:52:23.193

It looks like you don't care about the order, in which case you could use Promise.all. Does this work for you? It will resolve once all of the promises have been resolved, or it will reject as soon as any one of them rejects.

function contract(data) { return new Promise(...) }
function policy(data) { return new Promise(...) }
function invoice(data) { return new Promise(...) }

function dc(data) {
  var filename = 'Test';
  return new Promise(function(resolve, reject) {
    Promise.all([contract(data), policy(data), invoice(data)]).then(
      function (values) {
        console.log(filename);
        resolve(filename)
      },
      function (err) {
        reject(err);
      }
    );
  });
}

If you care about the order, then you do have to chain them, like you've tried to do. You're code is passing promises as an argument to then. You need to pass functions.

function contract(data) { return new Promise(...) }
function policy(data) { return new Promise(...) }
function invoice(data) { return new Promise(...) }

function dc(data) {
  var filename = 'Test';
  return new Promise(function(resolve, reject) {
    contract(data).then(
      function (contract_res) {
        policy(data).then(
          function (policy_res) {
            invoice(data).then(
              function (invoice_res) {
                console.log(filename);
                resolve(filename);
              },
              function (err) { reject(err); } // invoice promise rejected
            );
          },
          function (err) { reject(err); } // policy promise rejected
        );
      },
      function (err) { reject(err); } // contract policy rejected
    );
  });
}

You may be able to simplify this with catch, but the deep nesting is a pain. Take a look at this post about flattening Promise chains.

What is wrong with promise resolving?

2 Answers2

2017 Update

Linked