reactphp / promise

Promises/A implementation for PHP.
https://reactphp.org/promise/
MIT License
2.38k stars 146 forks source link

Report any unhandled promise rejections #248

Closed clue closed 1 year ago

clue commented 1 year ago

This changeset adds new functionality to make sure we report any unhandled promise rejections. If you remove the last reference to a rejected promise that has not been handled, it will now report an unhandled promise rejection:

function incorrect(): int
{
     $promise = React\Promise\reject(new RuntimeException('Request failed'));

     // Commented out: No rejection handler registered here.
     // $promise->then(null, function (\Throwable $e): void { /* ignore */ });

     // Returning from a function will remove all local variable references, hence why
     // this will report an unhandled promise rejection here.
     return 42;
}

// Calling this function will log an error message plus its stack trace:
// Unhandled promise rejection with RuntimeException: Request failed in example.php:10
incorrect();

The old behavior was to simply hide any unhandled rejections which has been a constant source of frustration for newcomers and for larger applications alike (see #87 and dozens of referenced tickets). The updated implementation provides a better default behavior that is more in line with how exceptions in PHP are also reported by default and significantly improves error reporting.

Any unhandled promise rejections will currently always be written to PHP's default error log location (STDERR for cli, Apache error log, etc.). Once this PR is merged, I've prepared a follow-up PR to provide a custom log handler function to write to custom log locations similar to how set_exception_handler() and set_error_handler() work.

I've also tested this branch against all ReactPHP components and dozens of downstream dependencies. Most projects do indeed execute just fine these changes. However, it is quite common for unhandled promise rejections to occur in their test suites. I've prepared a number of PRs for downstream components that I would file in the next couple of days to address this.

This PR comes with a sophisticated test suite and does not otherwise affect the existing behavior as verified by the existing test suite, so this should be safe to apply. Combined, you're looking a several days worth of work, enjoy! 🎉

Resolves / closes #87 Supersedes / closes #170 Supersedes / closes #222 Builds on top of #246, #240 and others