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

Minimal change to #2.

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! :)

Patch from #11 works fine.

Tested with Drupal 9.3.13

#11 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.

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 #2972846: Improve oEmbed exception logging 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 #2972846: Improve oEmbed exception logging.

#2972846: Improve oEmbed exception logging: 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()]
);

@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 ;))

Applied a patch from the Merge Request diff successfully to Drupal Core 10.3.13.

I reviewed the Merge Request in #2972846 first, but that issue does not add an access check before logging the error and does not appear to resolve the issue of this error showing to anonymous users. As another user commented in that issue, it appears the changes there are exposing even more detailed information to anonymous users which exacerbates the issue presented here.

Maybe it could dispatch an event when the source isn't found?

I have a site with over 100,000 entities with media references, many of these have become old and the youtube urls are no longer active. I would like to be able to flag the posts so I can easily find them and remove them or take some appropriate action.

If i was able to listen for an event when there is an error I could do this.

I ran into a unique fatal error caused by the patch. In short, the media entity being used to generate the edit link for the error message will not always have an ID or valid edit link. I recommend adding a condition to check for the ID of the media entity before continuing.

In length, a content type with an oEmbed media entity will reference the media entity using an entity reference field. When using layout builder for the default display for the content type, the content preview for layout builder uses the generateSampleValue method on the entity reference item to generate a preview. The method attempts to find a random, valid entity to use as a sample value. If it cannot find one, it generates an entity on the fly. The entity generated on the fly is not saved, and does not have a valid ID. This throws a fatal error then when using this code because it attempts to generate an edit link on a media entity without an ID. I recommend adding a condition to check for the ID of the media entity before continuing.

Perhaps there's a better way to go about this then adding a check for the $media->id().

I added the conditional to the merge request and cut a new patch on 10.4.x

Maybe it could dispatch an event when the source isn't found?

This sounds like a good follow up task as it expands the scope of this, and I think it would be better to fix the user-facing issue as originally reported first. This is definitely something I have noticed and agree it is a problem!

Re #20 and #31 - I think this issue should be fixed regardless of the #2972846: Improve oEmbed exception logging, because the error should not be visible to anonymous users. The other issue's MR does not fix the message displayed for anonymous users, so let's keep both issues open.