Add self awareness of terminal promise to simulate done()? about promises-spec HOT 15 CLOSED

On the subject of whether this belongs in Promises/A+, my first instinct is that it doesn't. It feels out of scope, even though I know that debugging promises is a point of frustration, especially for folks who aren't as experienced with them. A debug module/plugin can provide this kind of functionality (e.g. when/debug).

I've actually been experimenting with that idea, specifically here and here, and also in when/debug module. It seems to work ok, but there are some tricky cases. For example:

var pending = createPending();
pending.reject(err);
return pending.promise;

It's not clear at that point whether someone will add a rejection handler or not after the promise is returned. So, in some cases, an algorithm such as the one you described would probably end up logging something when maybe it shouldn't, or not logging something when maybe it should.

That said, the algorithm is avow.js is quite simple and very similar to what you described. It seems to help in many debugging cases that I've tried, just not all of them ... yet! :) It's certainly not as extensive as the stuff that's being done in Q, but then again, it's not intended to be.

from promises-spec.

Good point. Returning a rejected promise, or simply preserving a resolved/rejected promise for later chaining. The former case could be handled in avows.js by

function rejected(reason) {
    var v = _pending(true);
    v.reject(reason);
    return v.promise;
}

function _pending(handled) { /* body of current pending() */ }
function pending() { return _pending(); }

As for the latter case, since the error is for reporting, it's more an inconvenience than genuine problem. Also, it's likely uncommon to have a promise stowed away to access its value at a later time, especially since access to the resolved value via then() is async. So that seems like not only an edge case, but an edge case whose "bad" part is that it also (harmlessly) logs an error. The promise state is still rejected, so rejectedPromise.then(fulfill, fixerup) should still work as expected.

On the subject of whether this belongs in Promises/A+, my first instinct is that it doesn't.

My first thought as well, though A+ is defining the behavior of then(), and in particular the default behavior in the absence of onRejected.

A debug module/plugin can provide this kind of functionality (e.g. when/debug).

Because of the private nature of the relationship between the promise and its resolver, that seems like it would entail nearly an entire rewrite for the debug module. But I suppose A+ isn't specifying that relationship, so technically the resolver can be stored as a property on the promise to allow defining a class with Promise.prototype.then = .... That said, the want to avoid a rewrite for the error propagation feature would promote the use of prototype based implementations with the side effect of violating the privacy of the resolver. Maybe I'm missing a gray area.

from promises-spec.

though A+ is defining the behavior of then(), and in particular the default behavior in the absence of onRejected.

Hmmm, yes, that's a good point. This feels like behavior that reaches beyond just .then(), though. As you implied, it gets into the realm of fulfill/reject, especially since an error should probably only manifest at the point when the promise is rejected, and not at the point of calling .then().

Interesting thought about allowing handled to be passed in to a private func. Thanks for the idea--I'll do some more thinking along those lines. It's also a good point that doing something simple along the lines of what avow.js does now, possibly erring on the side of being too noisy, might be "good enough" for most situations.

from promises-spec.

Node does something interesting here in Streams: it warns you when you never registered a listener for the error event.

I think it belongs in a standard (specially considering the last discussions at es-discuss). For instance, if promises were to be part of the DOM, how would you expect them to react to unhandled errors?

from promises-spec.

Node does something interesting here in Streams: it warns you when you never registered a listener for the error event.

Yes, and that's effectively what avow.js does now. For how minimal it is, it can be pretty helpful, but doesn't cover all cases.

The be-all-end-all here will be platform support for unhandled rejections. It's not clear exactly what form that would take, but it's the logical analog to platforms logging uncaught exceptions. Of course, that means promises would need to be a platform-provided feature, which may indeed be the case for DOM Promises.

I still feel like this crosses too much into resolver-land to be specified for .then(). It doesn't seem right for .then() to provide error logging about the rejection state of a promise chain. It makes more sense to me that a resolver's reject() method might log something. In that case, the particular implementation of .then() might set a flag that a corresponding resolver checks later, but that's totally an implementation detail and not something that should be in a spec for .then(), imho.

Maybe it's another topic as to whether we should address a .done() or .end() API, similar to what Q provides. I'm not a fan of sprinkling those throughout code--it feels very artificial, although I certainly see the practical value of getting better stack traces.

from promises-spec.

The only way to implement it, without breaking in the case where a rejected promise is passed around or cached through multiple event loops etc, would be to have a language feature so you could throw when a promise is garbage collected if the rejection has never been handled. That would require the feature to be provided by the host environment. I think it's important to ensure that the spec doesn't prohibit a promise library supporting such behaviour, but I don't think we need to specify exactly how they go about supporting the behaviour.

from promises-spec.

Yes, this is not something we can or should include in the spec. Adding it contradicts the existing spec and tests. (retracted) @ForbesLindesay summed up the problem most accurately.

For more details, see my response on es-discuss.

from promises-spec.

Let me clarify. I'm, not saying that "the runtime should throw if no handlers present" should be part of the spec. I'm saying handling of errors when there are no handlers should be spec'd, whether that means doing nothing, throwing or doing something else.

from promises-spec.

@juandopazo ok good clarification. I think it's impossible to conform to the rest of the spec if you throw, but I might be wrong. Will see if I can come up with a proof.

from promises-spec.

OK, I've thought about this. And I believe that the behavior is not covered by the spec... and that's probably OK. As long as an implementation can meet all other requirements, it can do whatever it wants. That is, as long as a rejected promise can have a rejection handler attached to it, even a second after rejection, and that handler is called according to the spec, it doesn't matter if the implementation did something else about the unhandled rejection in that intervening second.

From another perspective, I personally believe that rethrowing the error is a bad idea (it will crash your app for example). But I also believe that deleting your C:\ drive is a bad idea. We don't currently prohibit any particular behaviors by an implementation, and the list of "bad" things an implementation could do are limitless.

from promises-spec.

From another perspective, I personally believe that rethrowing the error is a bad idea (it will crash your app for example)

As mentioned in the original issue description, I was imagining the rethrow would be deferred to the next tick with setImmediate() or its ilk to prevent this. I agree throwing synchronously is a bad idea.

I'm fine with leaving additional reporting/error propagation unspecified, since it wouldn't impact the functional behavior of then() on a pending or rejected promise (assuming the setImmediate() wrapping).

from promises-spec.

As mentioned in the original issue description, I was imagining the rethrow would be deferred to the next tick with setImmediate() or its ilk to prevent this.

Actually the setImmediate is exactly what crashes your app, since it makes the error uncatchable.

I agree throwing synchronously is a bad idea.

This is a good point, and related to #25. I think it would be worth specifying that then should never throw an exception. I will open a pull request for that.

from promises-spec.

Unfortunately, a next-tick throw will crash node (and maybe others?)

from promises-spec.

Obviously, I haven't done any significant coding in node. GTK, thanks.

Seems this issue is closed, unless I'm forgetting something. Feel free to close it if you agree.

from promises-spec.

Ok, sounds we're all ok with leaving it unspecified. Closing.

from promises-spec.