[Debug] Add BufferingLogger for errors that happen before a proper logger is configured #15521
Conversation
8353fc2 to
054c169
Compare
| * | ||
| * @author Nicolas Grekas <p@tchwork.com> | ||
| */ | ||
| class BootstrappingLogger extends AbstractLogger |
There was a problem hiding this comment.
BootstrappingLogger does not make sense IMO. This logger has nothing specific to bootstrapping. It is a BufferLogger
There was a problem hiding this comment.
It's more than a BufferLogger because it has a special behavior in ErrorHandler, this is what makes it a BootstrappingLogger IMO
There was a problem hiding this comment.
well, the logger itself is not aware of this behavior. You could reuse it for what you want
500668d to
1ae2789
Compare
1ae2789 to
6e87463
Compare
| use Psr\Log\LoggerInterface; | ||
|
|
||
| /** | ||
| * A bootstrapping logger that can stack logs and flush them when required to an other logger. |
There was a problem hiding this comment.
to an other logger -> to another logger ?
4536f85 to
c2e5ba8
Compare
| { | ||
| public function testFlush() | ||
| { | ||
| for ($strip = 0; $strip < 2; ++$strip) { |
There was a problem hiding this comment.
use a dataProvider for that instead of a loop in the test. It will be much easier to debug failures as we will know which case is failing.
There was a problem hiding this comment.
dataProvider added
e944b5e to
28d05be
Compare
|
Since deprecations are silenced and the default config is not to "scream" them, this will not save them for later logging for now. I need to figure out something about this. Status: needs work |
28d05be to
1aa7c22
Compare
|
Fixed: deprecation notices are now screamed when $displayErrors is set to true Status: needs review |
1aa7c22 to
d69e38b
Compare
|
Last iteration: I removed the code that stripped scope vars and trace args. We have no use case for it: deprecations are already cleanup from those, and we throw all the other error levels. |
4b282c9 to
6026cf2
Compare
|
PR refactored, really the last round this time (although I can't ensure that @stof won't have any comments ;)): |
6026cf2 to
2a5bd28
Compare
| @@ -152,6 +153,14 @@ public static function register($handler = null, $replace = true) | |||
| return $handler; | |||
| } | |||
|
|
|||
| public function __construct(BufferingLogger $bootstrappingLogger = null) | |||
| { | |||
| if ($bootstrappingLogger) { | |||
There was a problem hiding this comment.
I don't think so: it's a BufferingLogger that is used for the bootstrapping step, thus a bootstrappingLogger internally.
2a5bd28 to
119c41c
Compare
|
@symfony/deciders should we merge this PR in 2.7? Even if this can be considered as a new feature, it fixes collecting early deprecation notices, that are currently completely silent on 2.7, and would stay so if we merge this one on 2.8. What do you think? |
…gger is configured
119c41c to
2a9647d
Compare
|
Thank you @nicolas-grekas. |
…ore a proper logger is configured (nicolas-grekas) This PR was merged into the 2.8 branch. Discussion ---------- [Debug] Add BufferingLogger for errors that happen before a proper logger is configured | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This allows catching e.g. deprecations that happen during bootstrapping. Commits ------- 2a9647d [Debug] Add BufferingLogger for errors that happen before a proper logger is configured
|
What about backporting this to 2.7 as @nicolas-grekas suggested? |
|
Well, it is technically a new feature, so it belongs to 2.8. and if people don't see some early deprecation warnings in 2.8, it is not a big issue: things still work. They will simply see it after upgrading to 2.8 (alongside new ones) |
This allows catching e.g. deprecations that happen during bootstrapping.