Detecting unhandled rejections

[arnaud-lb]

When using promises, there are a good chance that some code will forget to handle rejections/exceptions in a promise chain. This has the result of effectively hidding any rejection/exception that occurred during the execution of a promise's callback, which is dangerous and can lead to undetected bugs.

What makes this worse is that is suffices of a single error/forget in a promise chain to hide all exceptions thrown anywhere in the chain, including errors in react-php: reactphp/http-client#31, reactphp/dns#26.

It's super easy to not handle a rejection: if a promise chain is ended by anything except done(), any errors in the chain will be unhandled and potentially hidden:

$promise->always(function() {
    doSomething();
});

This example is quite obvious: any exception thrown in doSomething() is caught and hidden from the user.

Other example from react/http-client:

$promise->then(function () {
    // ok
}, function ($err) {
    $this->emit('error', $err);
    // in the 'error' callback:
    $this->retry();
})->then(function () {
    doSomething();
});

This one is less obvious. In this example, any exception thrown in retry() is caught by the promise and effectively hidden from the user.

Some promise implementations are trying to detect unhandled rejections, and log them: http://bluebirdjs.com/docs/api/error-management-configuration.html

It is possible to do so in react/promise too. For that, we would have to simply watch when promises are instantiated, handled, and destructed. If a non-handled promise is destructed, it means that an unhandled rejection occurred.

Here is a PoC adding tracing functionality to promises: https://github.com/arnaud-lb/promise/commit/90e6e350c0464961c032e196826bc509dfe1cc82

And here is a PoC tracer that detects unhandled rejections: https://gist.github.com/arnaud-lb/a2a5a5480bbd80013f756ff968282936

This can be used like this:

<?php

$tracer = new UnhandledRejectionTracer(function ($info) {
    var_dump("unhandled promise", $info);
});
RejectedPromise::setTracer($tracer);
FulfilledPromise::setTracer($tracer);

register_shutdown_function(function () use ($tracer) {
    // handle non-destructed promises
    foreach ($tracer->getRemainingPromises() as $info) {
        var_dump("unhandled promise", $info);
    }
});

This immediately discovered a few bugs in my code. I'm using this since a few months already, and this prevents the introduction of new bugs.

[jsor]

Thanks for this issue and for the interesting approach. Interestingly, i've started looking into this myself a few days ago :)

As you said, the only way to not swallow exceptions is to end every promise chain with done() (which i highly recommend, also for the reason, that done() does not create an unused child promise).

There are some issues that need to be resolved first, but i definitely will look into this then.

[ghost]

@jsor Is there any update on this matter?

[pavarnos]

This would help so much for people new to Promises / ReactPHP. There are many ways to hurt yourself... I have found a lot of them :-). Making it also work for EventEmitters would be a big help too: finding all emitters that do not have an on('error', ...).

In the docs it says to use done(), but done() is not part of the PromiseInterface that everything returns....

[arnaud-lb]

FWIW, I'm using https://github.com/arnaud-lb/promise/commit/90e6e350c0464961c032e196826bc509dfe1cc82 in production with this tracer for years now, and it has successfully detected any unhandled rejection since then.

Feel free to make a PR from the diff :)

[pavarnos]

Thank you. I had a go at making something based on your gist. It is here https://github.com/pavarnos/promise/tree/tracer. I hit a few snags I could not resolve by myself

• unit tests fail with 1) React\Promise\RejectedPromiseTest::shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToRejectedPromiseWithAlwaysFollowers Failed asserting that 154 is identical to 0. iff RejectedPromise has a __destruct() method. I have no clue why the test is needed or why it fails
• I tried to guess where best to put 'traceHandled()' calls in the Promise library then ran it on my code. I got a lot of reports of unhandled promises (one or two listed below) that are deep in third party libraries and not from my code. I have spent a fair bit of time trying to understand the Promise library and why it works the way it does, but have not had much luck yet (still a bit new to this style of programming).

Some examples

• https://github.com/reactphp/dns/blob/master/src/Query/CachingExecutor.php#L48 wants to store something in the cache but probably doesn't need to worry about when the cache is finished? Does this mean I need to always ignore some classes?
• https://github.com/reactphp/promise-timer/blob/master/src/functions.php#L31: should this return $resolve($v)? Similarly line 37
• https://github.com/friends-of-reactphp/mysql/blob/master/src/Factory.php#L180: reports as an error (trace below) when using a LazyConnection but it probably shouldn't. Again my promise-fu is letting me down somewhere? Maybe I set it up the tracing wrong: it seems like Deferred are not quite handled correctly?

#0 /var/local/advantage/vendor/react/promise/src/TraceableTrait.php(34): React\Promise\UnhandledRejectionTracer->instantiated()
#1 /var/local/advantage/vendor/react/promise/src/FulfilledPromise.php(21): React\Promise\FulfilledPromise->traceInstantiated()
#2 /var/local/advantage/vendor/react/promise/src/functions.php(39): React\Promise\FulfilledPromise->__construct()
#3 /var/local/advantage/vendor/react/promise/src/Promise.php(237): React\Promise\resolve()
#4 /var/local/advantage/vendor/react/promise/src/FulfilledPromise.php(49): React\Promise\Promise::React\Promise\{closure}()
#5 /var/local/advantage/vendor/react/promise/src/Promise.php(141): React\Promise\FulfilledPromise->done()
#6 /var/local/advantage/vendor/react/promise/src/Promise.php(174): React\Promise\Promise::React\Promise\{closure}()
#7 /var/local/advantage/vendor/react/promise/src/Promise.php(237): React\Promise\Promise->settle()
#8 /var/local/advantage/vendor/react/promise/src/Deferred.php(36): React\Promise\Promise::React\Promise\{closure}()
#9 /var/local/advantage/vendor/react/mysql/src/Factory.php(180): React\Promise\Deferred->resolve()
#10 /var/local/advantage/vendor/evenement/evenement/src/Evenement/EventEmitterTrait.php(123): React\MySQL\Factory->React\MySQL\{closure}()
#11 /var/local/advantage/vendor/react/mysql/src/Io/Parser.php(313): Evenement\EventEmitter->emit()
#12 /var/local/advantage/vendor/react/mysql/src/Io/Parser.php(204): React\MySQL\Io\Parser->onSuccess()
#13 /var/local/advantage/vendor/evenement/evenement/src/Evenement/EventEmitterTrait.php(123): React\MySQL\Io\Parser->parse()
#14 /var/local/advantage/vendor/react/stream/src/Util.php(71): Evenement\EventEmitter->emit()
#15 /var/local/advantage/vendor/evenement/evenement/src/Evenement/EventEmitterTrait.php(123): React\Stream\Util::React\Stream\{closure}()
#16 /var/local/advantage/vendor/react/stream/src/DuplexResourceStream.php(193): Evenement\EventEmitter->emit()
#17 /var/local/advantage/vendor/react/event-loop/src/StreamSelectLoop.php(245): React\Stream\DuplexResourceStream->handleData()
#18 /var/local/advantage/vendor/react/event-loop/src/StreamSelectLoop.php(212): React\EventLoop\StreamSelectLoop->waitForStreamActivity()
#19 /var/local/advantage/src/cylance-poll.php(40): React\EventLoop\StreamSelectLoop->run()
#20 {main}

[WyriHaximus]

Just opened a PR to implement this: https://github.com/reactphp/promise/pull/170