repeaterjs / repeater

The missing constructor for creating safe async iterators
https://repeater.js.org
MIT License
459 stars 12 forks source link

Stop promise not entirely compatible with Promise.race #74

Open jessmorecroft opened 2 years ago

jessmorecroft commented 2 years ago

Hi, noticing in my tests that when i'm trying to use the stop promise in a call to Promise.race, it's not prioritising this promise even though it's clearly settled. I'm guessing this is happening because Stop is not a "native" promise, and Promise.race is not seeing it as immediately settled?

To demonstrate, here's some code that runs to completion:

    async function* increment() {
      let count = 0;
      for (;;) {
        ++count;
        await promisify(nextTick)(); // this line is required or we never exit
        yield count;
      }
    }

    const repeater = new Repeater<number>(async (push, stop) => {
      const iter = increment()[Symbol.asyncIterator]();
      //const stop2 = stop.then((a) => a); // workaround
      for (;;) {
        const result = await Promise.race([stop, iter.next()]); // workaround: use stop2 here instead of stop
        if (result === undefined) {
          // stopped by our caller
          await iter.return();
          break;
        }
        if (result.done) {
          // source iterator is done
          stop();
          break;
        }

        await push(result.value);
      }
    });

    for await (const count of repeater) {
      if (count >= 5) {
        break;
      }
    }

If you now try running this but this time comment out the "nextTick" line, you'll note the code hangs. What's actually happening is the implicit call to end the repeater on the final loop's break never ends, as the code is stuck in the repeater loop with the call to Promise.race always returning the promise result for iter.next, despite stop having clearly resolved and being listed first.

To "fix" this behaviour I can uncomment the workaround line and use stop2 in the race call. This then causes race to resolve to the stop2 promise and we therefore exit the loop.

Please let me know if there's a better way to structure this code (dealing with never ending source async iterators) to avoid this entirely.