Node JS Promise then block executed before promise resolution

Question

Simple code path, that is better explained with bullet points

1. API call
2. Call to DB
3. Result is a list of objects
4. Inside the then block make a DB call for each object to hydrate children
5. Inside another then block res.send(hydrated object)

Problem

Step 5 happens before step 4 completes (Code below)

//api endpoint
router.post('/get-data', getObjects);

export const getObjects: express.RequestHandler = (req, res) => {
    queryContainer(containerId, querySpec)
    .then((result) => {
        return getChildren(result, req.body.criteria);
    })
    .then((result) => {
        res.send(result);
    });
}

export async function queryContainer(containerId, querySpec) {
    const { result: results } = await client.database(databaseId).container(containerId).items.query(querySpec, {enableCrossPartitionQuery: true}).toArray()
    .catch( (error) => {
        console.log("Error! ", error);
    });
    return results;
}

function getChildren(result: any, criteria: any) {
    if(criteria) {
        result.children = [];
        var actions = result
                        .map(result => result.element)
                        .map(dbCallToGetChildren);
        var results = Promise.all(actions);
        results.then(children => {
            result.children.push(...children)
            return result;
        });
    } 

    return result;
}

export const dbCallToGetChildren = (async function (username) {
    const querySpec = {
        query: "SELECT * FROM root r WHERE r.userName=@userName",
        parameters: [
            {name: "@userName", value: username}
        ]
    };
    queryContainer(containerId, querySpec)
    .then((results) => { 
        return results;
    })
    .catch((error) => {
        console.log("Error " + error);
        return Promise.resolve;
    });
});

Answer 1

Step 5 happens before step 4 (getChildren function) completes because getChildren isn't returning a Promise. Changing it to the following might fix the problem:

function getChildren(result: any, criteria: any) {
  return new Promise(resolve => {
    if(criteria) {
        result.children = [];
        var actions = result
                        .map(result => result.element)
                        .map(dbCallToGetChildren);
        var results = Promise.all(actions);
        results.then(children => {
            result.children.push(...children)
            resolve(result);
        });
    } else {
      resolve(result);
    }
  });
}

Inside results.then(children => { ... } there is now resolve(result); to ensure the return getChildren(result, req.body.criteria); statement within the queryContainer call doesn't complete until the Promise resolves.

Answer 2

In order for step 4 to fully complete before step 5 is executed, we need to promisify every code-path that goes through getChildren.

That means we should change this:

function getChildren(result: any, criteria: any) {
    if(criteria) {
        result.children = [];
        var actions = result
                        .map(result => result.element)
                        .map(dbCallToGetChildren);
        var results = Promise.all(actions);
        results.then(children => {
            result.children.push(...children)
            return result;
        });
    } 
    return result;
}

Into the following (pay attention to the code comments):

function getChildren(result: any, criteria: any) {
    if(criteria) {
        result.children = [];
        var actions = result
                        .map(result => result.element)
                        .map(dbCallToGetChildren);
        var results = Promise.all(actions);
        return results.then(children => { // returns a promise 
            result.children.push(...children)
            return result;
        });
    } 
    return Promise.resolve(result); // returns a promise
}

It's important to return otherwise the code in this line is being executed in async manner (which doesn't guarantee that step 4 will complete before step 5 begins).

Answer 3

I have a few comments about your code:

1. you are doing mutation which is a bad practice, getChildren function accepts result type any and then it makes some changes to it like

result.children = []

2. Avoid using any and try to define a type

3. in your code because you are doing mutation so you do not even need to return back the result because the original object is already changed but as I mentioned earlier you should avoid mutation.

4. The main problem that step 5 executes before step 4 is that getChildren won't return back promise, I made some changes in your code to accommodate promise.

      function getChildren(result: any, criteria: any): Promise<any> {
      return new Promise((resolve, reject) => {
        if (criteria) {
          result.children = []
          const actions = result
            .map(item => item.element)
            .map(dbCallToGetChildren)
          Promise.all(actions).then(children => { // returns a promise
            result.children.push(...children)
            return resolve("successfully is processed!")
          })
        }
        reject("invalid criteria!")
      })
    }