ArrayKnight ArrayKnight - 5 months ago 11
Node.js Question

Async/Await not waiting

I'm running into an issue which I don't fully understand. I feel like there are likely concepts which I haven't grasped, code that could be optimized, and possibly a bug thrown in for good measure.

To greatly simplify the overall flow:


  1. A request is made to an external API

  2. The returned JSON object is parsed and scanned for link references

  3. If any link references are found, additional requests are made to populate/replace link references with real JSON data

  4. Once all link references have been replaced, the original request is returned and used to build content



Here, is the original request (#1):

await Store.get(Constants.Contentful.ENTRY, Contentful[page.file])


Store.get is represented by:

async get(type, id) {
return await this._get(type, id);
}


Which calls:

_get(type, id) {
return new Promise(async (resolve, reject) => {
var data = _json[id] = _json[id] || await this._api(type, id);

console.log(data)

if(isAsset(data)) {
resolve(data);
} else if(isEntry(data)) {
await this._scan(data);

resolve(data);
} else {
const error = 'Response is not entry/asset.';

console.log(error);

reject(error);
}
});
}


The API call is:

_api(type, id) {
return new Promise((resolve, reject) => {
Request('http://cdn.contentful.com/spaces/' + Constants.Contentful.SPACE + '/' + (!type || type === Constants.Contentful.ENTRY ? 'entries' : 'assets') + '/' + id + '?access_token=' + Constants.Contentful.PRODUCTION_TOKEN, (error, response, data) => {
if(error) {
console.log(error);

reject(error);
} else {
data = JSON.parse(data);

if(data.sys.type === Constants.Contentful.ERROR) {
console.log(data);

reject(data);
} else {
resolve(data);
}
}
});
});
}


When an entry is returned, it is scanned:

_scan(data) {
return new Promise((resolve, reject) => {
if(data && data.fields) {
const keys = Object.keys(data.fields);

keys.forEach(async (key, i) => {
var val = data.fields[key];

if(isLink(val)) {
var child = await this._get(val.sys.linkType.toUpperCase(), val.sys.id);

this._inject(data.fields, key, undefined, child);
} else if(isLinkArray(val)) {
var children = await* val.map(async (link) => await this._get(link.sys.linkType.toUpperCase(), link.sys.id));

children.forEach((child, index) => {
this._inject(data.fields, key, index, child);
});
} else {
await new Promise((resolve) => setTimeout(resolve, 0));
}

if(i === keys.length - 1) {
resolve();
}
});
} else {
const error = 'Required data is unavailable.';

console.log(error);

reject(error);
}
});
}


If link references are found, additional requests are made and then the resulting JSON is injected into the original JSON in place of the reference:

_inject(fields, key, index, data) {
if(isNaN(index)) {
fields[key] = data;
} else {
fields[key][index] = data;
}
}


Notice, I'm using
async
,
await
, and
Promise
's I believe in their intended manor. What ends up happening: The calls for referenced data (gets resulting of _scan) end up occurring after the original request is returned. This ends up providing incomplete data to the content template.

Additional information concerning my build setup:


  • npm@2.14.2

  • node@4.0.0

  • webpack@1.12.2

  • babel@5.8.34

  • babel-loader@5.4.0


Answer

I believe the issue is in your forEach call in _scan. For reference, see this passage in Taming the asynchronous beast with ES7:

However, if you try to use an async function, then you will get a more subtle bug:

let docs = [{}, {}, {}];

// WARNING: this won't work
docs.forEach(async function (doc, i) {
  await db.post(doc);
  console.log(i);
});
console.log('main loop done');

This will compile, but the problem is that this will print out:

main loop done
0
1
2

What's happening is that the main function is exiting early, because the await is actually in the sub-function. Furthermore, this will execute each promise concurrently, which is not what we intended.

The lesson is: be careful when you have any function inside your async function. The await will only pause its parent function, so check that it's doing what you actually think it's doing.

So each iteration of the forEach call is running concurrently; they're not executing one at a time. As soon as the one that matches the criteria i === keys.length - 1 finishes, the promise is resolved and _scan returns, even though other async functions called via forEach are still executing.

You would need to either change the forEach to a map to return an array of promises, which you can then await* from _scan (if you want to execute them all concurrently and then call something when they're all done), or execute them one-at-a-time if you want them to execute in sequence.


As a side note, if I'm reading them right, some of your async functions can be simplified a bit; remember that, while awaiting an async function call returns a value, simply calling it returns another promise, and returning a value from an async function is the same as returning a promise that resolves to that value in a non-async function. So, for example, _get can be:

async _get(type, id) {
  var data = _json[id] = _json[id] || await this._api(type, id);

  console.log(data)

  if (isAsset(data)) {
    return data;
  } else if (isEntry(data)) {
    await this._scan(data);
    return data;
  } else {
    const error = 'Response is not entry/asset.';
    console.log(error);
    throw error;
  }
}

Similarly, _scan could be (assuming you want the forEach bodies to execute concurrently):

async _scan(data) {
  if (data && data.fields) {
    const keys = Object.keys(data.fields);

    const promises = keys.map(async (key, i) => {
      var val = data.fields[key];

      if (isLink(val)) {
        var child = await this._get(val.sys.linkType.toUpperCase(), val.sys.id);

        this._inject(data.fields, key, undefined, child);
      } else if (isLinkArray(val)) {
        var children = await* val.map(async (link) => await this._get(link.sys.linkType.toUpperCase(), link.sys.id));

        children.forEach((child, index) => {
          this._inject(data.fields, key, index, child);
        });
      } else {
        await new Promise((resolve) => setTimeout(resolve, 0));
      }
    });

    await* promises;
  } else {
    const error = 'Required data is unavailable.';
    console.log(error);
    throw error;
  }
}
Comments