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.

Sign up for GitHub

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

Jump to bottom

Add fast path for ignoring errors #1737

Merged
merged 2 commits into from
Jun 4, 2024
Merged

Add fast path for ignoring errors #1737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
71a0154
Add fast path for ignoring errors
stayallive May 3, 2024
78e822e
Merge branch 'master' into optimize-error-handler
stayallive Jun 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 32 additions & 9 deletions src/ErrorHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ final class ErrorHandler
*/
private $memoryLimitIncreaseOnOutOfMemoryErrorValue = 5 * 1024 * 1024; // 5 MiB

/**
* @var Options|null The SDK options
*/
private $options;

/**
* @var bool Whether the memory limit has been increased
*/
Expand Down Expand Up @@ -153,12 +158,14 @@ private function __construct()
/**
* Registers the error handler once and returns its instance.
*/
public static function registerOnceErrorHandler(): self
public static function registerOnceErrorHandler(?Options $options = null): self
{
if (self::$handlerInstance === null) {
self::$handlerInstance = new self();
}

self::$handlerInstance->options = $options;

if (self::$handlerInstance->isErrorHandlerRegistered) {
return self::$handlerInstance;
}
Expand Down Expand Up @@ -326,17 +333,19 @@ private function handleError(int $level, string $message, string $file, int $lin
}
}

if ($isSilencedError) {
$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);
}
if ($this->shouldHandleError($level, $isSilencedError)) {
if ($isSilencedError) {
$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(), $errorAsException->getFile(), $errorAsException->getLine());
$backtrace = $this->cleanBacktraceFromErrorHandlerFrames($errorAsException->getTrace(), $errorAsException->getFile(), $errorAsException->getLine());

$this->exceptionReflection->setValue($errorAsException, $backtrace);
$this->exceptionReflection->setValue($errorAsException, $backtrace);

$this->invokeListeners($this->errorListeners, $errorAsException);
$this->invokeListeners($this->errorListeners, $errorAsException);
}

if ($this->previousErrorHandler !== null) {
return false !== ($this->previousErrorHandler)($level, $message, $file, $line, $errcontext);
Expand All @@ -345,6 +354,20 @@ private function handleError(int $level, string $message, string $file, int $lin
return false;
}

private function shouldHandleError(int $level, bool $silenced): bool
{
// If we were not given any options, we should handle all errors
if ($this->options === null) {
return true;
}

if ($silenced) {
return $this->options->shouldCaptureSilencedErrors();
}

return ($this->options->getErrorTypes() & $level) !== 0;
}

/**
* Tries to handle a fatal error if any and relay them to the listeners.
* It only tries to do this if we still have some reserved memory at
Expand Down
71 changes: 48 additions & 23 deletions src/Integration/ErrorListenerIntegration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,40 +6,65 @@

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

/**
* This integration hooks into the global error handlers and emits events to
* Sentry.
*/
final class ErrorListenerIntegration extends AbstractErrorListenerIntegration
final class ErrorListenerIntegration extends AbstractErrorListenerIntegration implements OptionAwareIntegrationInterface
{
/**
* @var Options
*/
private $options;

public function setOptions(Options $options): void
{
$this->options = $options;
}

/**
* {@inheritdoc}
*/
public function setupOnce(): void
{
$errorHandler = ErrorHandler::registerOnceErrorHandler();
$errorHandler->addErrorHandlerListener(static function (\ErrorException $exception): void {
$currentHub = SentrySdk::getCurrentHub();
$integration = $currentHub->getIntegration(self::class);
$client = $currentHub->getClient();

// The client bound to the current hub, if any, could not have this
// integration enabled. If this is the case, bail out
if ($integration === null || $client === null) {
return;
}

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

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

$integration->captureException($currentHub, $exception);
});
ErrorHandler::registerOnceErrorHandler($this->options)
->addErrorHandlerListener(
static function (\ErrorException $exception): void {
$currentHub = SentrySdk::getCurrentHub();
$integration = $currentHub->getIntegration(self::class);
$client = $currentHub->getClient();

// The client bound to the current hub, if any, could not have this
// integration enabled. If this is the case, bail out
if ($integration === null || $client === null) {
return;
}

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

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

$integration->captureException($currentHub, $exception);
}
);
}

/**
* @internal this is a convenience method to create an instance of this integration for tests
*/
public static function make(Options $options): self
{
$integration = new self();

$integration->setOptions($options);

return $integration;
}
}
8 changes: 6 additions & 2 deletions src/Integration/IntegrationRegistry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public function setupIntegrations(Options $options, LoggerInterface $logger): ar

$integrations[$integrationName] = $integration;

if ($this->setupIntegration($integration)) {
if ($this->setupIntegration($integration, $options)) {
$installed[] = $integrationName;
}
}
Expand All @@ -70,14 +70,18 @@ public function setupIntegrations(Options $options, LoggerInterface $logger): ar
return $integrations;
}

