Introduce ExtendedPromiseInterface

[jsor]

The ExtendedPromiseInterface extends the PromiseInterface with useful shortcut
and utility methods which are not part of the Promises/A specification.

Implemented methods:

ExtendedPromiseInterface::done()

Status: Implemented

$promise->done(callable $onFulfilled = null, callable $onRejected = null, callable $onProgress = null);

When working with promises, it's a common issue that exceptions (and/or rejections) get lost because there is no way to break out of a promise chain.

With the new ExtendedPromiseInterface::done() you can end a promise chain and unhandled rejections/exceptions will be rethrown in an uncatchable way causing a fatal error.

See:

• done() vs. then()
• ExtendedPromiseInterface::done()
• Issue How am I supposed to deal with the last exception? #18

ExtendedPromiseInterface::otherwise()

Status: Implemented

$promise->otherwise(callable $onRejected);

Shorthand for $promise->then(null, $onRejected).

Additionally, you can type hint the $reason argument of $onRejected to catch only specific errors.

Example:

$promise
    ->otherwise(function(\RuntimeException $reason) {
        // Only catch \RuntimeException instances
    })
    ->otherwise(function(\Exception $reason) {
        // Only catch \Exception instances
    })
    ->otherwise(function($reason) {
        // Catch other errors
    });

The typehint is checked via reflection from $onRejected. Performance impact?

See:

• ExtendedPromiseInterface::otherwise()

ExtendedPromiseInterface::always()

Status: Implemented

$promise->always(callable $onFulfilledOrRejected);

The $onFulfilledOrRejected handler gets called, with no arguments, when the promise either fulfills or rejects.

Example

$promise->always(function () {
    // close files, connections, etc.
});

See:

• ExtendedPromiseInterface::always()

ExtendedPromiseInterface::progress()

Status: Implemented

$promise->progress(callable $onProgress);

Shorthand for $promise->then(null, null, $onProgress).

See:

• ExtendedPromiseInterface::progress()

---

I would love to hear feeback

• on the addition of ExtendedPromiseInterface at all
• on the implementations of the methods of ExtendedPromiseInterface

[nicolas-grekas]

Just read the code: if I throw a React\Promise\UnhandledRejectionException on my side, especially in a ->then() callback, then I can break the promise chain. Good or evil?

[jsor]

Sure, you can abuse that and get unpredictable results ;)
Note, that it is heavily WIP, I'm currently trying to rework done() to be simpler and get rid of UnhandledRejectionException (at least when you reject by throwing an exception).

[jsor]

@nicolas-grekas See c0ba5c3. UnhandledRejectionException is now only a wrapper for non-exception rejection values.

[nicolas-grekas]

Thanks I should test now! Reading the code, I think some type hints/instanceof should be changed from PromiseInterface to ExtendedPromiseInterface isn't it?

[nicolas-grekas]

👍 from me: it solves my debugging experience problem, thanks!

[jsor]

All proposed methods are now implemented.

[mtdowling]

You mentioned in the description that there may be a performance concern with _checkTypeHint. You could measure the performance impact by comparing it against a baseline of just providing a class name followed by a callback function.

For example:

$promise
    ->otherwise('RuntimeException', function(\RuntimeException $reason) {
        // Only catch \RuntimeException instances
    })
    ->otherwise('Exception', function(\Exception $reason) {
        // Only catch \Exception instances
    })
    ->otherwise(function($reason) {
        // Catch other errors
    });

[jsor]

Yes, that's something to consider. I had this version locally but i didn't liked the unexplicit argument signature. Alternatively, there could be 2 methods, otherwise(func) and otherwiseIf(exceptionClass, func).

The typehint check is only done in otherwise, not in then(null, func). Internally, otherwise isn't used as it is intended for userland usage. So, ideally, invocations should be relatively rare.

[jsor]

I did a quick benchmark with your proposed otherwise() implemention.

