search

fraxken / combine-async-iterators

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

Adding option to don't throw error when some of the async iterables throws an error #2

Closed Farenheith closed 4 years ago

Farenheith commented 4 years ago

Motivation

The objective of merging many distinct async iterators maybe is to get all the items you can from all iterables, regardless of one of them throwing an error or not. Because of this, is interessant to keep iterating on all other ones. Also, even if you want to keep iterating regadless of errors, is still good to know if some error ocurred in the process. That's why I put an "errorCallback" in the options parameter.

Also, consider that native Readables are Async Iterables, so, this merging approach could also be used to merge streams :)

Other infos

I also changed the strategy used with unresolved promises because this, in a high concurrent scenario, can lead to some memory leaks in the V8 engine https://bugs.chromium.org/p/v8/issues/detail?id=9858

fraxken commented 4 years ago

Hi @Farenheith,

Sorry i missed the PR in all my notifications. Can you resolve conflict and update version for 2.0.0 ? (i prefer to go with a Major release).

Best Regards, Thomas

Farenheith commented 4 years ago

Thanks for the reply! I'll look into your comments ASAP

Farenheith commented 4 years ago

Hello, Thomas!

I fixed the details you pointed out and changed the version to 2.0.0!

If there is any other change you want me to make just let me know!

Also, the first test sometimes fail the last assertion:

assert.notStrictEqual(retrievedValues.toString(), sorted.toString());

It has something to do with the setTimeout duration randomization. It's a very small chance I suppose, but I didn't get deep into it to not mess up too much with the code.

Best regards!

fraxken commented 4 years ago

Thanks for your work. I have plan for another feat for 2.0.0 so i guess this will be published in few days!