Reputation: 482
I'm struggling a bit with JS promises.
I am using a library to pull data from Spotify that returns promises. In my main function I can use an await to build an object from the response data and push it to an array (called nodes):
var nodes = [];
main();
async function main() {
var id = '0gusqTJKxtU1UTmNRMHZcv';
var artist = await getArtistFromSpotify(id).then(data => buildArtistObject(data));
nodes.push(artist);
When I debug here then all is good, nodes has my object.
However, when I introduce a 2nd await underneath to make another call:
nodes.forEach((node, i) => {
if (node.done == false) {
console.log(node.toString());
var related_artists = await getRelatedArtists(node.spotify_id);
I get the following error: SyntaxError: await is only valid in async function
I thought the first await statement would be resolved and the execution would continue until the next..?
Any help would be greatly appreciated.
EDIT The other functions, if that helps, are just as follows:
function getArtistFromSpotify(id) {
let response = spotify
.request('https://api.spotify.com/v1/artists/' + id).then(function (data) {
return data;
})
.catch(function (err) {
console.error('Error occurred: ' + err);
return null;
});
return response;
}
function getRelatedArtists(id) {
let response = spotify
.request('https://api.spotify.com/v1/artists/' + id + '/related-artists').then(function (data) {
return data;
})
.catch(function (err) {
console.error('Error occurred: ' + err);
return null;
});
return response;
}
function buildArtistObject(data) {
var artist = {
node_id: nodes.length,
name: null,
genres: null,
popularity: null,
spotify_id: null,
done: false
}
artist.name = data.name;
artist.genres = data.genres;
artist.popularity = data.popularity > 0 ? data.popularity : 0;
artist.spotify_id = data.id;
return artist;
}
Upvotes: 0
Views: 516
Reputation: 135197
You have some misunderstandings of how to use promises -
let response = spotify
.request(url)
.then(function(data) { return data }) // this does nothing
.catch(function (err) { // don't swallow errors
console.error('Error occurred: ' + err);
return null;
})
return response
You'll be happy there's a more concise way to write your basic functions -
const getArtist = id =>
spotify
.request('https://api.spotify.com/v1/artists/' + id)
const getRelatedArtists = id =>
spotify
.request('https://api.spotify.com/v1/artists/' + id + '/related-artists')
Now in your main
function, we can await
as many things as needed. Let's first see how we would work with a single artist ID -
async function main(artistId) {
const artistData = await getArtist(artistId)
const relatedData = await getRelatedArtists(artistId)
return buildArtist(artistData, relatedData)
}
If you have many artist IDs -
async function main(artistIds) {
const result = []
for (const id of artistIds) {
const artistData = await getArtist(artistId)
const relatedData = await getRelatedArtists(artistId)
result.push(buildArtist(artistData, relatedData))
}
return result
}
Either way, the caller can handle errors as
main([693, 2525, 4598])
.then(console.log) // display result
.catch(console.error) // handle errors
Which is the same as -
main([693, 2525, 4598]).then(console.log, console.error)
The pattern above is typical but sub-optimal as the caller has to wait for all data to fetch before the complete result is returned. Perhaps you would like to display the information, one-by-one as they are fetched. This is possible with async generators -
async function* buildArtists(artistIds) {
for (const id of artistIds) {
const artistData = await getArtist(artistId)
const relatedData = await getRelatedArtists(artistId)
yield buildArtist(artistData, relatedData) // <- yield
}
}
async function main(artistIds) {
for await (const a of buildArtists(artistIds)) // <- for await
displayArtist(a)
}
main([693, 2525, 4598]).catch(console.error)
Upvotes: 1
Reputation: 56865
The code below has multiple problems.
var nodes = [];
main();
async function main() {
var id = '0gusqTJKxtU1UTmNRMHZcv';
var artist = await getArtistFromSpotify(id).then(data => buildArtistObject(data));
nodes.push(artist);
// ...
First of all, main
mutates global scope nodes
. Not only is this an antipattern even in synchronous code (functions should not rely on, or modify, global variable names; use parameters and return values instead), in asynchronous code, nodes
will never be available for use anywhere but within main
. See How do I return the response from an asynchronous call?.
Secondly, try to avoid combining then
and await
. It's confusing.
It's also a little odd that an array of nodes
is used, yet only one artist
is pushed onto it...
As for this code:
nodes.forEach((node, i) => {
if (node.done == false) {
console.log(node.toString());
var related_artists = await getRelatedArtists(node.spotify_id);
// ...
The error is self-explanatory. You must add async
to the enclosing function if you want it to be asynchronous: nodes.forEach(async (node, i) => { // ...
. But that spawns a new promise chain per node, meaning future code that's dependent on the result won't be able to await all of the promises in the loop resolving. See Using async/await with a forEach loop. The likely solution is for..of
or Promise.all
.
While I'm not 100% sure what your final goal is, this is the general pattern I'd use:
async function main() {
const id = '0gusqTJKxtU1UTmNRMHZcv';
const data = await getArtistFromSpotify(id);
const artist = await buildArtistObject(data);
const nodes = [artist]; // odd but I assume you have more artists somewhere...
for (const node of nodes) {
if (!node.done) {
const relatedArtists = await getRelatedArtists(node.spotify_id);
}
}
/* or run all promises in parallel:
const allRelatedArtists = await Promise.all(
nodes.filter(e => !e.done).map(e => getRelatedArtists(e.spotify_id))
);
*/
// ...
}
main();
Since your code isn't runnable and some of the intent is unclear from the context, you'll likely need to adapt this a bit, so consider it pseudocode.
Upvotes: 1