Timeout for an async loop: if loop do not finishes before timeout, it will break anyway.

Hi Team,

I propose a new syntax : await[timeoutInMs] to control timeout of a promise.
I have summarized this whole thread here in the proposal.

Original work thread:


Currently we can loop through an async source like this :

let someHeavyData = getBigDataOnRam();

for await (let t of asyncGenratorOrStream){
// collect this async data and do something 
}

//once loop finishes/break do something

In such a case we are at mercy of asyncGenratorOrStream that it will send Data on time, but what if it never send any data or take unexpectedly too long amount of time on some values !?! All our resources in RAM are burden on the system.

We should have a control to break it , if the loop does not finishes within a given time.

for await[3000] (let t of asyncGenratorOrStream){
/// collect this async data and do something 
}
//code wll continue from here anyway.

Such a code will break at most by 3000ms irrespective even if async source is still delivering values. If loop finishes on time (before 3000ms) well and good continue as normally it would, but if it goes beyond 3000 ms break the loop and continue from then on like a normal loop break.


Context : Why it is required or its application ?

Async loops are left exploitable, the control is in the hands of such an asyncGenratorOrStream, when they want the async loop to end!

I have been working on a consensus algorithm.
Where each participant node needs to send their response before a timeout for a transaction. If all the response is not received before timeout, I should collect whatver response I have got by then and break out of loop.

I get this reponse via websocket channel and its then transformed to a stream (by the framework I use) and looped to make/collect a consensus.
There is no easy way to control this, as participants can always delay their answer and let my RAM be loaded for no reason!

Proposal:
A syntactic sugar like await[timeInMs] would solve many such problems, with great ease.

I think this await[timeInMs] can be extends to even promise (which is currently done with help of Promise.race).

await[timeInMs] gives a very clear idea (just by reading code), if a particular line be bottle neck of a code flow, and gives a much better control on async code.

The best way would be that such an await would throw an error (when tmeout occurs), which can be later caught to know if the code was transfered to next line by normal flow or timeout problem happened.

try{
  for await[3000](let t of asyncGeneratorOrStream){
    // blah blah....
  }
}catch(e){
//by e we can know if async loop had timed out.
}

Or for a promise:

let someExpectedValue;
try{
 someExpectedValue = await[3000] getFromPromise();
}catch(e){
  //by e we can know if awaited promise had timed out.
}

It perplexes me to this day that promise cancellation was considered an optional thing we can do without and doesn't even have a proposal to this day. Imagine if we didn't have window/document.removeEventListener and/or clearTimeout.

Is there a precedent of a lnguage where you can't ever stop an async task once it has started?

Yes, there's AbortController but each and every task creator library/method has to implement support for it by hand. Aaand it's also rendered useless if that library is using any promise based API upstream which doesn't support AbortController.

(Promise.race still keeps the timed out task running in the background forever if the task so wills btw).

The Promise API is so severely limited. Check out the crazy hacks React 18 is doing because you can't synchronously tell if a promise has settled... they've resorted to using throw to keep throwing a pending promise until it's settled. Fun times ahead. :confused: I think the argument for making promises statically non-observable must've been some supposed optimizability? Well come next React release, everybody is gonna start throwing their Promises instead. That really doesn't sound optimizable.

1 Like

The ideal solution to your problem would be to use an API that supports cancel tokens, and to cancel the token after your timeout, like this:

const cancelToken = new CancelToken()
setTimeout(() => cancelToken.cancel(), 3000)
for await (const x of getStream(cancelToken)) {
  ...
}

Cancel tokens are more cumbersome to use, but they are also much more powerful, and provide a lot for flexibility for future cancelation needs. e.g. if you wanted to perform a different async task first, and have your timeout span both of these operations, you can provide the same cancel token to both functions. See this excelent article on cancel tokens for more details on cancel tokens vs timeouts.

