Should the Repeater automatically stop when its executor fulfills? about repeater HOT 5 OPEN

As I am starting to use this library I was actually expecting this and it took me some time to realize that I have to add a stop call at the end of the executor function. If the AsyncIterable isnt disposed once the end of the executor is reached people can just add an await stop at the end of the executor function 🤔

from repeater.

@n1ru4l This is one of those issues I’ve been thinking about on and off this late, wretched year. I do think there’s something aesthetically pleasing about keeping the repeater open only during the execution of the executor; nevertheless, my current thinking is that if we changed the behavior to auto-stop, it would become a common pitfall, as people might forget to leave the repeater open and wonder why the repeater had abruptly closed. I think auto-stopping would be tricky to debug if you were not familiar with the library. Therefore, I concluded that the current behavior is not that bad even if it is unexpected, and that if you want to end iteration, calling stop() isn’t a bad trade-off for the sake of explicitness and ease of use for new users. On top of that, not having to release another major version is always a plus.

I truly appreciate you sharing your experience, but I’m not sure if it’s reason enough to change the behavior of the library at this time. But I do like to keep an open mind. Maybe we can compile a pro-con list for auto-stopping the repeater and decide based on that.

from repeater.

@brainkim this caught me by surprise too, I think it was causing my event loop to die, though I haven't gotten to the bottom of it. Event loop death is one of the most confusing things to debug in Node.js so I argue that keeping the repeater open should be opt-in behavior rather than opt-out. I think it will be easier for people who forgot to keep their repeater open will figure out their mistake than it is for people like me to figure out we explicitly have to call stop.

from repeater.

@brainkim the docs are certainly unclear on this. This adds to the confusion:

If the executor returns normally, the return value of the executor is used as the final value of the repeater.

Also, nothing I see in the docs explicitly states that the repeater will stay open if stop is never called (potentially allowing the event loop to die).

from repeater.

Another question: in practice are there many use cases where you would want to leave a repeater open as long as the consumer is iterating without needing to await stop and perform some kind of cleanup afterward?

from repeater.