0

I'm doing a admin script in my node backend includes mongo queries/updates and api calls. Im using async and await but it does not work as I want.

(async () => {
// connect to mongo (works ok)

 const products = await getProducts(); // Here I have my 10 elements

 await Promise.all(products.map(async (prod) => {
    response = await getProductInfo(prod.id);
  })); // here I call to getProductInfo ten times.

})()
  .catch(err => console.error(err));

const getProductInfo = async(idProduct) => {

  const response = await rq(optionsProductInfo); //request to API

  await Product.updateOne({ sku: response.sku }, { $set: {
    priority: response.priority,
    price: response.price,
  } });  // update product info
};

The problem is that the update in the database is not execute 10 times. Sometimes executed 2, 3, 4 times but never execute for the 10 elements.

Gere
  • 2,114
  • 24
  • 24
  • Don't promiscuously mix syntaxes. Use try/catch/async/await **or** .then.catch. I can't even read this. One error I see immediately is that you aren't returning anything from your callback to map, although it looks like you're not doing anything with it? What is all of this? – Jared Smith Nov 21 '18 at 20:20
  • Try to change the line where you assign response, as basically it will reassign every time new value. Or maybe you should assign response with result returned from `Promise.all`. – Oleksandr Fedotov Nov 21 '18 at 20:22
  • I got it. That's is not my complete/real code, I "simplify it" for this question – Gere Nov 21 '18 at 20:22
  • Don't think in terms of "nesting". There is no "waiting for the return" so there technically isn't any nesting happening. When working with async/await think in terms of delegation. Is it okay to delegate tasks as part of a task that is itself delegated? Of course. Good task management means only doing what you need to do and delegate everything else. – Mike 'Pomax' Kamermans Nov 21 '18 at 20:25
  • I think I don't need a response. I need that the getProductInfo() method will be execute for the all products listed. – Gere Nov 21 '18 at 20:28
  • 1
    If you don't need the response, then why `await` anything? Why even build a promise? Just run `products.forEach(p => getProductInfo(p.id))` in your main code? – Mike 'Pomax' Kamermans Nov 21 '18 at 20:36
  • 1
    @Gere Please show your actual code, or a minimal example that still exhibits the unexpected behaviour. – Bergi Nov 21 '18 at 21:10
  • @Mike Never use `forEach` :-) In this case, calling `getProductInfo` without waiting for anything will potentially cause cause unhandled rejections. Make sure to [do something about that](https://stackoverflow.com/a/32385430/1048572) – Bergi Nov 21 '18 at 21:11
  • That's a bizarre thing to say, @Bergi =) – Mike 'Pomax' Kamermans Nov 22 '18 at 00:31

1 Answers1

0

Just make sure you're awaiting the right thing, and that any function that has an await is itself async, and you should be just fine. The following code for example works just fine.

const products = [1,2,3,4,5,6];

async function updateRecord(data) {
  return new Promise((resolve, reject) => {
    // using timeout for demonstration purposes
    setTimeout(async() => {
      console.log("pretending I updated", data.id);
      resolve();
    }, 1000 * Math.random());
  });
};

async function getProductInfo(id) {
  return new Promise((resolve, reject) => {
    // again using timeout for demonstration purposes
    setTimeout(async() => {
      console.log("fake-retrieved product info for", id);
      resolve(await updateRecord({ id }));
    }, 1000 * Math.random());
  });
};

Promise.all(products.map(pid => getProductInfo(pid)))
       .then(() => console.log("we're done."));
       .catch(e => console.error("what?", e));
Mike 'Pomax' Kamermans
  • 49,297
  • 16
  • 112
  • 153
  • Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! Even (or especially) if `getProductInfo` is just an example mock. And don't use `async` functions as callbacks to e.g. `setTimeout`. – Bergi Nov 21 '18 at 21:13
  • It's not, I'm very intentionally not doing any then/catching inside these demonstration promises that are very mcuh placeholders for database API calls, and the same goes for setTimeout: there is nothing wrong with marking the function as async which in this case is strictly necessary for that `await` to even work. I'm all for es-today evangelism, but code that's used as stand-in for real APIs is just code that's used as stand-in for real APIs. – Mike 'Pomax' Kamermans Nov 22 '18 at 00:34
  • But you don't even need to `await` anything in those callbacks! The first one is entirely synchronous, and the second one should simply `resolve` the promise with the other promise (which properly forwards rejections). – Bergi Nov 22 '18 at 08:59
  • But database API calls are not synchronous, and that's what we're simulating with the timeouts: highly artificial asynchronous flow. Feel free to write your own answer with what you feel is better code, of course. More than happy to see your take. – Mike 'Pomax' Kamermans Nov 22 '18 at 16:39
  • `setTimeout` is not synchronous, but the callback that you pass to it is. No need to make that `async`. – Bergi Nov 22 '18 at 20:05