search

morris / vinyl-ftp

Blazing fast vinyl adapter for FTP
Other
388 stars 31 forks source link

Propagate error #68

Open hmalphettes opened 8 years ago

hmalphettes commented 8 years ago

Testing with an ftp URL that does not hit the right port, I get an UncaughtException.

This patch will propagate the error so that it can be handled with an error listener on the stream returned by vinylFtp.src

Thanks again for your attention!

morris commented 8 years ago

This looks like a hack, the error should be propagated already. Maybe it's an issue with the parallel stream? Might be better to fix that instead.

hmalphettes commented 8 years ago

@morris as far as I understand and as I have experienced it, it is expected that when one uses pipe one is in charge of setting up the error propagation.

Here is an old thread from 2012: https://groups.google.com/forum/#!topic/nodejs/lJYT9hZxFu0 or a more concise summary: http://stackoverflow.com/a/22389498/1273401

In my code I never use pipe. I rely on https://github.com/mafintosh/pump and https://github.com/alessioalex/bubble-stream-error to propagate the errors and destroy each step of the pipeline.

hmalphettes commented 8 years ago

@morris I apologize if you are already on the top of all this. I have not used parallel-transform much really.

morris commented 8 years ago

No problem at all! I'm glad you found an issue and contributed a solution. I'll dig into this many thanks 👍

arabm commented 7 years ago

@morris Did you had time to dig into it ?