arobson / rabbot

Deprecated: Please see https://github.com/Foo-Foo-MQ/foo-foo-mq
MIT License
277 stars 129 forks source link

rabbit.request().progress() swallowing errors #57

Closed grimurd closed 7 years ago

grimurd commented 7 years ago

When performing a request that has multiple replies, if an error is thrown in the progress() method it seems to be completely swallowed.

Example code:

        rabbot
            .request('exchange', {})
            .progress(reply => {
                throw new Error('ERR')
            })
            .then(final => {
                console.log('final', final);
            })
            .catch(err => console.error(err));

Nothing is logged to the console until the last response arrives. If an error is thrown in the .then() method then it gets propagated down correctly and is logged.

I understand that this something that needs to be handled in a specific way but if this is intended behavior it needs to be documented. I also suggest that any uncaught errors in the progress() method should be printed to the console.

EDIT After looking at the code this seems to be a part of the when promise library. Which has deprecated the progress method. So this will have to be implemented in a different way sometime in the near future. Documenting this behavior will probably be sufficient until that happens.

grimurd commented 7 years ago

After reading the when.js documentation there seems to be a workaround for this.

From their documentation:

Known Issue: If you allow an exception to propagate and there are no more progress handlers in the chain, the exception will be silently ignored. We're working on a solution to this.

So after testing it, this workaround works:

        rabbot
            .request('exchange', {})
            .progress(reply => {
                throw new Error('ERR')
            })
            .progress(err => {
                // This method is called every time a reply is received (except for 
                // the final reply), the argument will be whatever is returned from 
                // the first progress() call or an error if one was thrown. 
                if(err) console.error('progress error', err);
            })
            .then(final => {
                console.log('final', final);
            })
            .catch(err => console.error(err));
iamjochem commented 7 years ago

when.js is not the only Promise lib to effectively abandon support for progress: http://bluebirdjs.com/docs/api/progression-migration.html (that page offers a few alternatively strategies for tracking progress which might be of interest here)