Does .then(Promise.mapSeries(...)) behave differently than .then(function() { return Promise.mapSeries(...); })?

Question

I initially assumed that passing a bare Promise.mapSeries(...) call as an argument to .then() would be the same as wrapping it in a function, like .then(function() { return Promise.mapSeries(...); }). Having written out this question, I'm no longer entirely sure why it works at all.

In the simplified code below, I open a couple of databases asynchronously (openDBAsync()) and then read a file containing a JS object. I use _.map() to iterate over all of the key/value pairs in the object and asynchronously update their values in a database, while keeping track of which ones meet certain criteria (whether the value is odd, in this toy example). Promise.all() waits for all of the async database calls to settle, and then Promise.mapSeries() is used to process each of the subset of keys, which makes another async database call for each one. Finally, I close all the databases.

function processData(path)
{
    var oddKeys = [];

    return openDBAsync()
        .then(function() { return readFileAsync(path); })
        .then(function(dataObject) {
            return Promise.all(_.map(dataObject, function(value, key) {
                if (value % 2) {
                    oddKeys.push(key);
                }

                return updateDBAsync(key, ++value);
            }))
            .then(Promise.mapSeries(
                oddKeys,
                function(key) {
                    return updateOddDBAsync(key);
                }
            ))
        })
        .then(closeDBAsync);
}

The problem is that the database throws errors complaining that I'm trying to update the database after it's been closed. That means that some of the promises generated in the .mapSeries() call are being called after the final closeDBAsync(). I expected all of them to settle before the final .then() call.

If I wrap the call to Promise.mapSeries() in a function:

            .then(function() {
                return Promise.mapSeries(
                    oddKeys,
                    function(key) {
                        return updateOddDBAsync(key);
                    }
                );
            })

Then I don't get any errors. It also works if I put a .delay(2000) before the close database call, which indicates that Promise.mapSeries() isn't settling all of the promises before it finishes.

This seems like either a bug in Bluebird, or, much more likely, I'm not understanding something fundamental about how Promise.mapSeries() works. Any guidance would be much appreciated.

Answer 1

much more likely, I'm not understanding something fundamental about how Promise.mapSeries() works

Nope, this seems to be more a misunderstanding about how .then(…) works.

The then method of promises does always take a callback function (if you're passing anything else [but null], Bluebird should spit out a warning!). By calling .then(Promise.mapSeries(…)), you were passing a promise instead, which is simply ignored. Being ignored, it also is not awaited by anything, which leads to that error of the database being closed too early.

But the direct call to Promise.mapSeries() doesn't get applied to the array immediately. If it was, the array would be empty and this code wouldn't work at all.

Yes it does. Your array is filled from by the _.map callback, which is executed synchronously, before then and mapSeries are invoked.

So the solution is indeed to wrap the call in a function expression, which will only be executed when the Promise.all(…) fulfills, and whose result will then not be ignored but rather awaited. There might be more, different solutions, depending on what degree of parallel execution you want to allow.

Btw, given you are doing a database transaction, your current code is quite fragile. Look into the promise disposer pattern.