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

Support sentry/sentry 2.3 and fix deprecations #298

Merged
merged 18 commits into from Jan 17, 2020
Merged
Show file tree
Hide file tree
Changes from 16 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
5 changes: 4 additions & 1 deletion CHANGELOG.md
Expand Up @@ -5,7 +5,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## Unreleased
- ...
- Add support for `sentry/sentry` 2.3 (#298)
- Drop support for `sentry/sentry` < 2.3 (#298)
- Add support to `in_app_include` client option (#298)
- Remap `excluded_exception` option to use the new `IgnoreErrorIntegration` (#298)

## 3.3.2 (2020-01-16)
- Fix issue with exception listener under Symfony 4.3 (#301)
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Expand Up @@ -22,7 +22,7 @@
"php": "^7.1",
"jean85/pretty-package-versions": "^1.0",
"ocramius/package-versions": "^1.3.0",
"sentry/sdk": "^2.0",
"sentry/sdk": "^2.1",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This locks against sentry/sentry:^2.3. Because of this, this should be released as 3.4, to leave space for 3.3.x patches.

"symfony/config": "^3.4||^4.0||^5.0",
"symfony/console": "^3.4||^4.0||^5.0",
"symfony/dependency-injection": "^3.4||^4.0||^5.0",
Expand Down
2 changes: 0 additions & 2 deletions phpstan.neon
Expand Up @@ -6,8 +6,6 @@ parameters:
ignoreErrors:
- "/Call to function method_exists.. with 'Symfony.+' and 'getRootNode' will always evaluate to false./"
- "/Call to function method_exists.. with 'Symfony.+' and 'getThrowable' will always evaluate to false./"
- "/Call to function method_exists.. with 'Sentry..Options' and 'getClassSerializers' will always evaluate to false./"
- "/Call to function method_exists.. with 'Sentry..Options' and 'getMaxRequestBodySi...' will always evaluate to false./"
- '/Class PHPUnit_Framework_TestCase not found/'
- '/Symfony\\Component\\HttpKernel\\Event\\(GetResponse|FilterController)Event not found.$/'
-
Expand Down
1 change: 0 additions & 1 deletion phpunit.xml
Expand Up @@ -6,7 +6,6 @@
backupGlobals="false"
bootstrap="vendor/autoload.php"
cacheResult="false"
processIsolation="true"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've used an annotation directly on the only test that needs it, the End2EndTest, so now the test suite is faster.

>
<php>
<env name="SYMFONY_DEPRECATIONS_HELPER" value="weak" />
Expand Down
4 changes: 2 additions & 2 deletions src/Command/SentryTestCommand.php
Expand Up @@ -2,7 +2,7 @@

namespace Sentry\SentryBundle\Command;

use Sentry\SentryBundle\SentryBundle;
use Sentry\SentrySdk;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
Expand All @@ -16,7 +16,7 @@ public function __construct()

protected function execute(InputInterface $input, OutputInterface $output): int
{
$currentHub = SentryBundle::getCurrentHub();
$currentHub = SentrySdk::getCurrentHub();
$client = $currentHub->getClient();

if (! $client) {
Expand Down
21 changes: 0 additions & 21 deletions src/DependencyInjection/ClientBuilderConfigurator.php
Expand Up @@ -3,9 +3,6 @@
namespace Sentry\SentryBundle\DependencyInjection;

use Sentry\ClientBuilderInterface;
use Sentry\Integration\ErrorListenerIntegration;
use Sentry\Integration\ExceptionListenerIntegration;
use Sentry\Integration\IntegrationInterface;
use Sentry\SentryBundle\SentryBundle;

class ClientBuilderConfigurator
Expand All @@ -14,23 +11,5 @@ public static function configure(ClientBuilderInterface $clientBuilder): void
{
$clientBuilder->setSdkIdentifier(SentryBundle::SDK_IDENTIFIER);
$clientBuilder->setSdkVersion(SentryBundle::getSdkVersion());

$options = $clientBuilder->getOptions();
if (! $options->hasDefaultIntegrations()) {
return;
}

$integrations = $options->getIntegrations();
$options->setIntegrations(array_filter($integrations, static function (IntegrationInterface $integration): bool {
if ($integration instanceof ErrorListenerIntegration) {
return false;
}

if ($integration instanceof ExceptionListenerIntegration) {
return false;
}

return true;
}));
}
}
49 changes: 18 additions & 31 deletions src/DependencyInjection/Configuration.php
Expand Up @@ -2,7 +2,6 @@

namespace Sentry\SentryBundle\DependencyInjection;

use Jean85\PrettyVersions;
use PackageVersions\Versions;
use Sentry\Options;
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
Expand Down Expand Up @@ -58,14 +57,10 @@ public function getConfigTreeBuilder(): TreeBuilder
->validate()
->ifTrue($this->isNotAValidCallback())
->thenInvalid('Expecting callable or service reference, got %s');
if (PrettyVersions::getVersion('sentry/sentry')->getPrettyVersion() !== '2.0.0') {
$optionsChildNodes->booleanNode('capture_silenced_errors');
}
if ($this->classSerializersAreSupported()) {
$optionsChildNodes->arrayNode('class_serializers')
->defaultValue([])
->prototype('scalar');
}
Comment on lines -61 to -68
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here (and below) I've removed all the conditions that were needed for backward compatibility with sentry/sentry 2.0, it's no longer needed.

$optionsChildNodes->booleanNode('capture_silenced_errors');
$optionsChildNodes->arrayNode('class_serializers')
->defaultValue([])
->prototype('scalar');
$optionsChildNodes->integerNode('context_lines')
->min(0)
->max(99);
Expand All @@ -75,14 +70,19 @@ public function getConfigTreeBuilder(): TreeBuilder
->defaultValue('%kernel.environment%')
->cannotBeEmpty();
$optionsChildNodes->scalarNode('error_types');
$optionsChildNodes->arrayNode('in_app_include')
->defaultValue([
'%kernel.project_dir%/src',
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I'm trying to guess the source folder, since using the parent is not feasibile due to getsentry/sentry-php#953

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jean85 With the change in sentry-php 2.3.1 all files are now included by default. Therefore, wouldn't an empty default value make more sense?

With the current default value '%kernel.project_dir%/src', it is impossible to exclude files inside '%kernel.project_dir%/src', since in_app_include overrides in_app_exclude. With an empty array as default value, this would still be possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true, but it shouldn't have any practical effect you since you can override this default.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still might want to remove it since it takes precedence over in_app_exclude which you need to know / remember to clear out the in_app_include if you want to exclude anything that is in the src dir.

If you keep it empty by default you can just start using the in_app_exclude without also needing to clear the in_app_include default.

It's not critical ofcourse but it is easier to not have it set and possibly make a in_app_exclude value not work because of it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that's true! Done in #311

])
->prototype('scalar');
$optionsChildNodes->arrayNode('in_app_exclude')
->defaultValue([
'%kernel.cache_dir%',
'%kernel.project_dir%/vendor',
])
->prototype('scalar');
$optionsChildNodes->arrayNode('excluded_exceptions')
->defaultValue($defaultValues->getExcludedExceptions())
->defaultValue([])
->prototype('scalar');
$optionsChildNodes->scalarNode('http_proxy');
$optionsChildNodes->arrayNode('integrations')
Expand All @@ -97,24 +97,21 @@ public function getConfigTreeBuilder(): TreeBuilder
})
->thenInvalid('Expecting service reference, got "%s"');
$optionsChildNodes->scalarNode('logger');
if ($this->maxRequestBodySizeIsSupported()) {
$optionsChildNodes->enumNode('max_request_body_size')
->values([
'none',
'small',
'medium',
'always',
]);
}
$optionsChildNodes->enumNode('max_request_body_size')
->values([
'none',
'small',
'medium',
'always',
]);
$optionsChildNodes->integerNode('max_breadcrumbs')
->min(1);
$optionsChildNodes->integerNode('max_value_length')
->min(1);
$optionsChildNodes->arrayNode('prefixes')
->defaultValue($defaultValues->getPrefixes())
->prototype('scalar');
$optionsChildNodes->scalarNode('project_root')
->defaultValue('%kernel.project_dir%');
$optionsChildNodes->scalarNode('project_root');
$optionsChildNodes->scalarNode('release')
->defaultValue(Versions::getVersion(Versions::ROOT_PACKAGE_NAME))
->info('Release version to be reported to sentry, see https://docs.sentry.io/workflow/releases/?platform=php')
Expand Down Expand Up @@ -193,14 +190,4 @@ private function isNotAValidCallback(): \Closure
return true;
};
}

private function classSerializersAreSupported(): bool
{
return method_exists(Options::class, 'getClassSerializers');
}

private function maxRequestBodySizeIsSupported(): bool
{
return method_exists(Options::class, 'getMaxRequestBodySize');
}
}
32 changes: 32 additions & 0 deletions src/DependencyInjection/IntegrationFilterFactory.php
@@ -0,0 +1,32 @@
<?php

namespace Sentry\SentryBundle\DependencyInjection;

use Sentry\Integration\ErrorListenerIntegration;
use Sentry\Integration\ExceptionListenerIntegration;
use Sentry\Integration\IntegrationInterface;

class IntegrationFilterFactory
{
public static function create(array $integrationsFromConfiguration): callable
{
return function (array $integrations) use ($integrationsFromConfiguration) {
$allIntegrations = array_merge($integrations, $integrationsFromConfiguration);

return array_filter(
$allIntegrations,
static function (IntegrationInterface $integration): bool {
if ($integration instanceof ErrorListenerIntegration) {
return false;
}

if ($integration instanceof ExceptionListenerIntegration) {
return false;
}

return true;
}
);
};
}
}
22 changes: 19 additions & 3 deletions src/DependencyInjection/SentryExtension.php
Expand Up @@ -4,13 +4,15 @@

use Monolog\Logger as MonologLogger;
use Sentry\ClientBuilderInterface;
use Sentry\Integration\IgnoreErrorsIntegration;
use Sentry\Monolog\Handler;
use Sentry\Options;
use Sentry\SentryBundle\ErrorTypesParser;
use Sentry\SentryBundle\EventListener\ErrorListener;
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Exception\LogicException;
use Symfony\Component\DependencyInjection\Loader;
use Symfony\Component\DependencyInjection\Reference;
Expand Down Expand Up @@ -63,7 +65,6 @@ private function passConfigurationToOptions(ContainerBuilder $container, array $
'default_integrations',
'enable_compression',
'environment',
'excluded_exceptions',
'http_proxy',
'logger',
'max_request_body_size',
Expand All @@ -90,6 +91,10 @@ private function passConfigurationToOptions(ContainerBuilder $container, array $
$options->addMethodCall('setInAppExcludedPaths', [$processedOptions['in_app_exclude']]);
}

if (\array_key_exists('in_app_include', $processedOptions)) {
$options->addMethodCall('setInAppIncludedPaths', [$processedOptions['in_app_include']]);
}

if (\array_key_exists('error_types', $processedOptions)) {
$parsedValue = (new ErrorTypesParser($processedOptions['error_types']))->parse();
$options->addMethodCall('setErrorTypes', [$parsedValue]);
Expand All @@ -114,14 +119,25 @@ private function passConfigurationToOptions(ContainerBuilder $container, array $
$options->addMethodCall('setClassSerializers', [$classSerializers]);
}

$integrations = [];
if (\array_key_exists('integrations', $processedOptions)) {
$integrations = [];
foreach ($processedOptions['integrations'] as $integrationName) {
$integrations[] = new Reference(substr($integrationName, 1));
}
}

$options->addMethodCall('setIntegrations', [$integrations]);
if (\array_key_exists('excluded_exceptions', $processedOptions) && $processedOptions['excluded_exceptions']) {
$ignoreOptions = [
'ignore_exceptions' => $processedOptions['excluded_exceptions'],
];

$integrations[] = new Definition(IgnoreErrorsIntegration::class, [$ignoreOptions]);
}

$integrationsCallable = new Definition('callable', [$integrations]);
$integrationsCallable->setFactory([IntegrationFilterFactory::class, 'create']);

$options->addMethodCall('setIntegrations', [$integrationsCallable]);
}

private function valueToCallable($value)
Expand Down
4 changes: 2 additions & 2 deletions src/EventListener/ConsoleListener.php
Expand Up @@ -2,7 +2,7 @@

namespace Sentry\SentryBundle\EventListener;

use Sentry\SentryBundle\SentryBundle;
use Sentry\SentrySdk;
use Sentry\State\HubInterface;
use Sentry\State\Scope;
use Symfony\Component\Console\Event\ConsoleCommandEvent;
Expand Down Expand Up @@ -38,7 +38,7 @@ public function onConsoleCommand(ConsoleCommandEvent $event): void
$commandName = $command->getName();
}

SentryBundle::getCurrentHub()
SentrySdk::getCurrentHub()
->configureScope(static function (Scope $scope) use ($commandName): void {
$scope->setTag('command', $commandName ?? 'N/A');
});
Expand Down
10 changes: 5 additions & 5 deletions src/EventListener/RequestListener.php
Expand Up @@ -2,7 +2,7 @@

namespace Sentry\SentryBundle\EventListener;

use Sentry\SentryBundle\SentryBundle;
use Sentry\SentrySdk;
use Sentry\State\HubInterface;
use Sentry\State\Scope;
use Symfony\Component\HttpKernel\Event\ControllerEvent;
Expand Down Expand Up @@ -56,7 +56,7 @@ public function onKernelRequest(RequestEvent $event): void
return;
}

