Skip to content

Fix error handling in Deferred's promise functions#191

Merged
particlebanana merged 1 commit intobalderdashy:masterfrom
leedm777:q-exceptions
Dec 19, 2013
Merged

Fix error handling in Deferred's promise functions#191
particlebanana merged 1 commit intobalderdashy:masterfrom
leedm777:q-exceptions

Conversation

@leedm777
Copy link
Contributor

@leedm777 leedm777 commented Dec 5, 2013

With Q promises, if the then(), fail() or spread() handlers throw an
exception, the promise is rejected. The wrapper functions in Deferred,
however, were simply throwing those exceptions on up.

Rather than try to duplicate those functions in waterline, this patch
adds a toPromise() method to Deferred which simply executes the query
and creates a Q Promise with the result. Then then(), fail() and
spread() functions are then just wrappers calling those methods on the
Promise.

@bmac
Copy link
Contributor

bmac commented Dec 14, 2013

I would love to see this merged in. Right now I end up using Q.ninvoke instead of waterline's built in then method because it doesn't support this functionality.

With Q promises, if the then(), fail() or spread() handlers throw an
exception, the promise is rejected. The wrapper functions in Deferred,
however, were simply throwing those exceptions on up.

Rather than try to duplicate those functions in waterline, this patch
adds a toPromise() method to Deferred which simply executes the query
and creates a Q Promise with the result. Then then(), fail() and
spread() functions are then just wrappers calling those methods on the
Promise.
particlebanana added a commit that referenced this pull request Dec 19, 2013
Fix error handling in Deferred's promise functions
@particlebanana particlebanana merged commit 0237e6e into balderdashy:master Dec 19, 2013
@particlebanana
Copy link
Contributor

Thanks, I'm not a user of promises so thanks for the updates guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants