search

liip / LiipMonitorBundle

Integrates the LiipMonitor library into Symfony
http://liip.ch
467 stars 103 forks source link

Missing parameters after update to Symfony 5 #242

Closed knallcharge closed 4 years ago

knallcharge commented 4 years ago

Hi,

I just updated a project to Symfony 5 and got two errors with the monitor bundle concerning the "custom_error_pages"-checks:

  1. You have requested a non-existent parameter "kernel.root_dir". I can get rid of this by replacing "'%kernel.root_dir%'" with "'%kernel.project_dir%/src'" in my monitor.yaml.

  2. You have requested a non-existent parameter "twig.exception_listener.controller". Can't find a fix for this, %twig.exception_listener.controller% is also still listed in the documentation (https://github.com/liip/LiipMonitorBundle#full-default-config), is there some change needed to make this work with Symfony 5?

kbond commented 4 years ago

Indeed, this is broken now. Are your error pages located in the documented place for 5.1?

%kernel.project_dir%/templates/bundles/TwigBundle/Exception/?

knallcharge commented 4 years ago

Thanks for getting back. Yes, my error pages are at the right place (this hasn't changed since 4.4 which was my prior version: https://symfony.com/doc/4.4/controller/error_pages.html).

kbond commented 4 years ago

Hmm, and the check worked for you in 4.4?

knallcharge commented 4 years ago

Yes.

kbond commented 4 years ago

Ok, looks like you may have been getting a false positive in 4.4 as the location is hard-coded in the check: https://github.com/liip/LiipMonitorBundle/blob/86865ca9aec9d93ef0ea7be9cd15d038f2c98dea/Check/CustomErrorPages.php#L64

I think https://github.com/liip/LiipMonitorBundle/blob/86865ca9aec9d93ef0ea7be9cd15d038f2c98dea/Check/CustomErrorPages.php#L60 must have been evaluating to false and always returning success...

kbond commented 4 years ago

Here's what I'm thinking as a solution:

  1. Change the default value of custom_error_pages.path to %kernel.project_dir%
  2. Deprecate custom_error_pages.controller as it would no longer be needed
  3. In the check:
    1. If the custom_error_pages.path === %kernel.project_dir% AND the %kernel.project_dir%/app/Resources/TwigBundle/views/Exception/ directory exists, user is using 3.4 location
    2. If the custom_error_pages.path === %kernel.project_dir% AND the %kernel.project_dir%/templates/bundles/TwigBundle/Exception/ directory exists, user is using 4.0+ location
    3. If the custom_error_pages.path is something other then %kernel.project_dir%, use that as the directory to look for the custom error pages (user is using something custom)

If someone is doing something very custom like using different template names (ie %d_error.html.twig) this would be incompatible with this check...

What do you think?

knallcharge commented 4 years ago

Sounds good.