Implementation: https://gist.github.com/jsor/885c4b54b325ac3d5c7f#file-otherwise-php
Test code: https://gist.github.com/jsor/885c4b54b325ac3d5c7f#file-test-php

Results for 1000 iterations

	otherwise() with typehinting	otherwise() with optional classname argument
Time	0.56231784820557 s	0.50371599197388 s
Memory	4.46MB (peak: 4.51MB)	0.5MB (peak: 0.56MB)

Performance impact seems to be neglible, but memory usage is much higher for the typehint variant.

[mtdowling]

Are you going to add an ExtendedPromisorInterface to be the new https://github.com/reactphp/promise/blob/master/src/PromisorInterface.php?

[jsor]

No, the plan for 3.0 is to merge the ExtendedPromiseInterface (and CancellablePromiseInterface) into PromiseInterface. PromiseInterface would then just mark promises created by React/Promise with the full API.

If you're dealing with untrusted promises, i recommend a little helper function similar to React\resolve:

function resolveExtended($promiseOrValue = null)
{
    if ($promiseOrValue instanceof ExtendedPromiseInterface) {
        return $promiseOrValue;
    }

    if ($promiseOrValue instanceof PromiseInterface) {
        return new Promise(function ($resolve, $reject, $notify) use ($promiseOrValue) {
            $promiseOrValue->then($resolve, $reject, $notify);
        });
    }

    return new FulfilledPromise($promiseOrValue);
}

Do you have a use case where such an interface whould be helpful?

[mtdowling]

Makes sense. Adding many more methods to PromiseInterface will change the way I glue my library and react promises together though. Instead of having my objects implement PromiseInterface (which would have 7 methods now I believe), I would implement PromisorInterface and provide a then() function as a convenience method.

[jsor]

React/Promise could provide a Trait for otherwise()/always()/progress() since they all are just shortcuts around then(). The only methods which must be implemented are done() and cancel() (which you provide anyway).

But i see your point. Will think this through and will definitely get your feedback before doing anything like this.

[mtdowling]

Throwing this out there, though I'm not sure if it's a good idea... What about adding a ThennableInterface that just has a then function on it. Then have PromiseInterface extend ThennableInterface. If this was added, it would be nice if promise forwarding and the aggregate promise functions all typehinted on ThennableInterface instead of the larger PromiseInterface (assuming that they don't require the new helper functions).

[jeremeamia]

👍 for a ThennableInterface.

[jsor]

Funny, i was about to propose the same :)

An alternative would be to handle it like in JavaScript land: All major promise libs allow arbitrary promises as input simply by checking if there's a then method. They then cast the input to an intenal trusted promise (See when, q, rsvp, Bluebird for example).

[mtdowling]

If you do make a change to support something like this, then I would vote for an interface over duck typing because you save the overhead of having to call method_exists() every time you do anything related to promises.

[cboden]

+1 for Thennable > 🐤

[igorw]

No major objections to the patch, though it does feel a bit bloaty. In particular, I feel like forcing every promise to implement ExtendedPromiseInterface should not be necessary. It would be better to turn it into a "facade" that then composes with any other promise, and you just wrap it around everything at the outermost layer without touching all of the existing promise implementations.

Not sure if that's feasible though. And consistent with whatever javascript-kiddies are doing.

[mtdowling]

After looking at the Promises/A+ spec a bit more in-depth, I see that they just say to use untrusted promises that have a then() method. I'm actually very much in favor of that. You could even check if a promise you're given is first Thennable, then check if it has a then method (that is, if foreign promises need to be treated differently for some reason). This would allow react to interop with any other promise libraries (which would be great because I just wrote one, but it has a different design and abstractions, but still implements the same spec).

[jsor]

That was what i proposed here :)
If there is still a need for a Thenable interface, it could be moved to a separate the repository (i wouldn't go so far and propose it as a PSR ;)).

[mtdowling]

Totally! I was wrong! You should just check for a then method. I don't think there needs to be a PSR either (I think the A+ spec is enough). There might not even need to be a Thennable interface really.

[jsor]

👍