private function setupIntegration(IntegrationInterface $integration): bool
private function setupIntegration(IntegrationInterface $integration, Options $options): bool
{
$integrationName = \get_class($integration);

if (isset($this->integrations[$integrationName])) {
return false;
}

if ($integration instanceof OptionAwareIntegrationInterface) {
$integration->setOptions($options);
}

$integration->setupOnce();

$this->integrations[$integrationName] = true;
Expand Down
15 changes: 15 additions & 0 deletions src/Integration/OptionAwareIntegrationInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types=1);

namespace Sentry\Integration;

use Sentry\Options;

interface OptionAwareIntegrationInterface extends IntegrationInterface
{
/**
* Sets the options for the integration, is called before `setupOnce()`.
*/
public function setOptions(Options $options): void;
}
20 changes: 10 additions & 10 deletions tests/Integration/IntegrationRegistryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ public function setupOnce(): void
];

yield 'Default integrations and no user integrations' => [
new Options([
$options = new Options([
'dsn' => 'http://public@example.com/sentry/1',
'default_integrations' => true,
]),
[
ExceptionListenerIntegration::class => new ExceptionListenerIntegration(),
ErrorListenerIntegration::class => new ErrorListenerIntegration(),
ErrorListenerIntegration::class => ErrorListenerIntegration::make($options),
FatalErrorListenerIntegration::class => new FatalErrorListenerIntegration(),
RequestIntegration::class => new RequestIntegration(),
TransactionIntegration::class => new TransactionIntegration(),
Expand All @@ -104,7 +104,7 @@ public function setupOnce(): void
];

yield 'Default integrations and some user integrations' => [
new Options([
$options = new Options([
'dsn' => 'http://public@example.com/sentry/1',
'default_integrations' => true,
'integrations' => [
Expand All @@ -114,7 +114,7 @@ public function setupOnce(): void
]),
[
ExceptionListenerIntegration::class => new ExceptionListenerIntegration(),
ErrorListenerIntegration::class => new ErrorListenerIntegration(),
ErrorListenerIntegration::class => ErrorListenerIntegration::make($options),
FatalErrorListenerIntegration::class => new FatalErrorListenerIntegration(),
RequestIntegration::class => new RequestIntegration(),
TransactionIntegration::class => new TransactionIntegration(),
Expand All @@ -127,7 +127,7 @@ public function setupOnce(): void
];

yield 'Default integrations and some user integrations, one of which is also a default integration' => [
new Options([
$options = new Options([
'dsn' => 'http://public@example.com/sentry/1',
'default_integrations' => true,
'integrations' => [
Expand All @@ -137,7 +137,7 @@ public function setupOnce(): void
]),
[
ExceptionListenerIntegration::class => new ExceptionListenerIntegration(),
ErrorListenerIntegration::class => new ErrorListenerIntegration(),
ErrorListenerIntegration::class => ErrorListenerIntegration::make($options),
FatalErrorListenerIntegration::class => new FatalErrorListenerIntegration(),
RequestIntegration::class => new RequestIntegration(),
FrameContextifierIntegration::class => new FrameContextifierIntegration(),
Expand All @@ -149,7 +149,7 @@ public function setupOnce(): void
];

yield 'Default integrations and one user integration, the ModulesIntegration is also a default integration' => [
new Options([
$options = new Options([
'dsn' => 'http://public@example.com/sentry/1',
'default_integrations' => true,
'integrations' => [
Expand All @@ -158,7 +158,7 @@ public function setupOnce(): void
]),
[
ExceptionListenerIntegration::class => new ExceptionListenerIntegration(),
ErrorListenerIntegration::class => new ErrorListenerIntegration(),
ErrorListenerIntegration::class => ErrorListenerIntegration::make($options),
FatalErrorListenerIntegration::class => new FatalErrorListenerIntegration(),
RequestIntegration::class => new RequestIntegration(),
TransactionIntegration::class => new TransactionIntegration(),
Expand Down Expand Up @@ -192,7 +192,7 @@ public function setupOnce(): void
];

yield 'Default integrations and a callable as user integrations' => [
new Options([
$options = new Options([
'dsn' => 'http://public@example.com/sentry/1',
'default_integrations' => true,
'integrations' => static function (array $defaultIntegrations): array {
Expand All @@ -201,7 +201,7 @@ public function setupOnce(): void
]),
[
ExceptionListenerIntegration::class => new ExceptionListenerIntegration(),
ErrorListenerIntegration::class => new ErrorListenerIntegration(),
ErrorListenerIntegration::class => ErrorListenerIntegration::make($options),
FatalErrorListenerIntegration::class => new FatalErrorListenerIntegration(),
RequestIntegration::class => new RequestIntegration(),
TransactionIntegration::class => new TransactionIntegration(),
Expand Down
Loading