Is it a bad practice to use await inside a while loop

Question

Is it a bad practice to use a await inside a while loop?

I have the following code:

// this is inside a async function
try {
  let res = await api.resource.create(...args) // create resource
  do {
    let res = await api.resource.show(res.body.id)
    if (res.body.status === 'COMPLETED')
      return res.body
  } while(1)
} catch (err) {
  errorHandler(err)
}

I have a couple of questions here:

Is it ok to use two res variables?
Am I going to use performance because I'm using a await inside a while loop?
Is there a better solutin?

Thank you in advance.

Key question to answer: why are you using a while loop here? — Adam Jenkins, May 09 '18 at 11:58
Hmm kind off.. This basically renders the benefit of using a async method completely useless, since you could just synchronously do this. Also what is the purpose of having a variable `res` in the try block and in the while loop. — Bellian, May 09 '18 at 12:19
For 1, it makes your code less readable. For 2, compared to what alternative? For 3, depends on 2, but you might want to add a timeout to not overkill your server (if `api.resource.show()` indeed includes a call to a server). Otherwise, in a general sense, await in while loop is just fine. — Kaiido, May 09 '18 at 12:26
@Kaiido In fact the timeout is not needed IF `api.resource.show()` includes some sort of async call, since the await will stop execution until the returned Promise resolves ;) — Bellian, May 09 '18 at 12:30
@Bellian if `api.resource.show()` consists only of `fetch(server.url)` and the response is just "in progress" you will stress your server out by making thousands requests per minute. Add a bunch of users doing it simultaneously and you've just DDoS your own server with your own bad js. Also, the main goal of syntactic sugar `async/await` is actually to make loops easier to manage, since even with Promises, that was an hell to manage. Probably since OP is awaiting for `show()`, it is an asynchronous call, which makes the use of async/await very on purpose,contrarily to what your comment states — Kaiido, May 09 '18 at 12:34
Just saying, `let res = …(res.body.id)` in a block scope will always throw a `ReferenceError` because of the temporal dead zone. Drop the `let`. — Bergi, May 09 '18 at 12:39

score 5 · Accepted Answer · answered May 09 '18 at 12:43

Is it ok to use two res variables?

No. The way you are using the inner one, the access in the arguments is always in the temporal dead zone and you will always get an exception.

Am I going to use performance because I'm using a await inside a while loop?

There's nothing wrong with using await in loops, as long as you expect them to run sequentially and not all iterations concurrently.

Is there a better solution?

try {
  let res = await api.resource.create(...args) // create resource
  do {
    res = await api.resource.show(res.body.id)
  } while (res.body.status !== 'COMPLETED')
  return res.body
} catch (err) {
  errorHandler(err)
}

Notice also that if errorHandler comes from a callback parameter, you should drop the entire try/catch and just use .catch(errorHandler) on the promise returned by the call.

Also, while I don't know what api.resource.show does, it looks like you are polling for a result. It would be better if the method would just return a promise that fulfills at the right time. If that is not possible and you need to poll, I would recommend at least some delay between the calls.

Hi @merge, your aswer really cleared my mind. What do you recommend to do to delay the calls? — FXux, May 10 '18 at 00:50
Just `await delay(500);` in the loop, with a `delay` function that wraps `setTimeout` in a promise. — Bergi, May 10 '18 at 10:06

Is it a bad practice to use await inside a while loop

1 Answers1