[FrameworkBundle] framework.php_errors.log now accept a log level #26504

Simperfit · 2018-03-13T08:33:58Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#26267
License	MIT
Doc PR	todo

We are testing that framework.php_errors.log is either a bool or an int (set the value of the log level you want).
This fixes #26267, so it gives a way to not log some level on production.

nicolas-grekas · 2018-03-13T10:01:35Z

would be great to turn the framework.php_errors.log config entry into a "bool|int", and when an "int" is provided, use the value to set the parameter

Simperfit · 2018-03-14T11:34:48Z

@nicolas-grekas I have changed this PR, could you please guide me on how to write test for this ?

nicolas-grekas

for tests, please check the existing ones, I don't have specific ideas on the topic sorry

nicolas-grekas · 2018-03-14T16:15:30Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

@@ -601,10 +601,14 @@ private function registerDebugConfiguration(array $config, ContainerBuilder $con

        $definition = $container->findDefinition('debug.debug_handlers_listener');

-        if (!$config['log']) {
+        if (\is_bool($config['log']) && !$config['log']) {


if (!$config['log']) { is enough: 0 also disables the logger

nicolas-grekas · 2018-03-14T16:15:52Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

            $definition->replaceArgument(1, null);
        }

+        if (\is_int($config['log'])) {


if (\is_int($config['log']) && $config['log']) {

nicolas-grekas · 2018-03-14T16:16:14Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

@@ -833,7 +833,7 @@ private function addPhpErrorsSection(ArrayNodeDefinition $rootNode)
                    ->info('PHP errors handling configuration')
                    ->addDefaultsIfNotSet()
                    ->children()
-                        ->booleanNode('log')
+                        ->scalarNode('log')


maybe forbid anything else than a bool|int?

Simperfit · 2018-03-26T06:33:29Z

Status: Needs Review

Simperfit · 2018-03-26T15:08:35Z

Tests Fixed.

fabpot · 2018-03-27T07:39:57Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

                            ->info('Use the app logger instead of the PHP logger for logging PHP errors.')
                            ->defaultValue($this->debug)
                            ->treatNullLike($this->debug)
+                            ->validate()
+                            ->ifTrue(function ($v) { return !(\is_int($v) || \is_bool($v)); })
+                            ->thenInvalid('The "php_errors.log" parameter should be either a int or a boolean.')


either an integer or a boolean?

@fabpot updated.

fabpot · 2018-03-27T07:47:54Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

                            ->info('Use the app logger instead of the PHP logger for logging PHP errors.')
                            ->defaultValue($this->debug)
                            ->treatNullLike($this->debug)
+                            ->validate()
+                            ->ifTrue(function ($v) { return !(\is_int($v) || \is_bool($v)); })
+                            ->thenInvalid('The "php_errors.log" parameter should be either a integer or a boolean.')


an integer :)

This is good now :p.

fabpot · 2018-03-27T18:28:25Z

Thank you @Simperfit.

…a log level (Simperfit) This PR was merged into the 4.1-dev branch. Discussion ---------- [FrameworkBundle] framework.php_errors.log now accept a log level | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes  | BC breaks? | no  | Deprecations? | no  | Tests pass? | yes  | Fixed tickets | #26267  | License | MIT | Doc PR | todo  We are testing that `framework.php_errors.log` is either a bool or an int (set the value of the log level you want). This fixes #26267, so it gives a way to not log some level on production. Commits ------- 664f821 [FrameworkBundle] framework.php_errors.log now accept a log level

…yrixx) This PR was merged into the 4.1-dev branch. Discussion ---------- [FrameworkBundle] Fixed configuration of php_errors.log | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #26504 and #26740 (wrong implementation) | License | MIT | Doc PR | Commits ------- 8e0fcf9 [FrameworkBundle] Fixed configuration of php_errors.log

bendavies · 2018-05-29T09:49:07Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

@@ -833,10 +833,14 @@ private function addPhpErrorsSection(ArrayNodeDefinition $rootNode)
                    ->info('PHP errors handling configuration')
                    ->addDefaultsIfNotSet()
                    ->children()
-                        ->booleanNode('log')
+                        ->scalarNode('log')
                            ->info('Use the app logger instead of the PHP logger for logging PHP errors.')


is this really descriptive now?

mathieu-pousse · 2019-05-23T08:48:28Z

Hi,

This PR looks great to me, but I am not sure to ensure the aim.
Does the log level behind the scene correspond to something like 'E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED & ~E_STRICT' or to DEVUG, INFO, WARN, ?

In any case, I think this should be explain in the documentation

SublimeRaiser · 2020-01-09T04:50:33Z

This PR looks great to me, but I am not sure to ensure the aim.
Does the log level behind the scene correspond to something like 'E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED & ~E_STRICT' or to DEVUG, INFO, WARN, ?

In any case, I think this should be explain in the documentation

Here is a link to the docs: https://symfony.com/doc/current/reference/configuration/framework.html#log

carsonbot added Status: Needs Review FrameworkBundle Feature labels Mar 13, 2018

nicolas-grekas added this to the 4.1 milestone Mar 13, 2018

Simperfit force-pushed the feature/add-a-parameter-for-log-level branch from d71b26f to 5b6a4fe Compare March 14, 2018 11:34

Simperfit changed the title ~~[FrameworkBundle] add a parameter for the debug production log level~~ [FrameworkBundle] framework.php_errors.log now accept a log level Mar 14, 2018

nicolas-grekas requested changes Mar 14, 2018

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Mar 14, 2018

Simperfit force-pushed the feature/add-a-parameter-for-log-level branch from 5b6a4fe to b02c7d7 Compare March 26, 2018 06:33

carsonbot added Status: Needs Review and removed Status: Needs Work labels Mar 26, 2018

nicolas-grekas approved these changes Mar 26, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Mar 26, 2018

Simperfit force-pushed the feature/add-a-parameter-for-log-level branch from b02c7d7 to c713dca Compare March 26, 2018 15:08

carsonbot removed the Status: Reviewed label Mar 27, 2018

fabpot requested changes Mar 27, 2018

View reviewed changes

carsonbot added the Status: Needs Work label Mar 27, 2018

Simperfit force-pushed the feature/add-a-parameter-for-log-level branch from c713dca to 556c6c1 Compare March 27, 2018 07:45

fabpot requested changes Mar 27, 2018

View reviewed changes

[FrameworkBundle] framework.php_errors.log now accept a log level

664f821

Simperfit force-pushed the feature/add-a-parameter-for-log-level branch from 556c6c1 to 664f821 Compare March 27, 2018 14:39

fabpot merged commit 664f821 into symfony:master Mar 27, 2018

Simperfit deleted the feature/add-a-parameter-for-log-level branch March 30, 2018 09:22

lyrixx mentioned this pull request Apr 9, 2018

[FrameworkBundle] Fixed configuration of php_errors.log #26874

Merged

javiereguiluz mentioned this pull request Apr 20, 2018

[FrameworkBundle] framework.php_errors.log now accept a log level symfony/symfony-docs#9649

Closed

fabpot mentioned this pull request May 7, 2018

Release v4.1.0-BETA1 #27181

Merged

bendavies reviewed May 29, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FrameworkBundle] framework.php_errors.log now accept a log level #26504

[FrameworkBundle] framework.php_errors.log now accept a log level #26504

Simperfit commented Mar 13, 2018 •

edited

Loading

nicolas-grekas commented Mar 13, 2018

Simperfit commented Mar 14, 2018

nicolas-grekas left a comment

nicolas-grekas Mar 14, 2018

nicolas-grekas Mar 14, 2018

nicolas-grekas Mar 14, 2018

Simperfit Mar 26, 2018

Simperfit commented Mar 26, 2018

Simperfit commented Mar 26, 2018

fabpot Mar 27, 2018

Simperfit Mar 27, 2018

fabpot Mar 27, 2018

Simperfit Mar 27, 2018

fabpot commented Mar 27, 2018

bendavies May 29, 2018

mathieu-pousse commented May 23, 2019

SublimeRaiser commented Jan 9, 2020

[FrameworkBundle] framework.php_errors.log now accept a log level #26504

[FrameworkBundle] framework.php_errors.log now accept a log level #26504

Conversation

Simperfit commented Mar 13, 2018 • edited Loading

nicolas-grekas commented Mar 13, 2018

Simperfit commented Mar 14, 2018

nicolas-grekas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Simperfit commented Mar 26, 2018

Simperfit commented Mar 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Mar 27, 2018

Choose a reason for hiding this comment

mathieu-pousse commented May 23, 2019

SublimeRaiser commented Jan 9, 2020

Simperfit commented Mar 13, 2018 •

edited

Loading