There is in fact a proposal for cancel tokens here, but it does appear to be pretty stagnant at the moment. Hopefully, it gets some traction soon, or some other proposal comes along to replace it. In the mean time, it's not that hard to design a cancel token API by hand - you can always mimick the AbortController API - whatever cacel-token API that JavaScript eventually adopts will need to be aware of the existing AbortController API and how best to stay consistent with it.

If you're in the scenario where you can't provide a cancel token or a timeout directly to this API, then the next best thing you can do is wrap this generator in a way so that it is at least ignorable. This wouldn't really be "canceling", because the only thing you have power to do is to stop awaiting the promises and clean up your own resources if a timeout/cancelToken token goes off. You can't actually tell this API to clean up and release anything they're holding on to.

I'm ok with this being a little more tedious because it's already not the ideal solution, and something that shouldn't be directly encouraged. The ideal solution would be if this API provided a way for us to ask it to cancel and clean up. This next-best solution is just ignoring the promise after a timeout and not having the API clean up.

An example solution

// Other people's code

const wait = ms => new Promise(resolve => setTimeout(resolve, ms))

async function* bunchOfTasks() {
  for (let i = 0; i < 5; ++i) {
    yield i
    await wait(500)
  }
}

// Cancel Token

class CancelError extends Error {}

class CancelToken {
  #reject
  constructor() {
    const wait = new Promise((_, reject) => {
      this.#reject = reject
    })

    this.signal = { wait }
  }

  abort() {
    this.#reject(new CancelError('Canceled!'))
  }
}


// Your code

async function* cancelableAsyncIterator(cancelSignal, iterator) {
  while (true) {
    const { done, value } = await Promise.race([
      cancelSignal.wait,
      iterator.next(),
    ])

    if (done) break
    yield value
  }
}

async function main() {
  let reallyBigResource = null
  const cancelToken = new CancelToken()
  setTimeout(() => cancelToken.abort(), 2000)
  for await (const x of cancelableAsyncIterator(cancelToken.signal, bunchOfTasks())) {
    console.log(x)
  }
}

main()

I admit that the cancelableAsyncIterator() function defined below is overly tedious - it would be nice if it were possible to get this objective done without needing to pick apart an async iterator like this. Perhaps this could be a job for the iterator helpers proposal - what's really needed here to improve the code quality would be some sort of way to map over the promises of an async iterator, so that you can turn them into races with your cancel token. i.e. something like this:

async function main() {
  let reallyBigResource = null
  const cancelToken = new CancelToken()
  setTimeout(() => cancelToken.abort(), 2000)

  const iterator = bunchOfTasks()
    .mapRawPromises(promise => Promise.race([
      cancelSignal.wait,
      promise,
    ]))
  for await (const x of iterator) {
    console.log(x)
  }
}

main()

Or something like that. Perhaps a mapRawPromises function's use case would be too specific to the task of promise ignoring, and maybe there's a better way to device such a helper function.

Or

iterator.throw('abort')

Well yes its certainly being possible and the code you shared in a way kind of works (with one flaw).
But its way tedious as we agree. A syntatic sugar (like await[timeInMs] ) might ease this efforts , and make code much more readable and understandable. Futher more implementation wise its not that difficult (as shared in your code, a typescript like transpiler can transpile await[timeInMs] into a the code you have shared).


Flaw I was talking about: atleast one last value of iterator which is taking longest time will be evaluated (or rather waited for it to resolve) by the code. As Promise.race do not kills/cancel the other promise which is not resolved on time. It will simply return the first one which resolves on time.

So for example a call like this, would still hold up RAM:

let res = await Promise.race([timeoutDunnmyFunc,reallySlowerDBAsyncFunction]);
  • in this code reallySlowerDBAsyncFunction will be called and the program won't exit uness this is resolved, although its value will be ignored, but it will be evaluated and all RAM resource its handle will be there pretty much doing nothing, even though its value is ignored any way!
  • God forbids, but if this reallySlowerDBAsyncFunction also mutates some of our code values, than we will end up with some values modified in our code, without a proper trace who might have mutated them.

You're correct. That's why that solution simply "ignores" the promise and does not actually cancel it.

