Redesign Repeater.prototype.throw

[brainkim]

Repeater.prototype.throw was initially implemented just to have parity with async generator objects in #2. It was designed so that it called stop on the repeater with an error and returned the final value, allowing consumers to catch the error, but after some thinking I’m wondering if we can also give producers a chance to catch the error as well. Specifically, what if calling throw caused the stop promise to reject? I was initially hesitant to implement throw in this way because I was worried about unhandled promise rejections, but after using repeaters for a while I think all responsible/non-toy repeaters involve some cleanup where the stop promise is awaited, and if the executor awaits the stop promise, throwing an error into the repeater should have the same effect. This means that the repeater would not be stopped if the executor caught the stop rejection, mimicking the behavior where generators can continue to yield values after a thrown error using try/catch/finally.

[brainkim]

After some initial work, I think that rather than causing the stop promise to reject, we should cause the push promise to reject. This is because with the 95% use-case of setting up event listeners, awaiting stop, and then tearing down listeners, causing throw to make stop reject would prevent cleanup from happening. Additionally, stop can only resolve once, so throwing multiple errors into the repeater would be impossible. By making push calls reject, we can make errors recoverable. To handle the problem of unhandled rejections, we can use the hack described in this stackoverflow answer to detect if the push call is floating, and rethrow the error passed to throw in the case that it is. If the repeater uses the push promise in any way, it is the responsibility of the producer to make sure that errors thrown into the repeater are handled and cleanup happens. This would preserve the previous behavior of having throw simply finish the repeater with the error.

[brainkim]

Edit: This comment is outdated see #43.

Ultimately, I chose to implement the following behavior for Repeater.prototype.throw. If an error is thrown into the repeater, the previous push call, i.e. the one which would have resolved had Repeater.prototype.next been called, rejects. It is up to the repeater to catch or handle the rejection, but the push promise will also be preemptively caught to prevent unhandled promise rejections. This means that repeaters which ignore the push call entirely, as is the case with 95% of repeaters, will also ignore the thrown in error and continue iteration normally. I don’t think of this as the exception being swallowed or ignored because Repeater.prototype.throw will return a normal iterator result, indicating to the caller that the exception was handled.

However, in the case of a non-empty buffer or a repeater which has yet to start, Repeater.prototype.throw will throw an error. This is because in these cases, there is no previous push call to reject. This might be surprising, but honestly the way the push and buffers work is kinda confusing to begin with, and was mostly done to emulate the way clojure channels/buffers worked.

I initially wanted to have some logic which made throw rethrow the error if the push promise wasn’t handled, but I couldn’t figure out how to get this to work. We can detect unhandled rejections by adding a catch listener to the promise returned from push and patching its then method to detect when the promise is handled, but there wasn’t a clean way to get the throw call itself to wait for this unhandled rejection: any of my attempts to get throw to wait for push to reject or be handled would end up deadlocking the repeater. Perhaps there is a way to get this to work in the future, but it might mean another big refactoring where we get results to wait for the previous value passed into next somehow.

I’m not sure about this solution, but to me this seems like the least surprising behavior, and throwing errors into generators is already so niche and seldom used that I don’t really feel like working on it anymore. Additionally, I’m not sure that patching push promises to detect unhandled rejections is a robust or okay thing to do anyways, so I am happy with this behavior.

[brainkim]

I’m gonna give this one more try. I had a eureka moment in a dream last night.

[brainkim]

Resolved by #43.