Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup the deprecations related to the error handler #1037

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

## Unreleased

- [BC BREAK] Remove the deprecated code that made the `Hub` class a singleton
- [BC BREAK] Remove the deprecated code that made the `Hub` class a singleton (#1038)
- [BC BREAK] Remove deprecated code that permitted to register the error, fatal error and exception handlers at once (#1037)
- [BC BREAK] Change the default value for the `error_types` option from `E_ALL` to the value get from `error_reporting()` (#1037)

### 2.4.1 (2020-07-03)

Expand Down
8 changes: 8 additions & 0 deletions UPGRADE-3.0.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
# Upgrade 2.x to 3.0

- Removed the `HubInterface::getCurrentHub()` and `HubInterface::setCurrentHub()` methods. Use `SentrySdk::getCurrentHub()` and `SentrySdk::setCurrentHub()` instead.
- Removed the `ErrorHandler::registerOnce()` method, use `ErrorHandler::register*Handler()` instead.
- Removed the `ErrorHandler::addErrorListener` method, use `ErrorHandler::addErrorHandlerListener()` instead.
- Removed the `ErrorHandler::addFatalErrorListener` method, use `ErrorHandler::addFatalErrorHandlerListener()` instead.
- Removed the `ErrorHandler::addExceptionListener` method, use `ErrorHandler::addExceptionHandlerListener()` instead.
- The signature of the `ErrorListenerIntegration::__construct()` method changed to not accept any parameter
- The signature of the `FatalErrorListenerIntegration::__construct()` method changed to not accept any parameter
- The `ErrorListenerIntegration` integration does not get called anymore when a fatal error occurs
- The default value of the `error_types` option changed to the value get from `error_reporting()`
99 changes: 0 additions & 99 deletions src/ErrorHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,38 +120,6 @@ private function __construct()
$this->exceptionReflection->setAccessible(true);
}

/**
* 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
*
* @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);
}

if (null === self::$handlerInstance) {
self::$handlerInstance = new self();
}

self::registerOnceErrorHandler();
self::registerOnceFatalErrorHandler($reservedMemorySize);
self::registerOnceExceptionHandler();

return self::$handlerInstance;
}

/**
* Registers the error handler once and returns its instance.
*/
Expand Down Expand Up @@ -234,72 +202,6 @@ public static function registerOnceExceptionHandler(): self
return self::$handlerInstance;
}

/**
* Adds a listener to the current error handler to be called upon each
* invoked captured 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
*
* @psalm-param callable(\ErrorException): void $listener
*
* @deprecated since version 2.1, to be removed in 3.0
*/
public static function addErrorListener(callable $listener): void
{
@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);

/** @psalm-suppress DeprecatedMethod */
$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
*
* @psalm-param callable(FatalErrorException): void $listener
*
* @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);

/** @psalm-suppress DeprecatedMethod */
$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
* will instantiate and register it.
*
* @param callable $listener A callable that will act as a listener;
* this callable will receive a single
* \Throwable argument
*
* @psalm-param callable(\Throwable): void $listener
*
* @deprecated since version 2.1, to be removed in 3.0
*/
public static function addExceptionListener(callable $listener): void
{
@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);

/** @psalm-suppress DeprecatedMethod */
$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.
Expand Down Expand Up @@ -403,7 +305,6 @@ private function handleFatalError(): void

$this->exceptionReflection->setValue($errorAsException, []);

$this->invokeListeners($this->errorListeners, $errorAsException);
$this->invokeListeners($this->fatalErrorListeners, $errorAsException);
}
}
Expand Down
47 changes: 4 additions & 43 deletions src/Integration/ErrorListenerIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
namespace Sentry\Integration;

use Sentry\ErrorHandler;
use Sentry\Exception\FatalErrorException;
use Sentry\Exception\SilencedErrorException;
use Sentry\Options;
use Sentry\SentrySdk;

/**
Expand All @@ -16,48 +14,13 @@
*/
final class ErrorListenerIntegration implements IntegrationInterface
{
/**
* @var Options|null The options, to know which error level to use
*/
private $options;

/**
* @var bool Whether to handle fatal errors or not
*/
private $handleFatalErrors;

/**
* Constructor.
*
* @param Options|null $options The options to be used with this integration
* @param bool $handleFatalErrors Whether to handle fatal errors or not
*/
public function __construct(?Options $options = null, bool $handleFatalErrors = true)
{
if (null !== $options) {
@trigger_error(sprintf('Passing the options as argument of the constructor of the "%s" class is deprecated since version 2.1 and will not work in 3.0.', self::class), E_USER_DEPRECATED);
}

if ($handleFatalErrors) {
@trigger_error(sprintf('Handling fatal errors with the "%s" class is deprecated since version 2.1. Use the "%s" integration instead.', self::class, FatalErrorListenerIntegration::class), E_USER_DEPRECATED);
}

$this->options = $options;
$this->handleFatalErrors = $handleFatalErrors;
}

/**
* {@inheritdoc}
*/
public function setupOnce(): void
{
/** @psalm-suppress DeprecatedMethod */
$errorHandler = ErrorHandler::registerOnce(ErrorHandler::DEFAULT_RESERVED_MEMORY_SIZE, false);
$errorHandler->addErrorHandlerListener(function (\ErrorException $exception): void {
if (!$this->handleFatalErrors && $exception instanceof FatalErrorException) {
return;
}

$errorHandler = ErrorHandler::registerOnceErrorHandler();
$errorHandler->addErrorHandlerListener(static function (\ErrorException $exception): void {
$currentHub = SentrySdk::getCurrentHub();
$integration = $currentHub->getIntegration(self::class);
$client = $currentHub->getClient();
Expand All @@ -68,13 +31,11 @@ public function setupOnce(): void
return;
}

$options = $this->options ?? $client->getOptions();

if ($exception instanceof SilencedErrorException && !$options->shouldCaptureSilencedErrors()) {
if ($exception instanceof SilencedErrorException && !$client->getOptions()->shouldCaptureSilencedErrors()) {
return;
}

if (!($options->getErrorTypes() & $exception->getSeverity())) {
if (!($client->getOptions()->getErrorTypes() & $exception->getSeverity())) {
return;
}

Expand Down
3 changes: 1 addition & 2 deletions src/Integration/ExceptionListenerIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ final class ExceptionListenerIntegration implements IntegrationInterface
*/
public function setupOnce(): void
{
/** @psalm-suppress DeprecatedMethod */
$errorHandler = ErrorHandler::registerOnce(ErrorHandler::DEFAULT_RESERVED_MEMORY_SIZE, false);
$errorHandler = ErrorHandler::registerOnceExceptionHandler();
$errorHandler->addExceptionHandlerListener(static function (\Throwable $exception): void {
$currentHub = SentrySdk::getCurrentHub();
$integration = $currentHub->getIntegration(self::class);
Expand Down
26 changes: 2 additions & 24 deletions src/Integration/FatalErrorListenerIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

use Sentry\ErrorHandler;
use Sentry\Exception\FatalErrorException;
use Sentry\Options;
use Sentry\SentrySdk;

/**
Expand All @@ -16,32 +15,13 @@
*/
final class FatalErrorListenerIntegration implements IntegrationInterface
{
/**
* @var Options|null The options, to know which error level to use
*/
private $options;

/**
* Constructor.
*
* @param Options|null $options The options to be used with this integration
*/
public function __construct(?Options $options = null)
{
if (null !== $options) {
@trigger_error(sprintf('Passing the options as argument of the constructor of the "%s" class is deprecated since version 2.1 and will not work in 3.0.', self::class), E_USER_DEPRECATED);
}

$this->options = $options;
}

/**
* {@inheritdoc}
*/
public function setupOnce(): void
{
$errorHandler = ErrorHandler::registerOnceFatalErrorHandler();
$errorHandler->addFatalErrorHandlerListener(function (FatalErrorException $exception): void {
$errorHandler->addFatalErrorHandlerListener(static function (FatalErrorException $exception): void {
$currentHub = SentrySdk::getCurrentHub();
$integration = $currentHub->getIntegration(self::class);
$client = $currentHub->getClient();
Expand All @@ -52,9 +32,7 @@ public function setupOnce(): void
return;
}

$options = $this->options ?? $client->getOptions();

if (!($options->getErrorTypes() & $exception->getSeverity())) {
if (!($client->getOptions()->getErrorTypes() & $exception->getSeverity())) {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/Options.php
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ private function configureOptions(OptionsResolver $resolver): void
return $event;
},
'tags' => [],
'error_types' => E_ALL,
'error_types' => error_reporting(),
ste93cry marked this conversation as resolved.
Show resolved Hide resolved
'max_breadcrumbs' => self::DEFAULT_MAX_BREADCRUMBS,
'before_breadcrumb' => static function (Breadcrumb $breadcrumb): Breadcrumb {
return $breadcrumb;
Expand Down Expand Up @@ -1086,7 +1086,7 @@ private function getDefaultIntegrations(): array
if (null === $this->defaultIntegrations) {
$this->defaultIntegrations = [
new ExceptionListenerIntegration(),
new ErrorListenerIntegration(null, false),
new ErrorListenerIntegration(),
new FatalErrorListenerIntegration(),
new RequestIntegration(),
new TransactionIntegration(),
Expand Down
5 changes: 4 additions & 1 deletion tests/phpt/error_handler_can_be_registered_once.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,14 @@ function getHandlerRegistrationCount(callable $setHandlerCallback, callable $res
return count($savedErrorHandlers);
}

var_dump(ErrorHandler::registerOnce(10240, false) === ErrorHandler::registerOnce(10240, false));
var_dump(ErrorHandler::registerOnceErrorHandler() === ErrorHandler::registerOnceErrorHandler());
var_dump(1 === getHandlerRegistrationCount('set_error_handler', 'restore_error_handler'));

var_dump(ErrorHandler::registerOnceExceptionHandler() === ErrorHandler::registerOnceExceptionHandler());
var_dump(1 === getHandlerRegistrationCount('set_exception_handler', 'restore_exception_handler'));
?>
--EXPECT--
bool(true)
bool(true)
bool(true)
bool(true)
3 changes: 1 addition & 2 deletions tests/phpt/error_handler_captures_fatal_error.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ SentrySdk::getCurrentHub()->bindClient($client);

$errorHandler = ErrorHandler::registerOnceErrorHandler();
$errorHandler->addErrorHandlerListener(static function (): void {
echo 'Error listener called' . PHP_EOL;
echo 'Error listener called (it should not have been)' . PHP_EOL;
});

$errorHandler = ErrorHandler::registerOnceFatalErrorHandler();
Expand All @@ -64,6 +64,5 @@ class TestClass implements \Serializable
?>
--EXPECTF--
Fatal error: Class Sentry\Tests\TestClass contains 2 abstract methods and must therefore be declared abstract or implement the remaining methods (Serializable::serialize, Serializable::unserialize) in %s on line %d
Error listener called
Transport called
Fatal error listener called
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ require $vendor . '/vendor/autoload.php';

$errorHandler = ErrorHandler::registerOnceErrorHandler();
$errorHandler->addErrorHandlerListener(static function (): void {
echo 'Error listener called' . PHP_EOL;
echo 'Error listener called (it should not have been)' . PHP_EOL;
});

$errorHandler = ErrorHandler::registerOnceFatalErrorHandler();
Expand All @@ -31,12 +31,11 @@ $errorHandler->addFatalErrorHandlerListener(static function (): void {

$errorHandler = ErrorHandler::registerOnceExceptionHandler();
$errorHandler->addExceptionHandlerListener(static function (): void {
echo 'Exception listener called' . PHP_EOL;
echo 'Exception listener called (it should not have been)' . PHP_EOL;
});

$foo = str_repeat('x', 1024 * 1024 * 30);
?>
--EXPECTF--
Fatal error: Allowed memory size of %d bytes exhausted (tried to allocate %d bytes) in %s on line %d
Error listener called
Fatal error listener called

This file was deleted.