Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise.done() doesn't return a promise, docs say it should. #160

Closed
oderby opened this issue Mar 22, 2014 · 7 comments
Closed

Promise.done() doesn't return a promise, docs say it should. #160

oderby opened this issue Mar 22, 2014 · 7 comments

Comments

@oderby
Copy link

oderby commented Mar 22, 2014

Perhaps I'm misunderstanding something here, but the API reference indicates that Promise.done() should return a promise (https://github.com/petkaantonov/bluebird/blob/master/API.md#donefunction-fulfilledhandler--function-rejectedhandler---function-progresshandler----promise)

But I've noticed that doesn't appear to be the case, and indeed looking at the source, the done method doesn't return anything (https://github.com/petkaantonov/bluebird/blob/master/src/promise.js#L140)

I'm not sure what is the proper behavior, but I think done() should return a promise.

My use case is something akin to the following: I have a server handling a client request. So I do the following

var promise = Promise.try(processRequest).cancellable().then(sendResponse).catch(handleError).done()
response.on('close', function() {promise.cancel();})

The problem is whenever the client disconnects before I respond, I can't cancel the promise because the promise var is undefined (because done() doesn't return anything).

@petkaantonov
Copy link
Owner

It's not supposed to return anything so the docs are wrong.

@benjamingr
Copy link
Collaborator

Personally I find unhandled rejection detection quite good and avoid 'done' altogether, that said:

I think it should return the same promise it gets as input - this is usually the assumed behavior since it's what other APIs do.

On 22 במרץ 2014, at 20:31, Antonov [email protected] wrote:

It's not supposed to return anything so the docs are wrong.


Reply to this email directly or view it on GitHub.

@benjamingr
Copy link
Collaborator

On second thought returning nothing is also acceptable IMO. I used .done when using jQuery to return a promise and handle it's resolution but that's just as easy as creating a temp reference to it.

@petkaantonov
Copy link
Owner

@benjamingr No other library's done returns the promise, it's always undefined.

@domenic
Copy link

domenic commented Mar 22, 2014

Returning undefined enforces good behavior, i.e. using done() as a chain terminator instead of a brancher.

@oderby
Copy link
Author

oderby commented Mar 22, 2014

Okay, makes sense that done() should return nothing.

@petkaantonov
Copy link
Owner

Fixed in e18fc46

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants