From 60e0768c399859205a084f6fe19ceacb840e579c Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Tue, 15 Nov 2016 17:30:00 +0100 Subject: [PATCH 01/16] Add PromiseErrorHandler to decouple the specification from the event-loop --- README.md | 2 +- composer.json | 2 +- src/PromiseErrorHandler.php | 89 +++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 src/PromiseErrorHandler.php diff --git a/README.md b/README.md index fe465e0..2f6ccfb 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,7 @@ Any implementation MUST at least provide these two parameters. The implementatio > **NOTE:** The signature doesn't specify a type for `$error`. This is due to the new `Throwable` interface introduced in PHP 7. As this specification is PHP 5 compatible, we can use neither `Throwable` nor `Exception`. -All registered callbacks MUST be executed in the order they were registered. If one of the callbacks throws an `Exception` or `Throwable`, it MUST be rethrown in a callable passed to `Loop::defer` so `Loop::onError` can be properly invoked by the loop. `Loop` refers to the [global event loop accessor](https://github.com/async-interop/event-loop/blob/master/src/Loop.php). The `Promise` implementation MUST then continue to call the remaining callbacks with the original parameters. +All registered callbacks MUST be executed in the order they were registered. If one of the callbacks throws an `Exception` or `Throwable`, it MUST be forwarded to `Async\Interop\PromiseErrorHandler::notify`. The `Promise` implementation MUST then continue to call the remaining callbacks with the original parameters. If a `Promise` is resolved with another `Promise`, the `Promise` MUST keep in pending state until the passed `Promise` is resolved. Thus, the value of a `Promise` can never be a `Promise`. diff --git a/composer.json b/composer.json index 170b9ad..501f553 100644 --- a/composer.json +++ b/composer.json @@ -11,4 +11,4 @@ "Interop\\Async\\": "src/" } } -} \ No newline at end of file +} diff --git a/src/PromiseErrorHandler.php b/src/PromiseErrorHandler.php new file mode 100644 index 0000000..19f277d --- /dev/null +++ b/src/PromiseErrorHandler.php @@ -0,0 +1,89 @@ + Date: Wed, 16 Nov 2016 10:03:25 +0100 Subject: [PATCH 02/16] Trigger a fatal error on last chance error handler failure, fail if no handler has been registered --- src/PromiseErrorHandler.php | 43 ++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/src/PromiseErrorHandler.php b/src/PromiseErrorHandler.php index 19f277d..8d47f3c 100644 --- a/src/PromiseErrorHandler.php +++ b/src/PromiseErrorHandler.php @@ -30,7 +30,7 @@ private function __construct() * * @return int Returns an integer identifier which allows to remove the handler again. */ - public static function addHandler(callable $onError) + public static function add(callable $onError) { self::$callbacks[] = $onError; end(self::$callbacks); @@ -42,7 +42,7 @@ public static function addHandler(callable $onError) * * @return bool Returns `true` if the handler existed, `false` otherwise. */ - public static function removeHandler($id) + public static function remove($id) { if (!is_int($id)) { throw new \InvalidArgumentException(sprintf( @@ -56,6 +56,17 @@ public static function removeHandler($id) return $exists; } + /** + * Removes all handlers. + * + * This method should usually not be used, but it may be helpful in unit tests to start from a clean state if + * certain handlers do not clean up after themselves. + */ + public static function reset() + { + self::$callbacks = []; + } + /** * Notifies all registered handlers, that an exception occurred. * @@ -69,7 +80,7 @@ public static function notify($error) // We have this error handler specifically so we never throw from Promise::when, so it doesn't make sense to // throw here. We just forward a generic exception to the registered handlers. $error = new \Exception(sprintf( - "Promise implementation called %s::%s with an invalid argument type: %s", + "Promise implementation called %s::%s with an invalid argument of type '%s'", __CLASS__, __METHOD__, is_object($error) ? get_class($error) : gettype($error) @@ -80,10 +91,32 @@ public static function notify($error) try { $callback($error); } catch (\Exception $e) { - // ignore, we're already a last chance handler + // We're already a last chance handler, throwing doesn't make sense, so use a real fatal + trigger_error(sprintf( + "An exception has been thrown in a handler registered to %s:\n%s", + __CLASS__, + (string) $e + ), E_USER_ERROR); } catch (\Throwable $e) { - // ignore, we're already a last chance handler + // We're already a last chance handler, throwing doesn't make sense, so use a real fatal + trigger_error(sprintf( + "An exception has been thrown in a handler registered to %s:\n%s", + __CLASS__, + (string) $e + ), E_USER_ERROR); } } + + if (empty(self::$callbacks)) { + trigger_error(sprintf( + "An exception has been thrown from an %s::when handler, but no handler has been registered via %s::add." + . " At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT" + . " install an empty handler that just does nothing. If the handler is called, there is ALWAYS" + . " something wrong.\n\n%s", + Promise::class, + PromiseErrorHandler::class, + (string) $error + ), E_USER_ERROR); + } } } From 17008a6229c01d9822c0bc2f5b4eab411d7e589e Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Wed, 16 Nov 2016 11:32:23 +0100 Subject: [PATCH 03/16] Move ErrorHandler to subnamespace, add tests --- .gitignore | 2 ++ .travis.yml | 26 +++++++++++++++++++ composer.json | 14 +++++++--- phpunit.xml.dist | 15 +++++++++++ .../ErrorHandler.php} | 11 ++++---- test/phpt/error_handler_001.phpt | 16 ++++++++++++ test/phpt/error_handler_002.phpt | 13 ++++++++++ test/phpt/error_handler_003.phpt | 18 +++++++++++++ test/phpt/error_handler_004.phpt | 17 ++++++++++++ 9 files changed, 124 insertions(+), 8 deletions(-) create mode 100644 .gitignore create mode 100644 .travis.yml create mode 100644 phpunit.xml.dist rename src/{PromiseErrorHandler.php => Promise/ErrorHandler.php} (95%) create mode 100644 test/phpt/error_handler_001.phpt create mode 100644 test/phpt/error_handler_002.phpt create mode 100644 test/phpt/error_handler_003.phpt create mode 100644 test/phpt/error_handler_004.phpt diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..c95d42e --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +/composer.lock +/vendor/ \ No newline at end of file diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..3203f74 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,26 @@ +language: php + +php: + - 5.4 + - 5.5 + - 5.6 + - 7.0 + - nightly + - hhvm + +matrix: + allow_failures: + - php: nightly + fast_finish: true + +cache: + directories: + - $HOME/.composer/cache + +install: + - composer install + - composer show + +script: + - find -name "*.php" -not -path "./vendor/*" -print0 | xargs -n 1 -0 php -l + - $(php -r 'if (PHP_MAJOR_VERSION >= 7) echo "phpdbg -qrr"; else echo "php";') vendor/bin/phpunit --coverage-text --coverage-clover build/logs/clover.xml \ No newline at end of file diff --git a/composer.json b/composer.json index 501f553..7f3a1db 100644 --- a/composer.json +++ b/composer.json @@ -1,14 +1,22 @@ { "name": "async-interop/promise", - "description": "Promise interface for implementing async operations", + "description": "A promise interface for interoperability in async operations.", "keywords": ["promise", "future", "awaitable", "async", "interop"], "license": "MIT", "require": { - "php": ">=5.4" + "php": ">=5.4.0" + }, + "require-dev": { + "phpunit/phpunit": "^4|^5" }, "autoload": { "psr-4": { - "Interop\\Async\\": "src/" + "Interop\\Async\\": "src" + } + }, + "autoload-dev": { + "psr-4": { + "Interop\\Async\\Promise\\Test\\": "test" } } } diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 0000000..5b07410 --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,15 @@ + + + + ./test + + + ./test/phpt + + + + + ./src + + + \ No newline at end of file diff --git a/src/PromiseErrorHandler.php b/src/Promise/ErrorHandler.php similarity index 95% rename from src/PromiseErrorHandler.php rename to src/Promise/ErrorHandler.php index 8d47f3c..abd4fa9 100644 --- a/src/PromiseErrorHandler.php +++ b/src/Promise/ErrorHandler.php @@ -1,6 +1,8 @@ +--EXPECTF-- +Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. + +Exception in -:%d +Stack trace: +#0 {main} in %s on line %d \ No newline at end of file diff --git a/test/phpt/error_handler_002.phpt b/test/phpt/error_handler_002.phpt new file mode 100644 index 0000000..41380ce --- /dev/null +++ b/test/phpt/error_handler_002.phpt @@ -0,0 +1,13 @@ +--TEST-- +ErrorHandler::notify() does not fatal with a handler +--FILE-- + +--EXPECT-- +1 \ No newline at end of file diff --git a/test/phpt/error_handler_003.phpt b/test/phpt/error_handler_003.phpt new file mode 100644 index 0000000..9dfa8a7 --- /dev/null +++ b/test/phpt/error_handler_003.phpt @@ -0,0 +1,18 @@ +--TEST-- +ErrorHandler::notify() fatals after handlers have been removed with ErrorHandler::reset() +--FILE-- + +--EXPECTF-- +Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. + +Exception in -:%d +Stack trace: +#0 {main} in %s on line %d \ No newline at end of file diff --git a/test/phpt/error_handler_004.phpt b/test/phpt/error_handler_004.phpt new file mode 100644 index 0000000..8f266af --- /dev/null +++ b/test/phpt/error_handler_004.phpt @@ -0,0 +1,17 @@ +--TEST-- +ErrorHandler::notify() converts non-exception to exception +--FILE-- + +--EXPECTF-- +Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. + +Exception: Promise implementation called Interop\Async\Promise\ErrorHandler::notify with an invalid argument of type 'integer' in %s:%d +Stack trace: +#0 -(%d): Interop\Async\Promise\ErrorHandler::notify(42) +#1 {main} in %s on line %d \ No newline at end of file From 765a6f696f72dd63da21826e78fc466e2bdf44a0 Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Wed, 16 Nov 2016 11:40:48 +0100 Subject: [PATCH 04/16] Remove ::class usage, as it's not available in PHP 5.4 --- src/Promise/ErrorHandler.php | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/Promise/ErrorHandler.php b/src/Promise/ErrorHandler.php index abd4fa9..50491c3 100644 --- a/src/Promise/ErrorHandler.php +++ b/src/Promise/ErrorHandler.php @@ -109,15 +109,13 @@ public static function notify($error) } if (empty(self::$callbacks)) { - trigger_error(sprintf( - "An exception has been thrown from an %s::when handler, but no handler has been registered via %s::add." - . " At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT" - . " install an empty handler that just does nothing. If the handler is called, there is ALWAYS" - . " something wrong.\n\n%s", - Promise::class, - ErrorHandler::class, - (string) $error - ), E_USER_ERROR); + trigger_error( + "An exception has been thrown from an Interop\\Async\\Promise::when handler, but no handler has been" + . " registered via Interop\\Async\\Promise\\ErrorHandler::add. At least one handler has to be" + . " registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just" + . " does nothing. If the handler is called, there is ALWAYS something wrong.\n\n" . (string) $error, + E_USER_ERROR + ); } } } From 83e3b815cb3935d4fbd39c5ab23225e973b86468 Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Wed, 16 Nov 2016 11:48:19 +0100 Subject: [PATCH 05/16] Use %s instead of exception name, because PHP 5 vs. PHP 7 --- test/phpt/error_handler_001.phpt | 2 +- test/phpt/error_handler_003.phpt | 2 +- test/phpt/error_handler_004.phpt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/phpt/error_handler_001.phpt b/test/phpt/error_handler_001.phpt index 1403d80..e12d57a 100644 --- a/test/phpt/error_handler_001.phpt +++ b/test/phpt/error_handler_001.phpt @@ -11,6 +11,6 @@ Interop\Async\Promise\ErrorHandler::notify(new Exception); --EXPECTF-- Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. -Exception in -:%d +%s in -:%d Stack trace: #0 {main} in %s on line %d \ No newline at end of file diff --git a/test/phpt/error_handler_003.phpt b/test/phpt/error_handler_003.phpt index 9dfa8a7..0ac7704 100644 --- a/test/phpt/error_handler_003.phpt +++ b/test/phpt/error_handler_003.phpt @@ -13,6 +13,6 @@ Interop\Async\Promise\ErrorHandler::notify(new Exception); --EXPECTF-- Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. -Exception in -:%d +%s in -:%d Stack trace: #0 {main} in %s on line %d \ No newline at end of file diff --git a/test/phpt/error_handler_004.phpt b/test/phpt/error_handler_004.phpt index 8f266af..359097f 100644 --- a/test/phpt/error_handler_004.phpt +++ b/test/phpt/error_handler_004.phpt @@ -11,7 +11,7 @@ Interop\Async\Promise\ErrorHandler::notify(42); --EXPECTF-- Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. -Exception: Promise implementation called Interop\Async\Promise\ErrorHandler::notify with an invalid argument of type 'integer' in %s:%d +%s Promise implementation called Interop\Async\Promise\ErrorHandler::notify with an invalid argument of type 'integer' in %s:%d Stack trace: #0 -(%d): Interop\Async\Promise\ErrorHandler::notify(42) #1 {main} in %s on line %d \ No newline at end of file From 131253d03aa1a2135868cb0029f33b59964c322e Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Wed, 16 Nov 2016 11:53:27 +0100 Subject: [PATCH 06/16] Use more %s because of PHP 5 vs. PHP 7 --- test/phpt/error_handler_004.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/phpt/error_handler_004.phpt b/test/phpt/error_handler_004.phpt index 359097f..460b3dc 100644 --- a/test/phpt/error_handler_004.phpt +++ b/test/phpt/error_handler_004.phpt @@ -11,7 +11,7 @@ Interop\Async\Promise\ErrorHandler::notify(42); --EXPECTF-- Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. -%s Promise implementation called Interop\Async\Promise\ErrorHandler::notify with an invalid argument of type 'integer' in %s:%d +%sPromise implementation called Interop\Async\Promise\ErrorHandler::notify with an invalid argument of type 'integer'%sin %s:%d Stack trace: #0 -(%d): Interop\Async\Promise\ErrorHandler::notify(42) #1 {main} in %s on line %d \ No newline at end of file From fcc5a3f9846baa79416f39a5a1db9e1fbfa44895 Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Wed, 16 Nov 2016 11:59:10 +0100 Subject: [PATCH 07/16] Add tests for order and remove, fix tests for PHP 7+ --- test/phpt/error_handler_001.phpt | 2 +- test/phpt/error_handler_003.phpt | 2 +- test/phpt/error_handler_004.phpt | 2 +- test/phpt/error_handler_005.phpt | 15 +++++++++++++++ test/phpt/error_handler_006.phpt | 18 ++++++++++++++++++ 5 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 test/phpt/error_handler_005.phpt create mode 100644 test/phpt/error_handler_006.phpt diff --git a/test/phpt/error_handler_001.phpt b/test/phpt/error_handler_001.phpt index e12d57a..f6eec67 100644 --- a/test/phpt/error_handler_001.phpt +++ b/test/phpt/error_handler_001.phpt @@ -11,6 +11,6 @@ Interop\Async\Promise\ErrorHandler::notify(new Exception); --EXPECTF-- Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. -%s in -:%d +%s in %s:%d Stack trace: #0 {main} in %s on line %d \ No newline at end of file diff --git a/test/phpt/error_handler_003.phpt b/test/phpt/error_handler_003.phpt index 0ac7704..cf5ef09 100644 --- a/test/phpt/error_handler_003.phpt +++ b/test/phpt/error_handler_003.phpt @@ -13,6 +13,6 @@ Interop\Async\Promise\ErrorHandler::notify(new Exception); --EXPECTF-- Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. -%s in -:%d +%s in %s:%d Stack trace: #0 {main} in %s on line %d \ No newline at end of file diff --git a/test/phpt/error_handler_004.phpt b/test/phpt/error_handler_004.phpt index 460b3dc..cd80def 100644 --- a/test/phpt/error_handler_004.phpt +++ b/test/phpt/error_handler_004.phpt @@ -13,5 +13,5 @@ Fatal error: An exception has been thrown from an Interop\Async\Promise::when ha %sPromise implementation called Interop\Async\Promise\ErrorHandler::notify with an invalid argument of type 'integer'%sin %s:%d Stack trace: -#0 -(%d): Interop\Async\Promise\ErrorHandler::notify(42) +#0 %s(%d): Interop\Async\Promise\ErrorHandler::notify(42) #1 {main} in %s on line %d \ No newline at end of file diff --git a/test/phpt/error_handler_005.phpt b/test/phpt/error_handler_005.phpt new file mode 100644 index 0000000..7061fb8 --- /dev/null +++ b/test/phpt/error_handler_005.phpt @@ -0,0 +1,15 @@ +--TEST-- +ErrorHandler::notify() does call handlers in order +--FILE-- + +--EXPECT-- +123 \ No newline at end of file diff --git a/test/phpt/error_handler_006.phpt b/test/phpt/error_handler_006.phpt new file mode 100644 index 0000000..de3579b --- /dev/null +++ b/test/phpt/error_handler_006.phpt @@ -0,0 +1,18 @@ +--TEST-- +ErrorHandler::notify() fatals after handlers have been removed with ErrorHandler::remove() +--FILE-- + +--EXPECTF-- +Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. + +%s in %s:%d +Stack trace: +#0 {main} in %s on line %d \ No newline at end of file From 10e0168d87f0838303c6dfb5e9e8ba72be7d38a3 Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Wed, 16 Nov 2016 12:05:51 +0100 Subject: [PATCH 08/16] Don't use PHPDBG for now, as it changes backtraces See https://github.com/sebastianbergmann/phpunit/blob/a7f2db56518e50ab92f28f739810dfad2f223b6b/src/Util/PHP.php#L216-L224 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3203f74..cd3f854 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,4 +23,4 @@ install: script: - find -name "*.php" -not -path "./vendor/*" -print0 | xargs -n 1 -0 php -l - - $(php -r 'if (PHP_MAJOR_VERSION >= 7) echo "phpdbg -qrr"; else echo "php";') vendor/bin/phpunit --coverage-text --coverage-clover build/logs/clover.xml \ No newline at end of file + - php vendor/bin/phpunit --coverage-text --coverage-clover build/logs/clover.xml \ No newline at end of file From 13e5aea7ef855fa7b0e8b98531923af8e9687448 Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Wed, 16 Nov 2016 12:14:19 +0100 Subject: [PATCH 09/16] Fix pattern for HHVM, which doesn't print arguments in backtraces --- test/phpt/error_handler_004.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/phpt/error_handler_004.phpt b/test/phpt/error_handler_004.phpt index cd80def..1e978f6 100644 --- a/test/phpt/error_handler_004.phpt +++ b/test/phpt/error_handler_004.phpt @@ -13,5 +13,5 @@ Fatal error: An exception has been thrown from an Interop\Async\Promise::when ha %sPromise implementation called Interop\Async\Promise\ErrorHandler::notify with an invalid argument of type 'integer'%sin %s:%d Stack trace: -#0 %s(%d): Interop\Async\Promise\ErrorHandler::notify(42) +#0 %s(%d): Interop\Async\Promise\ErrorHandler::notify(%s #1 {main} in %s on line %d \ No newline at end of file From dc85e8d52add9ca3b016d650137bc916c04abc88 Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Wed, 16 Nov 2016 12:21:27 +0100 Subject: [PATCH 10/16] Adjust tests to stricter matching --- test/phpt/error_handler_004.phpt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/phpt/error_handler_004.phpt b/test/phpt/error_handler_004.phpt index 1e978f6..af0e70c 100644 --- a/test/phpt/error_handler_004.phpt +++ b/test/phpt/error_handler_004.phpt @@ -11,7 +11,7 @@ Interop\Async\Promise\ErrorHandler::notify(42); --EXPECTF-- Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. -%sPromise implementation called Interop\Async\Promise\ErrorHandler::notify with an invalid argument of type 'integer'%sin %s:%d +%SException%SPromise implementation called Interop\Async\Promise\ErrorHandler::notify with an invalid argument of type 'integer'%S in %s:%d Stack trace: -#0 %s(%d): Interop\Async\Promise\ErrorHandler::notify(%s +#0 %s(%d): Interop\Async\Promise\ErrorHandler::notify(%S) #1 {main} in %s on line %d \ No newline at end of file From 80f375f6b7a55aa305f2128844303128cc68d060 Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Wed, 16 Nov 2016 12:29:19 +0100 Subject: [PATCH 11/16] Show dependency tree on install on Travis --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index cd3f854..13e1d5e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,7 +19,7 @@ cache: install: - composer install - - composer show + - composer show -t script: - find -name "*.php" -not -path "./vendor/*" -print0 | xargs -n 1 -0 php -l From d47e280e9930f874f9715397778302d454f5bba8 Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Wed, 16 Nov 2016 14:08:02 +0100 Subject: [PATCH 12/16] Use parallel lint --- .travis.yml | 4 ++-- composer.json | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 13e1d5e..2740850 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,5 +22,5 @@ install: - composer show -t script: - - find -name "*.php" -not -path "./vendor/*" -print0 | xargs -n 1 -0 php -l - - php vendor/bin/phpunit --coverage-text --coverage-clover build/logs/clover.xml \ No newline at end of file + - php vendor/bin/parallel-lint --exclude vendor . + - php vendor/bin/phpunit --coverage-text --coverage-clover build/logs/clover.xml diff --git a/composer.json b/composer.json index 7f3a1db..05bc9da 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,9 @@ "php": ">=5.4.0" }, "require-dev": { - "phpunit/phpunit": "^4|^5" + "phpunit/phpunit": "^4|^5", + "jakub-onderka/php-parallel-lint": "^0.9.2", + "jakub-onderka/php-console-highlighter": "^0.3.2" }, "autoload": { "psr-4": { From be876a0af94b34c2e47b03de31d565a14d2e64ab Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Wed, 16 Nov 2016 14:14:50 +0100 Subject: [PATCH 13/16] Remove clover coverage output --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 2740850..ede52f6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,4 +23,4 @@ install: script: - php vendor/bin/parallel-lint --exclude vendor . - - php vendor/bin/phpunit --coverage-text --coverage-clover build/logs/clover.xml + - php vendor/bin/phpunit --coverage-text From 341ac6557edd3be988a825f6d471ddbabb5976a4 Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Thu, 22 Dec 2016 09:07:59 +0100 Subject: [PATCH 14/16] Change error handler to a single callback --- README.md | 2 +- src/Promise/ErrorHandler.php | 103 ++++++++++++------------------- test/phpt/error_handler_001.phpt | 4 +- test/phpt/error_handler_002.phpt | 4 +- test/phpt/error_handler_003.phpt | 10 +-- test/phpt/error_handler_004.phpt | 4 +- test/phpt/error_handler_005.phpt | 10 +-- test/phpt/error_handler_006.phpt | 9 ++- 8 files changed, 60 insertions(+), 86 deletions(-) diff --git a/README.md b/README.md index 2f6ccfb..6743aca 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,7 @@ Any implementation MUST at least provide these two parameters. The implementatio > **NOTE:** The signature doesn't specify a type for `$error`. This is due to the new `Throwable` interface introduced in PHP 7. As this specification is PHP 5 compatible, we can use neither `Throwable` nor `Exception`. -All registered callbacks MUST be executed in the order they were registered. If one of the callbacks throws an `Exception` or `Throwable`, it MUST be forwarded to `Async\Interop\PromiseErrorHandler::notify`. The `Promise` implementation MUST then continue to call the remaining callbacks with the original parameters. +All registered callbacks MUST be executed in the order they were registered. If one of the callbacks throws an `Exception` or `Throwable`, it MUST be forwarded to `Async\Interop\Promise\ErrorHandler::notify`. The `Promise` implementation MUST then continue to call the remaining callbacks with the original parameters. If a `Promise` is resolved with another `Promise`, the `Promise` MUST keep in pending state until the passed `Promise` is resolved. Thus, the value of a `Promise` can never be a `Promise`. diff --git a/src/Promise/ErrorHandler.php b/src/Promise/ErrorHandler.php index 50491c3..bf5ede8 100644 --- a/src/Promise/ErrorHandler.php +++ b/src/Promise/ErrorHandler.php @@ -10,11 +10,14 @@ * Callbacks passed to `Promise::when()` should never throw, but they might. Such errors have to be passed to this * global error handler to make them easily loggable. These can't be handled gracefully in any way, so we just enable * logging with this handler and ignore them otherwise. + * + * If handler is set or that handler rethrows, it will fail hard by triggering an E_USER_ERROR leading to script + * abortion. */ final class ErrorHandler { - /** @var callable[] */ - private static $callbacks = []; + /** @var callable|null */ + private static $callback = null; private function __construct() { @@ -22,58 +25,29 @@ private function __construct() } /** - * Adds a new handler that will be notified on uncaught promise resolution callback errors. + * Set a new handler that will be notified on uncaught errors during promise resolution callback invocations. * * This callback can attempt to log the error or exit the execution of the script if it sees need. It receives the * exception as first and only parameter. * - * This callable MUST NOT throw in any case, as it's already a last chance handler. Thus it's suggested to always - * wrap the body of your callback in a generic `try` / `catch` block. - * - * @return int Returns an integer identifier which allows to remove the handler again. - */ - public static function add(callable $onError) - { - self::$callbacks[] = $onError; - end(self::$callbacks); - return key(self::$callbacks); - } - - /** - * Removes the handler with the specified identifier. + * As it's already a last chance handler, the script will be aborted using E_USER_ERROR if the handler throws. Thus + * it's suggested to always wrap the body of your callback in a generic `try` / `catch` block, if you want to avoid + * that. * - * @return bool Returns `true` if the handler existed, `false` otherwise. - */ - public static function remove($id) - { - if (!is_int($id)) { - throw new \InvalidArgumentException(sprintf( - "The provided identifier isn't an integer, %s provided.", - is_object($id) ? get_class($id) : gettype($id) - )); - } - - $exists = array_key_exists($id, self::$callbacks); - unset(self::$callbacks[$id]); - return $exists; - } - - /** - * Removes all handlers. + * @param callable|null $onError Callback to invoke on errors or `null` to reset. * - * This method should usually not be used, but it may be helpful in unit tests to start from a clean state if - * certain handlers do not clean up after themselves. + * @return void */ - public static function reset() + public static function set(callable $onError = null) { - self::$callbacks = []; + self::$callback = $onError; } /** - * Notifies all registered handlers, that an exception occurred. + * Notifies the registered handler, that an exception occurred. * * This method MUST be called by every promise implementation if a callback passed to `Promise::when()` throws upon - * invocation. + * invocation. It MUST NOT be called otherwise. */ public static function notify($error) { @@ -88,34 +62,35 @@ public static function notify($error) )); } - foreach (self::$callbacks as $callback) { - try { - $callback($error); - } catch (\Exception $e) { - // We're already a last chance handler, throwing doesn't make sense, so use a real fatal - trigger_error(sprintf( - "An exception has been thrown in a handler registered to %s:\n%s", - __CLASS__, - (string) $e - ), E_USER_ERROR); - } catch (\Throwable $e) { - // We're already a last chance handler, throwing doesn't make sense, so use a real fatal - trigger_error(sprintf( - "An exception has been thrown in a handler registered to %s:\n%s", - __CLASS__, - (string) $e - ), E_USER_ERROR); - } - } - - if (empty(self::$callbacks)) { + if (self::$callback === null) { trigger_error( "An exception has been thrown from an Interop\\Async\\Promise::when handler, but no handler has been" - . " registered via Interop\\Async\\Promise\\ErrorHandler::add. At least one handler has to be" - . " registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just" + . " registered via Interop\\Async\\Promise\\ErrorHandler::set. A handler has to be registered to" + . " prevent exceptions from going unnoticed. Do NOT install an empty handler that just" . " does nothing. If the handler is called, there is ALWAYS something wrong.\n\n" . (string) $error, E_USER_ERROR ); + + return; + } + + try { + $callback = self::$callback; + $callback($error); + } catch (\Exception $e) { + // We're already a last chance handler, throwing doesn't make sense, so use a real fatal + trigger_error(sprintf( + "An exception has been thrown from the promise error handler registered to %s::set().\n\n%s", + __CLASS__, + (string) $e + ), E_USER_ERROR); + } catch (\Throwable $e) { + // We're already a last chance handler, throwing doesn't make sense, so use a real fatal + trigger_error(sprintf( + "An exception has been thrown from the promise error handler registered to %s::set().\n\n%s", + __CLASS__, + (string) $e + ), E_USER_ERROR); } } } diff --git a/test/phpt/error_handler_001.phpt b/test/phpt/error_handler_001.phpt index f6eec67..0e6560d 100644 --- a/test/phpt/error_handler_001.phpt +++ b/test/phpt/error_handler_001.phpt @@ -9,8 +9,8 @@ Interop\Async\Promise\ErrorHandler::notify(new Exception); ?> --EXPECTF-- -Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. +Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::set. A handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. %s in %s:%d Stack trace: -#0 {main} in %s on line %d \ No newline at end of file +#0 {main} in %s on line %d diff --git a/test/phpt/error_handler_002.phpt b/test/phpt/error_handler_002.phpt index 41380ce..2f87a22 100644 --- a/test/phpt/error_handler_002.phpt +++ b/test/phpt/error_handler_002.phpt @@ -5,9 +5,9 @@ ErrorHandler::notify() does not fatal with a handler require __DIR__ . "/../../vendor/autoload.php"; -Interop\Async\Promise\ErrorHandler::add(function () { print "1"; }); +Interop\Async\Promise\ErrorHandler::set(function () { print "1"; }); Interop\Async\Promise\ErrorHandler::notify(new Exception); ?> --EXPECT-- -1 \ No newline at end of file +1 diff --git a/test/phpt/error_handler_003.phpt b/test/phpt/error_handler_003.phpt index cf5ef09..e8d86fb 100644 --- a/test/phpt/error_handler_003.phpt +++ b/test/phpt/error_handler_003.phpt @@ -1,18 +1,18 @@ --TEST-- -ErrorHandler::notify() fatals after handlers have been removed with ErrorHandler::reset() +ErrorHandler::notify() fatals after handlers have been removed with ErrorHandler::set(null) --FILE-- --EXPECTF-- -Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. +Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::set. A handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. %s in %s:%d Stack trace: -#0 {main} in %s on line %d \ No newline at end of file +#0 {main} in %s on line %d diff --git a/test/phpt/error_handler_004.phpt b/test/phpt/error_handler_004.phpt index af0e70c..9545f73 100644 --- a/test/phpt/error_handler_004.phpt +++ b/test/phpt/error_handler_004.phpt @@ -9,9 +9,9 @@ Interop\Async\Promise\ErrorHandler::notify(42); ?> --EXPECTF-- -Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. +Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::set. A handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. %SException%SPromise implementation called Interop\Async\Promise\ErrorHandler::notify with an invalid argument of type 'integer'%S in %s:%d Stack trace: #0 %s(%d): Interop\Async\Promise\ErrorHandler::notify(%S) -#1 {main} in %s on line %d \ No newline at end of file +#1 {main} in %s on line %d diff --git a/test/phpt/error_handler_005.phpt b/test/phpt/error_handler_005.phpt index 7061fb8..575af0f 100644 --- a/test/phpt/error_handler_005.phpt +++ b/test/phpt/error_handler_005.phpt @@ -1,15 +1,15 @@ --TEST-- -ErrorHandler::notify() does call handlers in order +ErrorHandler::set() replaces the current handler --FILE-- --EXPECT-- -123 \ No newline at end of file +3 diff --git a/test/phpt/error_handler_006.phpt b/test/phpt/error_handler_006.phpt index de3579b..b803c8e 100644 --- a/test/phpt/error_handler_006.phpt +++ b/test/phpt/error_handler_006.phpt @@ -1,18 +1,17 @@ --TEST-- -ErrorHandler::notify() fatals after handlers have been removed with ErrorHandler::remove() +ErrorHandler::notify() fatals if handler throws --FILE-- --EXPECTF-- -Fatal error: An exception has been thrown from an Interop\Async\Promise::when handler, but no handler has been registered via Interop\Async\Promise\ErrorHandler::add. At least one handler has to be registered to prevent exceptions from going unnoticed. Do NOT install an empty handler that just does nothing. If the handler is called, there is ALWAYS something wrong. +Fatal error: An exception has been thrown from the promise error handler registered to Interop\Async\Promise\ErrorHandler::set(). %s in %s:%d Stack trace: -#0 {main} in %s on line %d \ No newline at end of file +#0 {main} in %s on line %d From 4058691a7edb9d09df4474acecfe5109fd399d5c Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Thu, 22 Dec 2016 14:43:48 +0100 Subject: [PATCH 15/16] Use call_user_func instead of direct callback invocation --- src/Promise/ErrorHandler.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Promise/ErrorHandler.php b/src/Promise/ErrorHandler.php index bf5ede8..2eb1a87 100644 --- a/src/Promise/ErrorHandler.php +++ b/src/Promise/ErrorHandler.php @@ -75,8 +75,7 @@ public static function notify($error) } try { - $callback = self::$callback; - $callback($error); + \call_user_func(self::$callback, $error); } catch (\Exception $e) { // We're already a last chance handler, throwing doesn't make sense, so use a real fatal trigger_error(sprintf( From 8fc83b7fd5b49041436c0466b3f8910df2830a3e Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Thu, 22 Dec 2016 15:09:27 +0100 Subject: [PATCH 16/16] Return previous error handler callback upon set --- src/Promise/ErrorHandler.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Promise/ErrorHandler.php b/src/Promise/ErrorHandler.php index 2eb1a87..095d545 100644 --- a/src/Promise/ErrorHandler.php +++ b/src/Promise/ErrorHandler.php @@ -36,11 +36,13 @@ private function __construct() * * @param callable|null $onError Callback to invoke on errors or `null` to reset. * - * @return void + * @return callable|null Previous callback. */ public static function set(callable $onError = null) { + $previous = self::$callback; self::$callback = $onError; + return $previous; } /**