Apply code review suggestions

Omer LAKRAA · Omer LAKRAA · commit da2bfebcf344 · 2024-06-25T14:26:03.000+02:00
diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
@@ -8,7 +8,7 @@ CHANGELOG
  * Derivate `kernel.secret` from the decryption secret when its env var is not defined
  * Make the `config/` directory optional in `MicroKernelTrait`, add support for service arguments in the
    invokable Kernel class, and register `FrameworkBundle` by default when the `bundles.php` file is missing
- * Add exception.x.log_channel
+ * Add a new logging channel for exceptions to provide better granularity and control over exception logging.
 
 7.1
 ---
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -386,7 +386,7 @@ public function load(array $configs, ContainerBuilder $container): void
             if (!isset($exception['log_channel'])) {
                 continue;
             }
-            $loggers[$exception['log_channel']] = new Reference(sprintf('monolog.logger.%s', $exception['log_channel']));
+            $loggers[$exception['log_channel']] = new Reference(sprintf('monolog.logger.%s', $exception['log_channel']), ContainerInterface::NULL_ON_INVALID_REFERENCE);
         }
         
         $exceptionListener
diff --git a/src/Symfony/Component/HttpKernel/CHANGELOG.md b/src/Symfony/Component/HttpKernel/CHANGELOG.md
@@ -1,10 +1,11 @@
 CHANGELOG
 =========
+
 7.2
 ---
 
  * Remove `@internal` flag and add `@final` to `ServicesResetter`
- * Add log_channel in `ErrorListener`
+ * Add a new logging channel for exceptions to provide better granularity and control over exception logging.
 
 7.1
 ---
diff --git a/src/Symfony/Component/HttpKernel/EventListener/ErrorListener.php b/src/Symfony/Component/HttpKernel/EventListener/ErrorListener.php
@@ -165,12 +165,9 @@ public static function getSubscribedEvents(): array
     protected function logException(\Throwable $exception, string $message, ?string $logLevel = null, ?string $logChannel = null): void
     {
         $logLevel ??= $this->resolveLogLevel($exception);
-        
         $logChannel ??= $this->resolveLogChannel($exception);
         
-        $logger = $this->getLogger($logChannel);
-        
-        if(null === $logger) {
+        if(!$logger = $this->getLogger($logChannel)) {
             return;
         }
 
@@ -267,8 +264,8 @@ private function getInheritedAttribute(string $class, string $attribute): ?objec
         return $attributeReflector?->newInstance();
     }
 
-    private function getLogger(?string $logChannel = null): ?LoggerInterface
+    private function getLogger(?string $logChannel): ?LoggerInterface
     {
-        return $this->loggers[$logChannel] ?? $this->logger;
+        return $logChannel ? $this->loggers[$logChannel] ?? $this->logger : $this->logger;
     }
 }
diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/ErrorListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/ErrorListenerTest.php
@@ -143,7 +143,6 @@ public function testHandleWithLogLevelAttribute()
         $this->assertCount(1, $logger->getLogs('warning'));
     }
 
-
     public function testHandleWithLogChannel()
     {
         $request = new Request();
@@ -196,7 +195,7 @@ public function testHandleWithLoggerChannelNotUsed()
 
         $this->assertEquals(new Response('foo', 401), $event->getResponse());
 
-        $this->assertEquals(0, $defaultLogger->countErrors());
+        $this->assertSame(0, $defaultLogger->countErrors());
         $this->assertCount(0, $defaultLogger->getLogs('critical'));
         $this->assertCount(1, $defaultLogger->getLogs('warning'));
         $this->assertCount(0, $channelLoger->getLogs('warning'));

Original file line number	Diff line number	Diff line change
`@@ -386,7 +386,7 @@ public function load(array $configs, ContainerBuilder $container): void`
`386`	`386`	`if (!isset($exception['log_channel'])) {`
`387`	`387`	`continue;`
`388`	`388`	`}`
`389`		`- $loggers[$exception['log_channel']] = new Reference(sprintf('monolog.logger.%s', $exception['log_channel']));`
	`389`	`+ $loggers[$exception['log_channel']] = new Reference(sprintf('monolog.logger.%s', $exception['log_channel']), ContainerInterface::NULL_ON_INVALID_REFERENCE);`
`390`	`390`	`}`
`391`	`391`
`392`	`392`	`$exceptionListener`
Original file line number	Diff line number	Diff line change
`@@ -165,12 +165,9 @@ public static function getSubscribedEvents(): array`
`165`	`165`	`protected function logException(\Throwable $exception, string $message, ?string $logLevel = null, ?string $logChannel = null): void`
`166`	`166`	`{`
`167`	`167`	`$logLevel ??= $this->resolveLogLevel($exception);`
`168`		`-`
`169`	`168`	`$logChannel ??= $this->resolveLogChannel($exception);`
`170`	`169`
`171`		`- $logger = $this->getLogger($logChannel);`
`172`		`-`
`173`		`- if(null === $logger) {`
	`170`	`+ if(!$logger = $this->getLogger($logChannel)) {`
`174`	`171`	`return;`
`175`	`172`	`}`
`176`	`173`
`@@ -267,8 +264,8 @@ private function getInheritedAttribute(string $class, string $attribute): ?objec`
`267`	`264`	`return $attributeReflector?->newInstance();`
`268`	`265`	`}`
`269`	`266`
`270`		`- private function getLogger(?string $logChannel = null): ?LoggerInterface`
	`267`	`+ private function getLogger(?string $logChannel): ?LoggerInterface`
`271`	`268`	`{`
`272`		`- return $this->loggers[$logChannel] ?? $this->logger;`
	`269`	`+ return $logChannel ? $this->loggers[$logChannel] ?? $this->logger : $this->logger;`
`273`	`270`	`}`
`274`	`271`	`}`
Original file line number	Diff line number	Diff line change
`@@ -143,7 +143,6 @@ public function testHandleWithLogLevelAttribute()`
`143`	`143`	`$this->assertCount(1, $logger->getLogs('warning'));`
`144`	`144`	`}`
`145`	`145`
`146`		`-`
`147`	`146`	`public function testHandleWithLogChannel()`
`148`	`147`	`{`
`149`	`148`	`$request = new Request();`
`@@ -196,7 +195,7 @@ public function testHandleWithLoggerChannelNotUsed()`
`196`	`195`
`197`	`196`	`$this->assertEquals(new Response('foo', 401), $event->getResponse());`
`198`	`197`
`199`		`- $this->assertEquals(0, $defaultLogger->countErrors());`
	`198`	`+ $this->assertSame(0, $defaultLogger->countErrors());`
`200`	`199`	`$this->assertCount(0, $defaultLogger->getLogs('critical'));`
`201`	`200`	`$this->assertCount(1, $defaultLogger->getLogs('warning'));`
`202`	`201`	`$this->assertCount(0, $channelLoger->getLogs('warning'));`