Change error handler to a single callback

Author: kelunik

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`.
 

src/Promise/ErrorHandler.php

@@ -10,70 +10,44 @@
  * 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()
     {
         // disable construction, only static helper
     }
 
     /**
-     * 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);
         }
     }
 }

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
+#0 {main} in %s on line %d

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
+1

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--
 <?php
 
 require __DIR__ . "/../../vendor/autoload.php";
 
-Interop\Async\Promise\ErrorHandler::add(function () { print "1"; });
-Interop\Async\Promise\ErrorHandler::reset();
+Interop\Async\Promise\ErrorHandler::set(function () { print "1"; });
+Interop\Async\Promise\ErrorHandler::set(null);
 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
+#0 {main} in %s on line %d

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
+#1 {main} in %s on line %d

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--
 <?php
 
 require __DIR__ . "/../../vendor/autoload.php";
 
-Interop\Async\Promise\ErrorHandler::add(function () { print "1"; });
-Interop\Async\Promise\ErrorHandler::add(function () { print "2"; });
-Interop\Async\Promise\ErrorHandler::add(function () { print "3"; });
+Interop\Async\Promise\ErrorHandler::set(function () { print "1"; });
+Interop\Async\Promise\ErrorHandler::set(function () { print "2"; });
+Interop\Async\Promise\ErrorHandler::set(function () { print "3"; });
 Interop\Async\Promise\ErrorHandler::notify(new Exception);
 
 ?>
 --EXPECT--
-123
+3

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--
 <?php
 
 require __DIR__ . "/../../vendor/autoload.php";
 
-$id = Interop\Async\Promise\ErrorHandler::add(function () { print "1"; });
-Interop\Async\Promise\ErrorHandler::remove($id);
+Interop\Async\Promise\ErrorHandler::set(function ($e) { throw $e; });
 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 the promise error handler registered to Interop\Async\Promise\ErrorHandler::set().
 
 %s in %s:%d
 Stack trace:
-#0 {main} in %s on line %d
+#0 {main} in %s on line %d