It's actually pretty important that you can't reach into a long-running async task and force-cancel it. You could end up leaving stuff in a bad state if you were to do that, because you're not giving these APIs a chance to clean up after themselves. The cancel token README actually provides a good narrative on why this sort of design wouldn't play out well, and why they're choosing to have each API implement their own, smarter canceling behavior.

Here's an example of why this would be bad. Imagine this was the implementation of this long-running database query (and assume this implemenation is out of your control):

async function doLongRunningTask() {
  const connection = await getConnectionFromPool()
  const { error, resource } = await connection.fetchResource()
  connection.release()

  if (error) throw error
  return resource
}

In this example, fetchResource() won't ever throw an error, instead, it returns any network errors. In general, this function isn't ever supposed to throw an error until the database connection is released and you hit the if (error) throw error line.

But, what happens if you force timeout this function? Right in the middle of this long-running query? You're going to have worse problems in your hand than a query taking too long, now you have connections from the connection pool that are forever reserved by a timed-out promise.

This sort of design just can't happen. We can't add a new feature that lets people reach into the internals of running code and force-stop it. Many APIs aren't designed to handle this kind of behavior.

The only way to mitigate this issue is to have the API authors explicitly provide a mechanism to allow cancelation to happen, so that the API author can ensure they properly clean everything up when cancelation does happen. These APIs need to accept a cancel token as a parameter, and appropriately clean up when you want them to cancel.

So, for your original use case, what this means is that you just need a way to close the websocket of participants who have taken too long to respond. Any good websocket library should provide such a mechanism. This means, you can indeed do a promise.race, and if the timeout happens first, you just have to clean up your other promise by closing the websocket connection. The difficulty of this task depends on the library you are using. The quality of the library could determine the difficulty of accomplishing this task. What we can't do is have errors start being thrown in the middle of node's internals, in locations where they normally wouldn't be thrown - that would be a disaster.

As a defence, I will like to place following points:

  1. Regular await (without a timeout) will be in place and will behave the way it has always. All code which has been using it the regular way will work the way they have always worked. So code where its expected that the controll be in the hands of API, to do cleaning, are free to use the existing syntax.

  2. However where the control is deliberately needed at developer end, cases where such a functionality is required, developer can deliberatley used await[3000] to cause an interrupt.
    When something is deliberate, all repercussions, are not unexpected, they are intended outcomes.

User can create deliberate worse problem, by writing a code as such to the same problem without using await[timeInMs]:

//deliberate attempt to mess up some one's api code:
let t = getConnectionFromPool;
getConnectionFromPool = ()=>setTimeout(a=>throw "error",100); return t();

async function doLongRunningTask() {
  const connection = await getConnectionFromPool()
  const { error, resource } = await connection.fetchResource()
  connection.release()

  if (error) throw error
  return resource
} 

Both will have same effect and , are done deliberately, and hence user knows what is about to come.

  1. An api that intend to do a must have clean up , would have rather written code as such.
async function doLongRunningTask() {
let connection;  
try{
 //now any where any exception occurs, or an interrupt exception is thrown, or time out error is throw in middle, all clean up will still take place.
  }catch(e){
     if(connection) connection.release();
  }
} 

They have written the code as discussed in previous example (point 2), as may be thats what they wanted, as thats what the code does which they have written! (As it allows people to mess it up anyway even if await[timeOutInMs] is not there in place).

A simple wrap up of try catch block would solve the problems.

try{
 // do something which interrupt intentionaly: await[3000] or the override connection methods
await[2500] someProcessWhichCanTakeSomeTime();
}catch(e){
//do clean up resources left due to expected/deliberate developer introduced interruption.
}

The throwing up errors and catching up and messed up values because of throwing errors is not something new. It is always their, nor and becuase of this new suggestion.
Consider this example.

