Include default configuration for Contao managed edition

[aschempp]

This PR adds some default configurations if the bundle is included in a Contao Managed Edition. Because most times, the configuration will be identical and repetitive.

I also added an event listener to filter previous exceptions. In my case, I had Twig components throwing an AccessDeniedException (which correctly triggers the firewall), which are logged as RuntimeError even though the previous exception was one to be filtered.

The namespace also changed from Oneup\Contao\SentryBundle to Oneup\ContaoSentryBundle which I think would be more correct in terms of Composer packages.

@fritzmg @richardhj any opinions on this?

TODO:

• Update README

[bytehead]

/cc @Shadow-Devil

[fritzmg]

I also have

Symfony\Component\Security\Core\Exception\AuthenticationException

in my ignore_exceptions list.

[fritzmg]

I usually only set sentry.dsn and sentry.options.ignore_exceptions in my projects. Why do you also set sentry.register_error_listener, sentry.register_error_handler and monolog.handlers.sentry? i.e. what's the purpose/advantage.

[Shadow-Devil]

Would this be a major version release since all PHP, Contao and Sentry minimum versions were increased?

[bytehead]

Would this be a major version release since all PHP, Contao and Sentry minimum versions were increased?

Yes.

[aschempp]

I usually only set sentry.dsn and sentry.options.ignore_exceptions in my projects. Why do you also set sentry.register_error_listener, sentry.register_error_handler and monolog.handlers.sentry? i.e. what's the purpose/advantage.

I was wondering that as well, but that's what the official docs say: https://docs.sentry.io/platforms/php/guides/symfony/#monolog-integration

If you have monolog …

I know that we do often get error messages twice in our projects (with the same setup you have). I can only assume that's because Monolog integration is logged to sentry as well as the error handler 🤔

[fritzmg]

Hm, I never get error messages twice - I only get multiple sentry entries when there are previous exceptions, which your PR fixes. imho the default config without configuring monolog is enough.

[aschempp]

🤔 symfony/symfony#36472 (comment)

I think the reason is that you can only have one error handler in PHP, and if you already have Symfony/Monolog handling the errors, you don't want Sentry to override or interefer with that.

symfony/symfony#36472 (comment)

Found in getsentry/sentry-symfony#337

[fritzmg]

Ah, I see 👍

[aschempp]

Another big advantage I just noticed: this now also logs all other monolog errors to Sentry! In my case the CSP violation logged by Contao was added to Sentry 😎

[fritzmg]

Which is something that should be ignored though imho 😁 CSP reports can be quite spammy (as anyone can send anything to the controller).

[aschempp]

Which is something that should be ignored though imho 😁 CSP reports can be quite spammy (as anyone can send anything to the controller).

well yeah… but Sentry also has specific CSP monitoring, which would make more sense than this. But not really related to this PR 😆

[aschempp]

@bytehead are you ok with the namespace change or was that "group" intentional? In our bundle, we would actually use Oneup\SentryBundle because the contao part is fictional to me, to show the package is built for Contao. But there's never gonna be a Terminal42\DcawizardBundle that is not for Contao, so no point in adding Contao to the namespace.

[fritzmg]

Personally I would definitely call it Oneup\ContaoSentryBundle as there already exists a Sentry\SentryBundle - which the Oneup\ContaoSentryBundle loads for the Contao Managed Edition. Calling it Oneup\SentryBundle would make it look like a fork of Sentry\SentryBundle imho - which it is not.

[bytehead]

I would optin for Oneup\ContaoSentryBundle 👍

[aschempp]

I have now used this on several systems in production, and it seems to work fine 🙃

[bytehead]

Back from holidays, I'll have a look this week!

[bytehead]

Looks good to me apart from the service naming (best practices).

[bytehead]

❤️

[bytehead]

Thank you @aschempp & @fritzmg!