Dear Jakob,

we had to deal with Promises a lot in a project and ended up creating a copy of the current Squeak promises to adjust some of their behavior, so great that you're bringing it up again! You can find our changed version here: https://github.com/tom95/Pheno/tree/master/src/Pheno-Core.package/PHPromise.class

The example should also work as stated in the spec:

p := PHPromise new.
q := p ifRejected: [:e | 5].
p rejectWith: PHTestApplicationException new.
q "  a PHPromise(resolved: 5) "

Further, we introduced a differentiation between "application errors" and "system errors". It made sense for our usecase, since we were building a GUI application, where things like failed constraints on an update caused a nice user-readable error that was then considered an application error and caught by the ifError: statement. All other errors, however, we signaled right away, since usually this ended up making debugging routines a lot more complicated when each MNU got hidden away. This is also why I am throwing a PHTestApplicationException, rather than the KeyNotFound, in the example above: it overrides `isApplicationError` to return true while `Exception` returns false.

This differentiation worked very well for us, it might be a bit confusing otherwise though, since it may not be exactly what you would expect considering the spec. I would also be happy to adopt the solution that browsers took, i.e. only throwing an exception when there's no registered handlers, and otherwise making the caught exceptions more debuggable of course.

Best,
Tom
________________________________________
From: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> on behalf of Jakob Reschke <forums.jakob at resfarm.de>
Sent: Sunday, April 5, 2020 12:00 AM
To: squeak-dev at lists.squeakfoundation.org
Subject: [squeak-dev] Error in Promises/A+ reject implementation?

Hello all,

I think our Promise behaves wrong, but I would like a second opinion.
We have this test case:

    testifRejectedRunsBlockIfPromiseFails
        | p q error |
        error := nil.
        p := Promise new.
        q := p ifRejected: [:e | error := e].
        p rejectWith: KeyNotFound new.
        self assert: q isRejected.
        self assert: KeyNotFound equals: error class.

But the JavaScript behavior is actually different:

    x = {};
    p = Promise.reject("Rejected");
    q = p.then(null, error => error);
    q.then(value => { x.value = value; }, error => { x.error = error });
    x // <- { value: "Rejected" }

The difference: the chained promise q is resolved instead of rejected.
Otherwise x would look like this: { error: "Rejected" }.

I think the behavior in our Promise is also in violation with the
Promises/A+ specification it claims to implement
(https://promisesaplus.com/):

[...]
2.2.7 then must return a promise.

    promise2 = promise1.then(onFulfilled, onRejected);

2.2.7.1 If either onFulfilled or onRejected returns a value x, run the
Promise Resolution Procedure [[Resolve]](promise2, x).

2.2.7.2 If either onFulfilled or onRejected throws an exception e,
promise2 must be rejected with e as the reason.
[...]

So if I understand this correctly, since in the test case Promise p
gets rejected and the ifRejected: block evaluates to e without being
curtailed by an exception, Promise q should be resolved, not rejected,
with e, according to §2.2.7.1.

Do you agree and should we fix this?


On an only slightly related note, I am unhappy that I cannot debug
errors in promise handler blocks (then: [...] ifRejected: [...]). It
catches all those errors and just rejects the promise. In a server
environment that would probably be ok, but at development time at
least I want to have the chance to investigate the problem in a
debugger. Especially those programmer's errors resulting in
MessageNotUnderstood, which I could fix on the fly and let the program
proceed. In JavaScript you cannot resume exceptions, but in Smalltalk
you can. It might be nice to turn the on: Error do: [:e | p
rejectWith: e] code in Promise into an ifCurtailed: [p rejectWith: e]
to allow exceptions/errors to be resumed before rejecting the promise.
The promise would still be rejected if you press Abandon in the
debugger. Problem: further errors in the rejected handlers would
result in "Unwind error during termination". This could in turn be
avoided by rejecting the promise in the "future", that is the next
cycle of the world. Opinions?

Kind regards,
Jakob