$currentClient = SentryBundle::getCurrentHub()->getClient();
$currentClient = SentrySdk::getCurrentHub()->getClient();
if (null === $currentClient || ! $currentClient->getOptions()->shouldSendDefaultPii()) {
return;
}
Expand All @@ -79,9 +79,9 @@ public function onKernelRequest(RequestEvent $event): void

$userData['ip_address'] = $event->getRequest()->getClientIp();

SentryBundle::getCurrentHub()
SentrySdk::getCurrentHub()
->configureScope(function (Scope $scope) use ($userData): void {
$scope->setUser($userData);
$scope->setUser($userData, true);
});
}

Expand All @@ -97,7 +97,7 @@ public function onKernelController(ControllerEvent $event): void

$matchedRoute = (string) $event->getRequest()->attributes->get('_route');

SentryBundle::getCurrentHub()
SentrySdk::getCurrentHub()
->configureScope(function (Scope $scope) use ($matchedRoute): void {
$scope->setTag('route', $matchedRoute);
});
Expand Down
6 changes: 3 additions & 3 deletions src/EventListener/SubRequestListener.php
Expand Up @@ -2,7 +2,7 @@

namespace Sentry\SentryBundle\EventListener;

use Sentry\SentryBundle\SentryBundle;
use Sentry\SentrySdk;
use Symfony\Component\HttpKernel\Event\FinishRequestEvent;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\Event\RequestEvent;
Expand All @@ -24,7 +24,7 @@ public function onKernelRequest(RequestEvent $event): void
return;
}

