Make it possible to register fatal error listeners separately from th…

…e error listeners (getsentry#788)
fmata · May 3, 2019 · ce0cef9 · ce0cef9
1 parent 59a2113
commit ce0cef9
Show file tree

Hide file tree

Showing 38 changed files with 996 additions and 767 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,8 +3,9 @@
 ## Unreleased
 
 - Mark Sentry internal frames when using `attach_stacktrace` as `in_app` `false` (#786)
-- Increase default severity of `E_RECOVERABLE_ERROR` to `Severity::ERROR`, instead
-  of `Severity::WARNING` (#792)
+- Increase default severity of `E_RECOVERABLE_ERROR` to `Severity::ERROR`, instead of warning (#792)
+- Make it possible to register fatal error listeners separately from the error listeners
+  and change the type of the reported exception to `\Sentry\Exception\FatalErrorException` (#788)
 - Add a static factory method to create a breadcrumb from an array of data (#798)
 - Add support for `SENTRY_ENVRIONMENT` and `SENTRY_RELEASE` environment variables (#810)
 - Fix the default value of the `$exceptions` property of the Event class (#806)

diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -11,7 +11,7 @@
     </php>
 
     <testsuites>
-        <testsuite name="Sentry for PHP Test Suite">
+        <testsuite name="Sentry for PHP">
             <directory>tests</directory>
             <directory suffix=".phpt">tests/phpt</directory>
         </testsuite>

diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php
@@ -24,6 +24,7 @@
 use Sentry\HttpClient\Authentication\SentryAuthentication;
 use Sentry\Integration\ErrorListenerIntegration;
 use Sentry\Integration\ExceptionListenerIntegration;
+use Sentry\Integration\FatalErrorListenerIntegration;
 use Sentry\Integration\RequestIntegration;
 use Sentry\Serializer\RepresentationSerializer;
 use Sentry\Serializer\RepresentationSerializerInterface;
@@ -102,7 +103,8 @@ public function __construct(Options $options = null)
         if ($this->options->hasDefaultIntegrations()) {
             $this->options->setIntegrations(array_merge([
                 new ExceptionListenerIntegration(),
-                new ErrorListenerIntegration($this->options),
+                new ErrorListenerIntegration($this->options, false),
+                new FatalErrorListenerIntegration($this->options),
                 new RequestIntegration($this->options),
             ], $this->options->getIntegrations()));
         }

diff --git a/src/ErrorHandler.php b/src/ErrorHandler.php
@@ -4,6 +4,7 @@
 
 namespace Sentry;
 
+use Sentry\Exception\FatalErrorException;
 use Sentry\Exception\SilencedErrorException;
 
 /**
@@ -18,8 +19,10 @@ final class ErrorHandler
 {
     /**
      * The default amount of bytes of memory to reserve for the fatal error handler.
+     *
+     * @internal
      */
-    private const DEFAULT_RESERVED_MEMORY_SIZE = 10240;
+    public const DEFAULT_RESERVED_MEMORY_SIZE = 10240;
 
     /**
      * @var self The current registered handler (this class is a singleton)
@@ -31,6 +34,11 @@ final class ErrorHandler
      */
     private $errorListeners = [];
 
+    /**
+     * @var callable[] List of listeners that will act of each captured fatal error
+     */
+    private $fatalErrorListeners = [];
+
     /**
      * @var callable[] List of listeners that will act on each captured exception
      */
@@ -52,6 +60,21 @@ final class ErrorHandler
      */
     private $previousExceptionHandler;
 
+    /**
+     * @var bool Whether the error handler has been registered
+     */
+    private $isErrorHandlerRegistered = false;
+
+    /**
+     * @var bool Whether the exception handler has been registered
+     */
+    private $isExceptionHandlerRegistered = false;
+
+    /**
+     * @var bool Whether the fatal error handler has been registered
+     */
+    private $isFatalErrorHandlerRegistered = false;
+
     /**
      * @var string|null A portion of pre-allocated memory data that will be reclaimed
      *                  in case a fatal error occurs to handle it
@@ -82,50 +105,130 @@ final class ErrorHandler
     /**
      * Constructor.
      *
-     * @param int $reservedMemorySize The amount of memory to reserve for the fatal error handler
+     * @throws \ReflectionException If hooking into the \Exception class to
+     *                              make the `trace` property accessible fails
      */
-    private function __construct(int $reservedMemorySize)
+    private function __construct()
     {
-        if ($reservedMemorySize <= 0) {
-            throw new \InvalidArgumentException('The $reservedMemorySize argument must be greater than 0.');
-        }
-
         $this->exceptionReflection = new \ReflectionProperty(\Exception::class, 'trace');
         $this->exceptionReflection->setAccessible(true);
+    }
 
-        self::$reservedMemory = str_repeat('x', $reservedMemorySize);
+    /**
+     * Gets the current registered error handler; if none is present, it will
+     * register it. Subsequent calls will not change the reserved memory size.
+     *
+     * @param int  $reservedMemorySize The amount of memory to reserve for the
+     *                                 fatal error handler
+     * @param bool $triggerDeprecation Whether to trigger the deprecation about
+     *                                 the usage of this method. This is used
+     *                                 to avoid errors when this method is called
+     *                                 from other methods of this class until
+     *                                 their implementation and behavior of
+     *                                 registering all handlers can be changed
+     *
+     * @return self
+     *
+     * @deprecated since version 2.1, to be removed in 3.0.
+     */
+    public static function registerOnce(int $reservedMemorySize = self::DEFAULT_RESERVED_MEMORY_SIZE, bool $triggerDeprecation = true): self
+    {
+        if ($triggerDeprecation) {
+            @trigger_error(sprintf('Method %s() is deprecated since version 2.1 and will be removed in 3.0. Please use the registerOnceErrorHandler(), registerOnceFatalErrorHandler() or registerOnceExceptionHandler() methods instead.', __METHOD__), E_USER_DEPRECATED);
+        }
+
+        self::registerOnceErrorHandler();
+        self::registerOnceFatalErrorHandler($reservedMemorySize);
+        self::registerOnceExceptionHandler();
+
+        return self::$handlerInstance;
+    }
+
+    /**
+     * Registers the error handler once and returns its instance.
+     *
+     * @return self
+     */
+    public static function registerOnceErrorHandler(): self
+    {
+        if (null === self::$handlerInstance) {
+            self::$handlerInstance = new self();
+        }
+
+        if (self::$handlerInstance->isErrorHandlerRegistered) {
+            return self::$handlerInstance;
+        }
 
-        register_shutdown_function([$this, 'handleFatalError']);
+        $errorHandlerCallback = \Closure::fromCallable([self::$handlerInstance, 'handleError']);
 
-        $this->previousErrorHandler = set_error_handler([$this, 'handleError']);
+        self::$handlerInstance->isErrorHandlerRegistered = true;
+        self::$handlerInstance->previousErrorHandler = set_error_handler($errorHandlerCallback);
 
-        if (null === $this->previousErrorHandler) {
+        if (null === self::$handlerInstance->previousErrorHandler) {
             restore_error_handler();
 
             // Specifying the error types caught by the error handler with the
             // first call to the set_error_handler method would cause the PHP
             // bug https://bugs.php.net/63206 if the handler is not the first
             // one in the chain of handlers
-            set_error_handler([$this, 'handleError'], E_ALL);
+            set_error_handler($errorHandlerCallback, E_ALL);
         }
 
-        $this->previousExceptionHandler = set_exception_handler([$this, 'handleException']);
+        return self::$handlerInstance;
     }
 
     /**
-     * Gets the current registered error handler; if none is present, it will register it.
-     * Subsequent calls will not change the reserved memory size.
+     * Registers the fatal error handler and reserves a certain amount of memory
+     * that will be reclaimed to handle the errors (to prevent out of memory
+     * issues while handling them) and returns its instance.
      *
-     * @param int $reservedMemorySize The requested amount of memory to reserve
+     * @param int $reservedMemorySize The amount of memory to reserve for the fatal
+     *                                error handler expressed in bytes
+     *
+     * @return self
+     */
+    public static function registerOnceFatalErrorHandler(int $reservedMemorySize = self::DEFAULT_RESERVED_MEMORY_SIZE): self
+    {
+        if ($reservedMemorySize <= 0) {
+            throw new \InvalidArgumentException('The $reservedMemorySize argument must be greater than 0.');
+        }
+
+        if (null === self::$handlerInstance) {
+            self::$handlerInstance = new self();
+        }
+
+        if (self::$handlerInstance->isFatalErrorHandlerRegistered) {
+            return self::$handlerInstance;
+        }
+
+        self::$handlerInstance->isFatalErrorHandlerRegistered = true;
+        self::$reservedMemory = str_repeat('x', $reservedMemorySize);
+
+        register_shutdown_function(\Closure::fromCallable([self::$handlerInstance, 'handleFatalError']));
+
+        return self::$handlerInstance;
+    }
+
+    /**
+     * Registers the exception handler, effectively replacing the current one
+     * and returns its instance. The previous one will be saved anyway and
+     * called when appropriate.
      *
-     * @return self The ErrorHandler singleton
+     * @return self
      */
-    public static function registerOnce(int $reservedMemorySize = self::DEFAULT_RESERVED_MEMORY_SIZE): self
+    public static function registerOnceExceptionHandler(): self
     {
         if (null === self::$handlerInstance) {
-            self::$handlerInstance = new self($reservedMemorySize);
+            self::$handlerInstance = new self();
+        }
+
+        if (self::$handlerInstance->isExceptionHandlerRegistered) {
+            return self::$handlerInstance;
         }
 
+        self::$handlerInstance->isExceptionHandlerRegistered = true;
+        self::$handlerInstance->previousExceptionHandler = set_exception_handler(\Closure::fromCallable([self::$handlerInstance, 'handleException']));
+
         return self::$handlerInstance;
     }
 
@@ -137,13 +240,36 @@ public static function registerOnce(int $reservedMemorySize = self::DEFAULT_RESE
      * @param callable $listener A callable that will act as a listener;
      *                           this callable will receive a single
      *                           \ErrorException argument
+     *
+     * @deprecated since version 2.1, to be removed in 3.0
      */
     public static function addErrorListener(callable $listener): void
     {
-        $handler = self::registerOnce();
+        @trigger_error(sprintf('Method %s() is deprecated since version 2.1 and will be removed in 3.0. Use the addErrorHandlerListener() method instead.', __METHOD__), E_USER_DEPRECATED);
+
+        $handler = self::registerOnce(self::DEFAULT_RESERVED_MEMORY_SIZE, false);
         $handler->errorListeners[] = $listener;
     }
 
+    /**
+     * Adds a listener to the current error handler to be called upon each
+     * invoked captured fatal error; if no handler is registered, this method
+     * will instantiate and register it.
+     *
+     * @param callable $listener A callable that will act as a listener;
+     *                           this callable will receive a single
+     *                           \ErrorException argument
+     *
+     * @deprecated since version 2.1, to be removed in 3.0
+     */
+    public static function addFatalErrorListener(callable $listener): void
+    {
+        @trigger_error(sprintf('Method %s() is deprecated since version 2.1 and will be removed in 3.0. Use the addFatalErrorHandlerListener() method instead.', __METHOD__), E_USER_DEPRECATED);
+
+        $handler = self::registerOnce(self::DEFAULT_RESERVED_MEMORY_SIZE, false);
+        $handler->fatalErrorListeners[] = $listener;
+    }
+
     /**
      * Adds a listener to the current error handler to be called upon each
      * invoked captured exception; if no handler is registered, this method
@@ -152,13 +278,56 @@ public static function addErrorListener(callable $listener): void
      * @param callable $listener A callable that will act as a listener;
      *                           this callable will receive a single
      *                           \Throwable argument
+     *
+     * @deprecated since version 2.1, to be removed in 3.0
      */
     public static function addExceptionListener(callable $listener): void
     {
-        $handler = self::registerOnce();
+        @trigger_error(sprintf('Method %s() is deprecated since version 2.1 and will be removed in 3.0. Use the addExceptionHandlerListener() method instead.', __METHOD__), E_USER_DEPRECATED);
+
+        $handler = self::registerOnce(self::DEFAULT_RESERVED_MEMORY_SIZE, false);
         $handler->exceptionListeners[] = $listener;
     }
 
+    /**
+     * Adds a listener to the current error handler that will be called every
+     * time an error is captured.
+     *
+     * @param callable $listener A callable that will act as a listener
+     *                           and that must accept a single argument
+     *                           of type \ErrorException
+     */
+    public function addErrorHandlerListener(callable $listener): void
+    {
+        $this->errorListeners[] = $listener;
+    }
+
+    /**
+     * Adds a listener to the current error handler that will be called every
+     * time a fatal error handler is captured.
+     *
+     * @param callable $listener A callable that will act as a listener
+     *                           and that must accept a single argument
+     *                           of type \Sentry\Exception\FatalErrorException
+     */
+    public function addFatalErrorHandlerListener(callable $listener): void
+    {
+        $this->fatalErrorListeners[] = $listener;
+    }
+
+    /**
+     * Adds a listener to the current error handler that will be called every
+     * time an exception is captured.
+     *
+     * @param callable $listener A callable that will act as a listener
+     *                           and that must accept a single argument
+     *                           of type \Throwable
+     */
+    public function addExceptionHandlerListener(callable $listener): void
+    {
+        $this->exceptionListeners[] = $listener;
+    }
+
     /**
      * Handles errors by capturing them through the Raven client according to
      * the configured bit field.
@@ -173,18 +342,16 @@ public static function addExceptionListener(callable $listener): void
      *              handler will be called
      *
      * @throws \Throwable
-     *
-     * @internal
      */
-    public function handleError(int $level, string $message, string $file, int $line): bool
+    private function handleError(int $level, string $message, string $file, int $line): bool
     {
         if (0 === error_reporting()) {
             $errorAsException = new SilencedErrorException(self::ERROR_LEVELS_DESCRIPTION[$level] . ': ' . $message, 0, $level, $file, $line);
         } else {
             $errorAsException = new \ErrorException(self::ERROR_LEVELS_DESCRIPTION[$level] . ': ' . $message, 0, $level, $file, $line);
         }
 
-        $backtrace = $this->cleanBacktraceFromErrorHandlerFrames($errorAsException->getTrace(), $file, $line);
+        $backtrace = $this->cleanBacktraceFromErrorHandlerFrames($errorAsException->getTrace(), $errorAsException->getFile(), $errorAsException->getLine());
 
         $this->exceptionReflection->setValue($errorAsException, $backtrace);
 
@@ -202,10 +369,8 @@ public function handleError(int $level, string $message, string $file, int $line
      * method is used as callback of a shutdown function.
      *
      * @param array|null $error The error details as returned by error_get_last()
-     *
-     * @internal
      */
-    public function handleFatalError(array $error = null): void
+    private function handleFatalError(array $error = null): void
     {
         // If there is not enough memory that can be used to handle the error
         // do nothing
@@ -221,9 +386,12 @@ public function handleFatalError(array $error = null): void
         }
 
         if (!empty($error) && $error['type'] & (E_ERROR | E_PARSE | E_CORE_ERROR | E_CORE_WARNING | E_COMPILE_ERROR | E_COMPILE_WARNING)) {
-            $errorAsException = new \ErrorException(self::ERROR_LEVELS_DESCRIPTION[$error['type']] . ': ' . $error['message'], 0, $error['type'], $error['file'], $error['line']);
+            $errorAsException = new FatalErrorException(self::ERROR_LEVELS_DESCRIPTION[$error['type']] . ': ' . $error['message'], 0, $error['type'], $error['file'], $error['line']);
+
+            $this->exceptionReflection->setValue($errorAsException, []);
 
             $this->invokeListeners($this->errorListeners, $errorAsException);
+            $this->invokeListeners($this->fatalErrorListeners, $errorAsException);
         }
     }
 
@@ -234,10 +402,8 @@ public function handleFatalError(array $error = null): void
      * @param \Throwable $exception The exception to handle
      *
      * @throws \Throwable
-     *
-     * @internal This method is public only because it's used with set_exception_handler
      */
-    public function handleException(\Throwable $exception): void
+    private function handleException(\Throwable $exception): void
     {
         $this->invokeListeners($this->exceptionListeners, $exception);