Disable sentry for tests

[stedekay]

I've got the following problem:
When I run PHPUnit on my symfony application it always exits at the same test without any exception or hint. When I run PHPUnit just for this one test where it stops everything works fine, but I get the following message:

THE ERROR HANDLER HAS CHANGED!

When I disable sentry completely then everything works fine and at the end I get some deprecation warnings. When I remove those deprecations and enable sentry again everything works.
So it seems that it exits when it finds some deprecated calls.

So my question is, how can I disable sentry for the tests or is there another way to fix this issue?

[Addvilz]

See #39

[stedekay]

Oops, sorry. I was sure checking for an existing issue for that.

But I think I don't get it right. This means that I should override the sentry config in the config_test.yml, right?

Something like:

sentry:
    dsn: ""

But I get the same notification. What am I doing wrong?

[Addvilz]

Hmm, perhaps it's related to the fact that you are loading the sentry bundle in tests at all? It might be PHP Sentry is doing something with the error handling even when not configured. Try only loading the bundle in dev and prod environments, not tests.

[dcramer]

This is correct -- Sentry is loaded for tests even when DSN is disabled. This is semi-important as you want to test behavior that exists in prod, and without Sentry's instrumentation you may possibly be creating issues that wouldn't be exposed in the test environment (e.g. if our behavior doesnt play well with your app).

I think we could do something more around documentation here, and provide suggestions for how to completely disable it (with a warning).

[stedekay]

Okay, so I tried to load the config in config_prod.yml only. But it still seems to be loaded in the tests.
Next thing was adding the following lines into AppKernel.php

if (in_array($this->getEnvironment(), ['prod', 'dev'])) {
    $bundles[] = new Sentry\SentryBundle\SentryBundle();
}

With this addition the tests seem to work properly. So I'm closing this issue as it works for me.

[gregholland]

I still think this is an issue, @sentry.client is a dependency in one of my services so I can manually call captureMessage(). If I disable the bundle in the test environment, that code won't work.
Kind of scary that my tests have been incorrectly 'passing' since I've started using this bundle

[stedekay]

My tests also passed since I updated my whole infrastructure (from PHP 5.6 to 7.1, MySQL 5.5 to 5.7 and updated a lot of dependencies in my composer.json). So I can't tell you what exactly made the difference.

[skobkin]

@stedekay So what's about config.yml sentry section?

If you run Symfony console when bundle is disabled you'll get:

There is no extension able to load the configuration for "sentry" (in app/config/config.yml)

[stedekay]

I think you have to put the content into the prod config. Not sure about its name since I use the new directory structure.

[skobkin]

Yeah. I think it may work. Forgot about it. Thanks.

[stanislav-reshetnev]

Now (Symfony 4) You can choose needed environment in config/bundles.php:

Sentry\SentryBundle\SentryBundle::class => ['prod' => true],

[Jean85]

That was possible with a simple if in AppKernel even before.

In the new 3.0 release of this bundle we avoided that though, since I preferred to enable the bundle everywhere, but disable the client by not setting the DSN where not needed. This is better because it will not break your app if you want to inject any Sentry services in your services.

[megawilddaddy]

Hi, we wanted to disable error collecting/logging for test environment, but to leave the bundle, because we inject the sentry logger in some services.

This is the solution that worked for us:
PS. I dont remember why just setting the parameters didnt work for us.

You need to make services_test.yaml and override the sentry client:

services:
  sentry.client:
    public: true
    class: AppBundle\Services\SentryTestClient
    arguments: ['%sentry.dsn%', '%sentry.options%']

and then create the class that will disable the handlers in constructor:

class SentryTestClient extends SentrySymfonyClient
{
    /**
     * SentryClient constructor.
     * @param string|null $dsn
     * @param array $options
     * @param string $env
     */
    public function __construct(?string $dsn = null, array $options = [])
    {
        $options['install_default_breadcrumb_handlers'] = false;
        $options['install_shutdown_handler'] = false;
        parent::__construct($dsn, $options);
    }
}

[Jean85]

@megawilddaddy thanks this is similar to what I did previously. But that's the old version of the bundle, the new one is automatically disabled using a null DSN.

[VincentLanglet]

@Jean85 If I add a test/sentry.yaml file with

sentry:
    dsn: null

I still get THE ERROR HANDLER HAS CHANGED!