let s =0;
try{
 s++;
 x.e;
}catch(e){
console.log(e)
}
console.log("S is:",s); //here the s will be 1;
  1. The new additional syntax suggestion await[timeoutInMs] :
  • Do not breaks any prior codes, unless used.

  • Gives a finer control in developer hands.

  • Each library which has proper try catch bloc and cleaning in place , will have no effect as throwing errors and catching them is a pretty common trick.

  • await[timeoutInMS] is expected to create an interrupt error. And as user who is using it is well aware (even compiler can warn user about possible interrrupt error where that syntax is used), they must wrap such a code in try catch block.
    I have proposed this in original post, little later though:
    error expectation

  • . This make code more predictable. A line where await[timeInMs] is mentioned is guaranteed to be completed in a time span, if the our code is worst performing than that, this will help debug the code for potential bottlenecks.

You cannot know that. You have to wrap the thing in try..finally to protect yourself from bugs/exceptions it doesn't handle:

async function doLongRunningTask() {
  const connection = await getConnectionFromPool()
  let res;
  try {
    res = await connection.fetchResource()
  } finally {
    connection.release()
  }
  const { error, resource } = res;
  ...

No, but they must deal gracefully with exceptions at every function call or await or yield expression (those latter two are natural cancellation points, because they are allowed to never return in current ES). Cancellation is a request, not a command (ever tried Ctrl-C on a program that's not cooperating? :D). It's not the same thing as "force-stopping" a running function at any point, which obviously is a no-go.

Perhaps that wasn't the best example. Let's try another.

async function doLongRunningTask() {
  const connection = await getConnectionFromPool()
  return new Promise((resolve, reject) => {
    connection.fetchResource((err, resource) => {
      connection.release()
      if (err) reject(err)
      else resolve(resource)
    })
  })
}

Where should a timeout error be thrown in this scenario? It's a bit easier to pick a spot when everything is using await, but what if they're using callbacks internally?

And, can we agree that it's appropriate for connection.release() to go there? If something goes wrong, fetchResource() is supposed to call the callback with an error object, not throw it.

Doesn't work FWIW :(. It waits for the currently awaited promise to settle and only then throws.

I am re-evaluating, will take some time to come back with a solution to the seemigly valid problem.

async function* tickingStream() {
   let i = 0;
   while (i++ < 10) {
     await new Promise(resolve => setTimeout(resolve, 1_000));
     console.log(`tick yield ${i}`);
     yield i;
   }
}

async function* wrapAsyncIterableInTimeout(ms, it) {
   const finish = new Promise((_, reject) => setTimeout(reject, ms, new Error('timeout')));
   let v = it.next();
   while (true) {
      let n = await Promise.race([finish, v]);
      if (n.done) break;
      yield n.value;
      v = it.next(); 
   }
}

for await (let v of timeout(5_000, stream())) {
  console.log(v);
}
1 Like

@aclaymore - the thing @anuragvohraec isn't liking is the fact that, even though the Promise.race() in that example will correctly cause you to stop awaiting that particular iteration early, it does nothing to force stop the "v" promise that was passed into Promise.race(). That async task is still in the background, actively doing whatever it wanted to do, and if it was hogging up a lot of system resources, it still would be doing so.

1 Like

I will point out that foreign code having the ability to hog system resources, out of the control of the caller, is nothing unique to promises. For example, a badly implemented/utalized memoization function could easily become a memory leak.

Even if you force-stopped the execution of an async task, if they were storing stuff globally due to memoization, or whatever other reason, those resources are not going to be able to get cleaned up, since they weren't in local variables to begin wtih.

In general, when you use a third-party library, you're just going to have to trust that they do a good job with their implementation, and they don't hog a bunch of resources. Or, that they at least provide facilities to let you release those resources if the nature of their logic requires operation on large datasets. If you don't trust that library, then don't use it, pick another one. If they do store stuff globally, or on the module level, then there's nothing JavaScript could do to "smartly" release those resources - you have to ask the library itself to release them, because only the library knows how to properly dispose of those resources, and when it's ok to do so.

and Promises represent a ‘result’ not a ‘task’.
Now that AbortController is in Node and Web maybe more and more APIs that start tasks will take an AbortSignal as a parameter.

1 Like