I'm trying to migrate to YouTube data api v3 and so I'm trying to upgrade the site to media_youtube 3.0. When I try to add a new node for a content type with a video field (allowed for YouTube and file upload), every time calling media browser (link 'browse') I get this error:

Location:
https://mydomain.com/en/media/browser?render=media-popup&types%5Bvideo%5D=video&enabledPlugins%5Bupload%5D=upload&enabledPlugins%5Bmedia_default--media_browser_my_files%5D=media_default--media_browser_my_files&enabledPlugins%5Bmedia_internet%5D=media_internet&enabledPlugins%5Byoutube%5D=youtube&schemes%5Bvimeo%5D=vimeo&schemes%5Byoutube%5D=youtube&schemes%5Bpublic%5D=public&schemes%5Bprivate%5D=private&file_directory=&file_extensions=avi+mov+mp4+mpegps+mpeg4+ogg+wmv+flv+webm&max_filesize=500MB&uri_scheme=private&plugins=undefined

Referrer:
https://mydomain.com/en/node/add/video

Message:
Exception: Error Processing Request. (Error: 404, Not Found) in MediaYouTubeStreamWrapper->getOriginalThumbnailPath() (line 31 of /home/www/mydomain.com/sites/all/modules/media_youtube/includes/MediaYouTubeStreamWrapper.inc).

There seems to be called a YouTube url before any input is given. Which thumbail is searched? What could be wrong with the installation?

Backtrace attached as .txt file.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aquariumtap’s picture

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.

aquariumtap’s picture

Issue summary: View changes
FileSize
6.71 KB
BOGUƎ’s picture

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

rreiss’s picture

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

marrrc’s picture

Status: Active » Needs review
FileSize
664 bytes

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.

wouters_f’s picture

Fix running in production here. Works great.

zerolab’s picture

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.

ksi’s picture

Thank you aquariumtap and mii31. That helped a lot!

hctom’s picture

Status: Needs review » Reviewed & tested by the community

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

Devin Carlson’s picture

Title: Calling media browser runs into an error calling getOriginalThumbnailPath() » No exception handling when error from YouTube oEmbed
Version: 7.x-3.0 » 7.x-3.x-dev
Category: Support request » Bug report
Status: Reviewed & tested by the community » Needs review
Issue tags: -media browser
FileSize
2.2 KB

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

Devin Carlson’s picture

Status: Needs review » Fixed

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

zerolab’s picture

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"

zerolab’s picture

Status: Fixed » Needs work

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

Devin Carlson’s picture

@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.

zerolab’s picture

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

brunodbo’s picture

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

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

zerolab’s picture

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.

alienzed’s picture

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?

jucedogi’s picture

Version: 7.x-3.x-dev » 7.x-3.0

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.

Kazanir’s picture

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.

chrisgross’s picture

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

chrisgross’s picture

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.

zerolab’s picture

Status: Needs work » Needs review
steinmb’s picture

Version: 7.x-3.0 » 7.x-3.x-dev
Status: Needs review » Needs work

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.

steinmb’s picture

Status: Needs work » Postponed (maintainer needs more info)

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.

Nitesh Sethia’s picture

Version: 7.x-3.x-dev » 7.x-3.0
Status: Postponed (maintainer needs more info) » Reviewed & tested by the community

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.

steinmb’s picture

Status: Reviewed & tested by the community » Closed (won't fix)

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

marcoka’s picture

confirmed latest dev fixed it for me too

dani3lr0se’s picture

Confirming, latest dev version works for me as well.

sirtet’s picture

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.

bonrita’s picture

Update the patch to fit the new module version.

bonrita’s picture

bonrita’s picture

Updated patches for version 3.9

joseph.olstad’s picture

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!