Issue #3028664 by averagejoe3000, phenaproxima, nathandentzau, alexpott: Log...

Issue #3028664 by averagejoe3000, phenaproxima, nathandentzau, alexpott: Log exceptions for invalid oEmbed providers

core/modules/media/src/OEmbed/Provider.php

@@ -43,11 +43,11 @@ class Provider {
    * @throws \Drupal\media\OEmbed\ProviderException
    */
   public function __construct($name, $url, array $endpoints) {
+    $this->name = $name;
+
     if (!UrlHelper::isValid($url, TRUE) || !UrlHelper::isExternal($url)) {
       throw new ProviderException('Provider @name does not define a valid external URL.', $this);
     }
-
-    $this->name = $name;
     $this->url = $url;
 
     try {

core/modules/media/src/OEmbed/ProviderRepository.php

@@ -158,8 +158,9 @@ public function getAll() {
         $keyed_providers[$name] = new Provider($provider['provider_name'], $provider['provider_url'], $provider['endpoints']);
       }
       catch (ProviderException $e) {
-        // Just skip all the invalid providers.
-        // @todo Log the exception message to help with debugging.
+        // Skip invalid providers, but log the exception message to help with
+        // debugging.
+        $this->logger->warning($e->getMessage());
       }
     }
 

core/modules/media/tests/src/Unit/ProviderRepositoryTest.php

@@ -4,6 +4,7 @@
 
 use Drupal\Core\KeyValueStore\KeyValueMemoryFactory;
 use Drupal\Core\Logger\LoggerChannelFactory;
+use Drupal\Core\Logger\RfcLogLevel;
 use Drupal\media\OEmbed\ProviderException;
 use Drupal\media\OEmbed\ProviderRepository;
 use Drupal\Tests\UnitTestCase;
@@ -11,6 +12,7 @@
 use GuzzleHttp\Handler\MockHandler;
 use GuzzleHttp\HandlerStack;
 use GuzzleHttp\Psr7\Response;
+use Prophecy\Argument;
 
 /**
  * @coversDefaultClass \Drupal\media\OEmbed\ProviderRepository
@@ -47,6 +49,13 @@ class ProviderRepositoryTest extends UnitTestCase {
    */
   private $currentTime;
 
+  /**
+   * The mocked logger channel.
+   *
+   * @var \Psr\Log\LoggerInterface|\Prophecy\Prophecy\ObjectProphecy
+   */
+  private $logger;
+
   /**
    * {@inheritdoc}
    */
@@ -66,6 +75,10 @@ protected function setUp(): void {
     $time = $this->prophesize('\Drupal\Component\Datetime\TimeInterface');
     $time->getCurrentTime()->willReturn($this->currentTime);
 
+    $this->logger = $this->prophesize('\Psr\Log\LoggerInterface');
+    $logger_factory = new LoggerChannelFactory();
+    $logger_factory->addLogger($this->logger->reveal());
+
     $this->responses = new MockHandler();
     $client = new Client([
       'handler' => HandlerStack::create($this->responses),
@@ -75,7 +88,7 @@ protected function setUp(): void {
       $config_factory,
       $time->reveal(),
       $key_value_factory,
-      new LoggerChannelFactory()
+      $logger_factory
     );
   }
 
@@ -217,6 +230,13 @@ public function testCorruptProviderIgnored(): void {
     $response = new Response(200, [], $body);
     $this->responses->append($response);
 
+    // The corrupt provider should cause a warning to be logged.
+    $this->logger->log(
+      RfcLogLevel::WARNING,
+      "Provider Uncle Rico's football videos does not define a valid external URL.",
+      Argument::type('array')
+    )->shouldBeCalled();
+
     $youtube = $this->repository->get('YouTube');
     // The corrupt provider should not be stored.
     $stored_data = [