Either I'm doing something wrong, either it doesn't disable automatically sentry.

[Jean85]

Removing the DSN only disables the sending of events, not the whole bundle.

This is a contentious issue, that I was already discussing in symfony/recipes-contrib#1080: my opinion is that disabling the bundle completely would cause more harm because you wouldn't be testing the codepaths triggered by Sentry. The final solution for this would be #337 (switching from hooking with an error handler to Monolog) but it's a pretty intensive rewrite.

[VincentLanglet]

Until #337, wouldn't be possible to disable the override of the error handler if the DSN is null ?
If you can't send the error to sentry, I'm not sure there is a need to catch them.

[Jean85]

That would be a huge breaking change for end users, that could break tests if someone relies on Sentry services in their code.

You can shut down the whole bundle on your test environment with the same end result though.

[VincentLanglet]

That would be a huge breaking change for end users, that could break tests if someone relies on Sentry services in their code.
You can shut down the whole bundle on your test environment with the same end result though.

Not sure to understand.

If I rely on Sentry service in my code, I need the bundle ; so I can't shutdown it on my test environment.
But if I provide a null DSN, Sentry cannot send the log/error to the sentry API. So it's useless to override the error handler in order to send them.

Wouldn't be possible, to keeping all the Sentry classes/services, but not overriding the error handler.
I don't know how this bundle works. I was thinking that something like

set_error_handler($sentryErrorHandler)

was done. So I just wanted

if (DSN !== null) {
    set_error_handler($sentryErrorHandler);
}

without changing anything else.

When I listen to you, it seems like it doesn't work this way.

[Jean85]

That piece of code doesn't even live in the bundle but it's inside sentry/sentry, so it would have an even larger impact. I don't know if that would have no impact on tests, as error handling is a pretty messy topic.

cc @ste93cry WDYT?

[ste93cry]

Registering the error handler is done inside certain integrations, so it should be possible to change the way the client is built during the container compilation so that if the DSN is null then we skip adding those integrations to the client. However, this won't work if the DSN is set using an environment variable: in that case its value is known only at runtime, so after the service container has already been built, and once set on the client the integrations cannot be changed. We could clearly ignore this last drawback, but having something working only under certain conditions is more than suboptimal. Any other solution will have to involve development on the main SDK, though

If I rely on Sentry service in my code, I need the bundle ; so I can't shutdown it on my test environment.

Correct. If you just use the global functions or the SentrySdk singleton class then you should be fine even if the bundle is disabled, but of course if you instead choose to use (as it should be) the dependency injction pattern and you inject the hub into your services then you cannot stop enabling the bundle for a certain environment or the application will break

[VincentLanglet]

Registering the error handler is done inside certain integrations, so it should be possible to change the way the client is built during the container compilation so that if the DSN is null then we skip adding those integrations to the client. However, this won't work if the DSN is set using an environment variable: in that case its value is known only at runtime, so after the service container has already been built, and once set on the client the integrations cannot be changed. We could clearly ignore this last drawback, but having something working only under certain conditions is more than suboptimal. Any other solution will have to involve development on the main SDK, though

So

sentry:
    dsn: null

will disable the error handler but

sentry:
    dsn: '%env(SENTRY_DSN)%'

with SENTRY_DSN= won't.

This would still be a nice solution. And a flex recipe could be done in order to use sentry.dsn: null by default in the tests.
With a default flex recipe, and maybe some documentation ; this should be ok.

Do you prefer an option disable ?
Like the swiftmailer https://symfony.com/doc/4.1/email/dev_environment.html#disabling-sending

[Jean85]

symfony/recipes-contrib#1080 was exactly proposing to change the recipe, that's what I was rejecting. It's too much of a default and at risk of impacting unsuspecting people.

[VincentLanglet]

symfony/recipes-contrib#1080 was exactly proposing to change the recipe, that's what I was rejecting. It's too much of a default and at risk of impacting unsuspecting people.

My proposal is way different.

I don't propose to only use the sentry bundle in production.
I propose to use the sentry bundle in every environment but, to add a file, only in test,

sentry:
    dsn: null

or

sentry:
    disable_error_handler: true

if the functionality is implemented.

Anyway, that's another topic.
A way to disabled the error_handler with a dsn null a config option would be nice and seems doable according to @ste93cry