search

born05 / craft-sentry

Pushes Craft CMS errors to Sentry.
MIT License
10 stars 11 forks source link

Improve check for Craft Environment variable #27

Closed martinhellwagner closed 2 years ago

martinhellwagner commented 2 years ago

In one of our projects, we have the problem that the CRAFT_ENVIRONMENT variable (even though correctly defined in the web/index.php file) is not correctly consumed by the plugin. This unfortunately breaks our build pipeline as well as each queue job that wants to report a Sentry error.

I therefore propose an additional check in the file Plugin.php (line 64) in order to make sure that the right Craft Environment is consumed:

'environment' => defined('CRAFT_ENVIRONMENT') ? CRAFT_ENVIRONMENT : getenv('ENVIRONMENT')
roelvanhintum commented 2 years ago

@martinhellwagner currently (2.0.1) the plugin uses App::env, which should automatically solve this. see: https://github.com/craftcms/cms/blob/main/src/helpers/App.php#L91

Additionally: Craft 4 has deprecated the ENVIRONMENT env in favor of CRAFT_ENVIRONMENT.

martinhellwagner commented 2 years ago

@roelvanhintum

Thanks for the clarification! Is it possible to use version 2.x of the plugin with Craft 3 as well? We haven't yet upgraded the project to Craft 4.

roelvanhintum commented 2 years ago

@martinhellwagner not sure, but i think not. The upside is that the craft 3 version of the plugin does lean on the ‘CRAFT_ENVIRONMENT’ variable.

martinhellwagner commented 2 years ago

@roelvanhintum

Is there any way to make the 1.x version of the plugin work when the CRAFT_ENVIRONMENT variable is not correctly consumed?

roelvanhintum commented 2 years ago

@martinhellwagner not sure what you mean, isn’t this working? It should be just using the var as usual in craft 3. https://github.com/born05/craft-sentry/blob/master/src/Plugin.php#L64

martinhellwagner commented 2 years ago

@roelvanhintum

We're setting the variable like this in our web/index.php file:

define('CRAFT_ENVIRONMENT', getenv('ENVIRONMENT') ?: 'production');

But in line 64 of the Plugin.php file, this variable isn't available and can't be consumed, resulting in a failure of our pipeline (or every time a Sentry error should be reported).

roelvanhintum commented 2 years ago

Just to be sure, is the variable set before bootstrapping craft? And is it also set in the ‘craft’ file? Later versions of craft 3 suggest using a bootstrap.php, to make sure they are the same in both files.

martinhellwagner commented 2 years ago

@roelvanhintum

Acutally, it was not set in the craft file. That seems to have been the issue. Thanks a lot for pointing that out! 👍🏼