Don't display OEmbed error to anonymous visitors when resource stops being available

Adding patch to log instead of sending a message on oEmbed fetch error.

Drupal 8 is end-of-life as of November 17, 2021 → . There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .

Minimal change to #2.
(Sorry, a little problem in the previous patch.)

I agree this issue makes sense and it's a bug, since the "Could not retrieve the oEmbed resource" being displayed to anonymous visitors when a remote video stops being available isn't ideal.

However, I do think that's a good indicator for editors. So my suggestion is that instead of always logging, we should check $media->access('update')
- If the user can edit the media item, show the error on screen, so they can take action to fix the problem
- If the user can't edit, it's likely a visitor, in which case we should just just log the error.

And yes, we should definitely return NULL in any case (as suggested by #6), otherwise the site will break hard when it tries to call methods on the non-existing $resource variable.

Also, we need tests! :)

Drupal 9.4.0-alpha1 → was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .

Patch from #11 🐛 Don't display OEmbed error to anonymous visitors when resource stops being available Needs work works fine.

Tested with Drupal 9.3.13

Drupal 9.5.0-beta2 → and Drupal 10.0.0-beta2 → were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .

#11 🐛 Don't display OEmbed error to anonymous visitors when resource stops being available Needs work is good for me too.

Tested with Drupal 9.4.7

Added 10.1.x: PHP 8.1 & MySQL 5.7 to #11 to see how it works out.

Rewrote a bit issue-3202896-6.patch, added link for editing media entity.

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened → , as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .

Sounds like a good addition!

Only thing that is left is a proper test to move this issue forward.

Should we perhaps close this issue and merge efforts in ✨ Improve oEmbed exception logging Needs work to make fixing these issues and testing more easy? Both are touching the message details and are very close... What do you think?

To me the approach in this issue is better than ✨ Improve oEmbed exception logging Needs work .

✨ Improve oEmbed exception logging Needs work : The change in core/modules/media/src/OEmbed/ResourceFetcher.php will now expose a lot of information to an anonymous user when the fetcher error message is displayed by \Drupal\media\Plugin\media\Source\OEmbed::getMetadata(). That is not desired on any website.

  throw new ResourceException('Could not retrieve the oEmbed resource: ' . $e->getMessage(), $url, [], $e);

When indexing media with unavailable resource using search api, I see the following error in the log:
Warning: Undefined variable $resource_url in Drupal\media\Plugin\media\Source\OEmbed->getMetadata() (line 253 of core/modules/media/src/Plugin/media/Source/OEmbed.php)

Steps to reproduce:
- Create media entity using live stream video using e.g. youtube.
- From youtube, finish the steaming and make it unavailable.
- Use search api to index media.
- run search index.
- Check the log.

I think we should use media source field value to display the resource url as follows:

$this->logger->error(
  $e->getMessage() . ' Resource URL: %resource_url | Media ID: %media_id',
  ['%resource_url' => $media->getSource()->getSourceFieldValue($media), '%media_id' => $media->id()]
);

recrit → changed the visibility of the branch 3202896-dont-display-oembed to hidden.

recrit → changed the visibility of the branch 11.x to hidden.

recrit → changed the visibility of the branch 10.3.x to hidden.

recrit → changed the visibility of the branch 10.4.x to hidden.

@msnassar The Undefined variable $resource_url should be fixed in the latest MR.
I created an MR against 11.x with error message updated to the following. I removed the "|" from the message since system logs often are delimited with "|" which could cause issue with parsing the log.

      $this->logger->error(
       'An error occurred while fetching an oEmbed resource for Media ID: %media_id, Embed URL: %media_url, oEmbed Resource URL: %resource_url. Error: @error',
        [
          '%resource_url' => !empty($resource_url) ? $resource_url : 'unknown',
          '%media_url' => $media_url,
          '%media_id' => $media->id(),
          '@error' => $e->getMessage(),
        ]
      );

Attached is a static patch of the MR for composer builds. Please contrib to the MR for any further development.

The patch issue-3202896-7.patch and the diff drupal-3202896-MR9357-11.x--20240828-1.diff look good for me.

Have not reviewed but was previously tagged for tests which still appear to be needed.

See #20 I still think it would make more sense to get that one finished instead...? (And yes, it's annoying for users ;))