SentryBundle::getCurrentHub()->pushScope();
SentrySdk::getCurrentHub()->pushScope();
}

/**
Expand All @@ -38,6 +38,6 @@ public function onKernelFinishRequest(FinishRequestEvent $event): void
return;
}

SentryBundle::getCurrentHub()->popScope();
SentrySdk::getCurrentHub()->popScope();
}
}
15 changes: 2 additions & 13 deletions src/SentryBundle.php
Expand Up @@ -4,7 +4,6 @@

use Jean85\PrettyVersions;
use Sentry\SentrySdk;
use Sentry\State\Hub;
use Sentry\State\HubInterface;
use Symfony\Component\HttpKernel\Bundle\Bundle;

Expand All @@ -23,24 +22,14 @@ public static function getSdkVersion(): string
*/
public static function getCurrentHub(): HubInterface
{
if (class_exists(SentrySdk::class)) {
return SentrySdk::getCurrentHub();
}

return Hub::getCurrent();
return SentrySdk::getCurrentHub();
}

/**
* This method avoids deprecations with sentry/sentry:^2.2
*/
public static function setCurrentHub(HubInterface $hub): void
{
if (class_exists(SentrySdk::class)) {
SentrySdk::setCurrentHub($hub);

return;
}

Hub::setCurrent($hub);
SentrySdk::setCurrentHub($hub);
}
Comment on lines 23 to 34
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those two methods are left here for BC.

}