How to properly chain Promises with nesting

Question

My node project currently contains a sideway christmas tree of nested callbacks in order to fetch data and process them in the right order. Now I'm trying refactor that using Promises, but I'm unsure how to do it properly.

Let's say I'm fetching a list of offices, then for each office all their employees and then each employees' salary. In the end all entities (offices, employees and salaries) should be linked together and stored in a database.

Some pseudo-code illustrating my current code (error handling omitted):

fetch(officesEndpoint, function (data, response) {
    parse(data, function (err, offices) {
        offices.forEach(function (office) {
            save(office);
            fetch(employeesEndPoint, function (data, response) {
                parse(data, function (err, employees) {
                    // link each employee to office
                    save(office);
                    save(employee);
                    employees.forEach(function () {
                        fetch(salaryEndpoint, function (data, response) {
                            parse(data, function (err, salaries) {
                                // link salary to employee
                                save(employee);
                            });
                        });
                    });
                });
            });
        });
    });
});

I tried solving this with promises, but I have a couple of problems:

• kind of verbose?
• each office needs to be linked to their respective employees, but in the saveEmployees function I only have access to the employees, not the office from further up in the chain:

---

var restClient = require('node-rest-client');
var client = new restClient.Client();
var xml2js = require('xml2js');

// some imaginary endpoints
var officesEndpoint = 'http://api/offices';
var employeesEndpoint = 'http://api/offices/employees';
var salaryEndpoint = 'http://api/employees/:id/salary';


function fetch (url) {
    return new Promise(function (resolve, reject) {
        client.get(url, function (data, response) {
            if (response.statusCode !== 200) {
                reject(statusCode);
            }
            resolve(data);
        });
    });
}

function parse (data) {
    return new Promise(function (resolve, reject) {
        xml2js.parseString(data, function (err, result) {
            if (err) {
                reject(err);
            }
            resolve(result);
        });
    });
}

function saveOffices (offices) {
    var saveOffice = function (office) {
        return new Promise(function (resolve, reject) {
            setTimeout(function () {  // simulating async save()
                console.log('saved office in mongodb');
                resolve(office);
            }, 500);
        })
    }
    return Promise.all(offices.map(saveOffice));
}

function saveEmployees (employees) {
    var saveEmployee = function (employee) {
        return new Promise(function (resolve, reject) {
            setTimeout(function () { // simulating async save()
                console.log('saved employee in mongodb');
                resolve(office);
            }, 500);
        })
    }
    return Promise.all(offices.map(saveEmployee));
}

fetch(officesEndpoint)
.then(parse)
.then(saveOffices)
.then(function (savedOffices) {
    console.log('all offices saved!', savedOffices);
    return savedOffices;
})
.then(function (savedOffices) {
    fetch(employeesEndPoint)
    .then(parse)
    .then(saveEmployees)
    .then(function (savedEmployees) {
        // repeat the chain for fetching salaries?
    })
})
.catch(function (error) {
    console.log('something went wrong:', error);
});

Answer 1

You don't necesseraly have to nest, this would work too:

fetch(officesEndpoint)
  .then(parse)
  .then(saveOffices)
  .then(function(savedOffices) {
    console.log('all offices saved!', savedOffices);
    return savedOffices;
  })
  .then(function(savedOffices) {
    // return a promise
    return fetch(employeesEndPoint); // the returned promise can be more complex, like a Promise.all of fetchEmployeesOfThisOffice(officeId)
  })
  // so you can chain at this level
  .then(parse)
  .then(saveEmployees)
  .then(function(savedEmployees) {
    return fetch(salariesEndPoint);
  })
  .catch(function(error) {
    console.log('something went wrong:', error);
  });

Answer 2

It is good approach to change this

        if (response.statusCode !== 200) {
            reject(statusCode);
        }
        resolve(data);

to this

        if (response.statusCode !== 200) {
            return reject(statusCode);
        }
        resolve(data);

In your example, the result will be same, but if you are making more things (like doing something in database) the unexpected result may occure, because without return the whole method will be executed.

This example

var prom = new Promise((resolve,reject) => {
    reject(new Error('error'));
    console.log('What? It did not end');
    resolve('Ok, promise will not be called twice');
});

prom.then(val => {
    console.log(val);
}).catch(err => {
    console.log(err.message);
});

is having this output

What? It did not end
error

To the question - if you need access to more than one returned value (i.e. offices AND employies), you have basically two options :

• Nested promises - this is not generally bad, if it "makes sense". Altought promises are great to avoid huge callback nesting, it is ok to nest the promises, if the logic needs it.

• Having "global" variables - you can define variable in the scope of the promise itself and save results to it, therefore the promises are using these variables as "global" (in their scope).

Answer 3

Your promisified functions fetch, parse, saveOffices and saveEmployees are fine. With those, you can refactor your current code to use promises, chain instead of nest where applicable, and leave out a bunch of error handling boilerplate:

fetch(officesEndpoint)
.then(parse)
.then(function(offices) {
    return Promise.all(offices.map(function(office) {
        return save(office)
        .then(function(){ return fetch(employeesEndPoint); })
        .then(parse)
        .then(function(employees) {
            // link each employee to office
            // throw in a Promise.all([save(office), save(employee)]) if needed here
            return Promise.all(employees.map(function(employee) {
                return fetch(salaryEndpoint)
                .then(parse)
                .then(function(salaries) {
                    return Promise.all(salaries.map(function(salary) {
                        // link salary to employee
                        return save(employee);
                    }));
                });
            }));
        });
    }));
});

In the innermost loop callback, you've got all of office, employee and salary available to interlink them to your liking. You cannot really avoid this kind of nesting.

You'll get back a promise for a huge array of arrays of arrays of save results, or for any error in the whole process.