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

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 <[hidden email]> on behalf of Jakob Reschke <[hidden email]>
Sent: Sunday, April 5, 2020 12:00 AM
To: [hidden email]
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

Hi Jakob,

On 4/5/20 12:00 AM, Jakob Reschke wrote:
> I think our Promise behaves wrong, but I would like a second opinion.

I think you're right. And I think we should fix it. I'll draft a repair.

I don't think there are many users of Promise out there -- if there are,
and any are reading this, please speak up if fixing the bug Jakob
reported would be a problem.

Thanks for digging into the spec here. That'll be helpful for getting
the tests right.

> On an only slightly related note, I am unhappy that I cannot debug
> errors in promise handler blocks [...] Opinions?

I don't know what to think about this, I'm afraid! I'd be keen to hear
more from others about what a good alternative to the current behaviour
might be.

Regards,
  Tony

Hi Tony,

Well, this Promise implementation made it into the Kernel and schedules
into the future (this seems to be the only use). So it certainly strikes
me as official.

Have you ever had a chance to look at the 'CapabilitiesLocal'
implementation I modeled after ERights, many many years ago? It has
tests. I would welcome comment & feedback.

Installer ss project: 'Cryptography'; install: 'CapabilitiesLocal'.

Kindly,
Robert

On 6/4/20 10:16 AM, Tony Garnock-Jones wrote:

Hi Tony,

I reworked the packaging and the code split. Please load the following to see local promises:

Installer ss project: 'Cryptography'; install: 'PromisesLocal'.

and then run the PromisesLocal tests, all #green.

---

And to see the remote promises, load the following:

Installer ss project: 'Cryptography'; install: 'ProCrypto-1-1-1'; install: 'ProCryptoTests-1-1-1'.
Installer ss project: 'Cryptography'; install: 'PromisesLoader'.

Run all tests, EXCEPT PromisesRemote tests. 414 tests, all #green. Now run the following #green PromisesRemote tests:

• Base64EncodingTestCase
• RememberTableTestCase
• SealerUnsealerTestCase
• SerializationTestCase
• SubstitutionSerializationTestCase

As I am porting PromisesRemote onto the latest iteration of the ParrotTalk interface, the remote tests (LookupTestCase & OperationalTestCase) are not green. They're #Red. I wanted to give you a heads up on the package renames for local promises, such that you could not waste your time exploring this implementation. I'll email once I get those remote tests working again.

K, r

Hi Tony,

Well, this Promise implementation made it into the Kernel and schedules 
into the future (this seems to be the only use). So it certainly strikes 
me as official.

Have you ever had a chance to look at the 'CapabilitiesLocal' 
implementation I modeled after ERights, many many years ago? It has 
tests. I would welcome comment & feedback.

Installer ss project: 'Cryptography'; install: 'CapabilitiesLocal'.

Kindly,
Robert

On 6/4/20 10:16 AM, Tony Garnock-Jones wrote:

Hi Jakob,

On 4/5/20 12:00 AM, Jakob Reschke wrote:

I think our Promise behaves wrong, but I would like a second opinion.

I think you're right. And I think we should fix it. I'll draft a repair.

I don't think there are many users of Promise out there -- if there are,
and any are reading this, please speak up if fixing the bug Jakob
reported would be a problem.

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.
[...]

Thanks for digging into the spec here. That'll be helpful for getting
the tests right.

On an only slightly related note, I am unhappy that I cannot debug
errors in promise handler blocks [...] Opinions?

I don't know what to think about this, I'm afraid! I'd be keen to hear
more from others about what a good alternative to the current behaviour
might be.

Regards,
   Tony

Alright, I have updated ThunkStack and PromisesRemote. Please load with the script:

Installer ss project: 'Cryptography'; install: 'ProCrypto-1-1-1'; install: 'ProCryptoTests-1-1-1'.
Installer ss project: 'Cryptography'; install: 'PromisesLoader'.

and run tests, including the test category: PromisesRemote-Testing. #Green! This includes remote tests between two machines {vats}.

The test category: PromisesRemote-3WayTesting has two failures due to 3Way no working right, yet. Here are the two failing tests:

• testThreeWayGranovetter
• testThreeWayGranovetterGalaxyScenario

I am buried in SSL, but I will try to advance PromisesRemote by switching from STON to ASN1 encoding.

K, r

Hi Tony,

I reworked the packaging and the code split. Please load the following to see local promises:

Installer ss project: 'Cryptography'; install: 'PromisesLocal'.

and then run the PromisesLocal tests, all #green.

---

And to see the remote promises, load the following:

Installer ss project: 'Cryptography'; install: 'ProCrypto-1-1-1'; install: 'ProCryptoTests-1-1-1'.
Installer ss project: 'Cryptography'; install: 'PromisesLoader'.

Run all tests, EXCEPT PromisesRemote tests. 414 tests, all #green. Now run the following #green PromisesRemote tests:

• Base64EncodingTestCase
• RememberTableTestCase
• SealerUnsealerTestCase
• SerializationTestCase
• SubstitutionSerializationTestCase

As I am porting PromisesRemote onto the latest iteration of the ParrotTalk interface, the remote tests (LookupTestCase & OperationalTestCase) are not green. They're #Red. I wanted to give you a heads up on the package renames for local promises, such that you could not waste your time exploring this implementation. I'll email once I get those remote tests working again.

K, r

On 6/4/20 5:25 PM, Robert Withers wrote:

Hi Tony,

Well, this Promise implementation made it into the Kernel and schedules 
into the future (this seems to be the only use). So it certainly strikes 
me as official.

Have you ever had a chance to look at the 'CapabilitiesLocal' 
implementation I modeled after ERights, many many years ago? It has 
tests. I would welcome comment & feedback.

Installer ss project: 'Cryptography'; install: 'CapabilitiesLocal'.

Kindly,
Robert

On 6/4/20 10:16 AM, Tony Garnock-Jones wrote:

Hi Jakob,

On 4/5/20 12:00 AM, Jakob Reschke wrote:

I think our Promise behaves wrong, but I would like a second opinion.

I think you're right. And I think we should fix it. I'll draft a repair.

I don't think there are many users of Promise out there -- if there are,
and any are reading this, please speak up if fixing the bug Jakob
reported would be a problem.

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.
[...]

Thanks for digging into the spec here. That'll be helpful for getting
the tests right.

On an only slightly related note, I am unhappy that I cannot debug
errors in promise handler blocks [...] Opinions?

I don't know what to think about this, I'm afraid! I'd be keen to hear
more from others about what a good alternative to the current behaviour
might be.

Regards,
   Tony