No exception handling when error from YouTube oEmbed

An exception is thrown when a video stored in Drupal is no longer accessible on YouTube, and a new thumbnail needs to be generated.

This patch causes the getOriginalThumbnailPath() function to return the YouTube 404 image if it's missing.

Same problem here, patch works like a charm, thanks aquariumtap

#1 works great. Thanks for the quick patch man :)

In some cases, response data returns error code 401 for this, too, so we have to check for this as well.
In the future, it would be a good idea to somehow alert the user that there are assets in his library that do not have a source anymore.

Fix running in production here. Works great.

This worked well, thank you.
One note: had to resave the patch from #5 with Unix line endings in order to apply it on Debian/OS X.

IMHO, this is RTBC.

Except the line ending issues with the patch from #5, this works like a charm and I am finally able again to add video ;) Thanx a lot! Changing this to RTBC so it makes its way to the development branch quickly...

We should be handling the exception thrown by getOriginalThumbnailPath and using the appropriate MIME type icon provided by the Media module in the event of an endpoint error

Tested #10 and committed to Media: YouTube 7.x-3.x.

Devin,

The latest patch fails with the same exception as originally reported:

Exception: Error Processing Request. (Error: <em class="placeholder">404</em>, <em class="placeholder">Not Found</em>) in MediaYouTubeStreamWrapper->getOriginalThumbnailPath() (line 31 of /sites/all/modules/contrib/media_youtube/includes/MediaYouTubeStreamWrapper.inc)

The trouble is, once the video has been removed, the exception thrown in getOriginalThumbnailPath() is not caught in getLocalThumbnailPath() when going to edit the node.

Steps to reproduce:

1. Add a YouTube video via the media browser in a WYSIWYG-enabled text field
2. Save the node
3. Remove the video from YouTube
4. Go back to edit the node
5. Marvel at the Exception.

Cheers,
Dan

edit: changed 3. to add "from YouTube"

Sorry to reopen, but #10 did not fix this fully.
It works for new videos, but fails with old ones.

@zerolab I don't see an exception when following your steps to reproduce.

Why would the file be rendered at all if it has been deleted and, even if something was trying to render the file, why would the exception not get caught by getLocalThumbnailPath()?

I'll work on adding some test coverage and see if I can reproduce.

Hi Devin,

Step 3 should really read "remove the video from YouTube". Will run another batch of tests to confirm I did not dream this all up.

Thank you for moving the issue queue, btw.

Dan

Confirming that the patch in #10 fixes this error for me.

@zerolab: Did you get a chance to do some more testing?

Hi brunodbo,

Sorry, did not get to because of a hectic schedule. Will try to do it over the next coming days.

@Devin - Just re-read the steps to reproduce and I see step 3 can be confusing. It should read "Remove the video from YouTube", not Drupal.

applied the patch in #10, still getting an error. This happens when I paginate in the media browser from wysiwyg embed.. it kills the widget so this is pretty critical.
The patch in #1 and the subsequent addition for 401s works great though!
I am concerned that the URL to the Youtube 404 image may not be super stable... should Media Youtube perhaps include this image inside it's own module folder?

I just ran into this site breaking issue on 7.x-3.0; this is not a dev related issue only.
In my case I replaced that line and instead of throwing an Exception I registered it with the watchdog function on drupal's log.

It looks like the exception handling in Devin's patch isn't enough because ->getOriginalThumbnailPath() is called (for an unfilled media field) from media_youtube_file_formatter_image_view directly, rather than only via getLocalThumbnailPath(). If the oembed call fails it blows up that function rather than being caught as Devin's patch intends.

I can't figure out why an exception needs thrown at all by getOriginalThumbnailPath(), when exceptions are already being thrown by getOembed() and validId().

Someone correct me if I'm wrong on this, but I don't see a reason for throwing an exception for a missing thumbnail image. The issue that would cause this is a youtube video that is no longer accessible, and there is already exception handling for that. Here's a patch that removes it and grabs a default thumbnail in that scenario.

It is perfectly OK throwing a exception and returning why it failed, we just have to make sure we catch it and not let Drupal default exception handler catch it. media_video (and perhaps more) also does this. media_vimeo also fail like this module catching the exception. +1 on fixing the problem not removing it.

mmmm retested with latest dev of media, file_entity, media_youtube and media_vimeo and no longer able to reproduce the error. Verified with debugger and traced all exception thrown by vimeo and youtube modules and they got all catched.

This issue was there in 7.x-3.0 version of Media Youtube Module. After applying #23 patch, Media plugin of WYSIWYG editor started working.

Thanks chrisgross for the patch.

Changing the status of it to Reviewed and Tested by Community.

If it is fixed in dev. then there is nothing to review, commit or test. Any new patches must be done against dev.

confirmed latest dev fixed it for me too

Confirming, latest dev version works for me as well.

I ran into this, and that was my workaround:

I had a slideshow node-type, which had a field linking to file-nodes.
Because of a no longer available youtube vid i could no longer edit one of these slideshow-nodes.
It would still display, with the video-slide being just black.

Next i deleted the file-node of that video.

Now the viewing of the slideshow-node failded.
But editing worked.
Doing so, there was a duplicated add-media link at the bottom of the file-field (originating from the now-gone file-node).
Via that add-media link i could select a different file-node, and after that, i could delete it from the file-field.

Done.

Update the patch to fit the new module version.

Updated patches for version 3.9

Hi thanks for the patch however I think this patch merits a new issue with a new description as I don't think that the patch reflects the original purpose.

Please when you do create a new issue please add the hyperlink to the new issue into a new comment on this issue so that others can follow the link and find your patch.

I will review the new issue but I will not review any more patches for this issue that was closed 4 years ago.

If you want me to review your patch please create a new issue and put the link to it in a new comment here then I will follow the link and hopefully others will help review it.

in the new issue summary, please describe what your patch is trying to accomplish and how to reproduce your use case and configuration for the purposes of evaluating your patch.

Thanks again for your efforts!