search

fraxken / combine-async-iterators

Combine Asynchronous Iterators (no sequence)
MIT License
14 stars 4 forks source link

Children .return() is called on exception only, whilst it makes sense to call it in just a finally {} block #5

Closed dko-slapdash closed 4 years ago

dko-slapdash commented 4 years ago

https://github.com/fraxken/combine-async-iterators/blob/f47bea22a56dfa3bd88e06d5cbf2885477b23af6/index.js#L36

Example code to reproduce the behavior is here: https://github.com/reconbot/streaming-iterables/issues/46

In short, it the merging result iterator is stopped iterating (i.e. its .return() method was called directly or indirectly), yield on L29 will immediately call the function's return. And in this case, we need to propagate this return to all children iterators to end them too. (Currently the library doesn't do it, it propagates return only on an exception.)

fraxken commented 4 years ago

Hi @dko-slapdash,

Sorry i missed the notifications for both issues you created. There is an open PR that i review and will surely publish a major release soon to correct those issues (i hope).

Best Regards, Thomas

fraxken commented 4 years ago

Now use finally for v2.0.0.