justbetter / magento2-sentry

Magento 2 module to log to Sentry
MIT License
160 stars 71 forks source link

Deployment configuration ignored when "Environment Configuration Enabled" set to "No" #128

Open mfickers opened 5 months ago

mfickers commented 5 months ago

Bug: What is the current behavior? When store configuration "Environment Configuration Enabled" is saved to "No", the deployment configuration is ignored and no data is sent to Sentry. This only happens when there is an entry like this in the core_config_data table: sentry/environment/enabled 0

Bug: What is the expected behavior? When store configuration "Environment Configuration Enabled" is saved to "No", the deployment configuration is used instead.

Bug: What is the proposed solution? I've traced this bug to \JustBetter\Sentry\Helper\Data::collectModuleConfig:

            $this->config['enabled'] = $this->scopeConfig->getValue('sentry/environment/enabled')
                ?? $this->deploymentConfig->get('sentry') !== null;

If getValue() returns null the second check is performed and the configuration is loaded from the deployment config. But if getValue() returns "0", then "0" will be saved to $config['enabled'], which means the module will be treated as disabled, regardless of the deployment config.

What is the version of Magento and of Sentry extension you are using? Always use the latest version of the extension one before opening a bug issue.

PHP 8.2 Magento 2.4.6-p4 Sentry extension 3.5.2

indykoning commented 5 months ago

You're absolutely right on where the problem is coming from. Though as a user i would expect it being disabled in the backend meaning that it is indeed disabled regardless of existence in deployment config.

Perhaps we should disable all sentry/environment config in the system.xml when sentry has been configured using deploymentConfig

mfickers commented 4 months ago

If someone else is experiencing this problem, you can work around this issue by deleting the entry for sentry/environment/enabled from the core_config_data table.

@indykoning, what happened to us was I wanted to enable JavaScript tracking. So I've changed "Enable Javascript Tracking" in the admin config to "Yes" and clicked on "Save". This broke the Sentry integration completely, as the environment configuration field was saved too. And there is no way to fix it without either access to the CLI or the database.