In debugging #2831944-275: Implement media source plugin for remote video via oEmbed → with @alexpott, it was discovered that media source plugins can, despite the safeguards in place to prevent this, return an empty/null thumbnail URI. This can break media entity save operations (and other situations), raising ugly things like database exceptions. The oEmbed system is especially prone to this because it often has make requests to external HTTP APIs in order to determine the the thumbnail URI. Sensitive operations like entity writes are exactly the wrong time for such activity.
The truth is, thumbnail handling is a sticky problem in Media that has never been satisfactorily solved. I'm not suggesting we fix the entire rotten enchilada in this one issue, but this is a nasty symptom of the overarching problem. We definitely need to fix it for oEmbed first, though, since remote media is very dependent on external APIs and network conditions.
TBD, but @catch suggested this in Slack:
alexpott [12:00 PM]
One thing that all of this points to is that we have fragility when the request to get info doesn't work
\Drupal\media\Entity\Media::updateThumbnail has to handle \Drupal\media\Entity\Media::getThumbnailUri() returning NULL
Since \Drupal\media\MediaSourceInterface::getMetadata() can return NULL
catch [12:05 PM]
@alexpott why is presave doing an httprequest?
phenaproxima [12:05 PM]
@alexpott Agreed. I think the reason that case was never handled is that *all* media items must have a thumbnail, so it's never supposed to be returning NULL. That's why we have fallback thumbnail values
catch [12:05 PM]
@alexpott is it thumbnails?
phenaproxima [12:05 PM]
@catch It is.
@catch In the case of oEmbed, anyway.
catch [12:05 PM]
We should really use a dummy thumbnail and swap it out in the background for all this.
phenaproxima [12:06 PM]
@catch Thumbnail handling in Media was never, really, satisfactorily solved.
@catch So TL;DR -- I agree. This shit needs work.
@alexpott @catch I am opening a new issue for fixing this in oEmbed
catch [12:08 PM]
So blank thumbnail -> queue item -> replace in the queue.
Also add https://www.drupal.org/project/drupal/issues/1189464 to core and encourage waiting_queue type stuff for bigger sites.
Drupal.org
Add a browser-based queue runner
Problem/Motivation Drupal has two ways to handle long-running tasks: 1. Queue API, by default queues are run on cron, often sites run queues via jenkins on different schedules, there are projects like 'waiting queue' which aim to minimise the time before queue items are picked up.
Jun 15th, 2011
alexpott [12:10 PM]
@catch do you think in order to land oembed we need to fix this so the thumbnail is not updated as part of the transaction?
catch [12:11 PM]
@alexpott if the site you’re ombedding from is down temporarily, saving an entity should not fail.
So I think it’s worth fixing yeah.
I get the point about starting the transaction before presave but that’s a big change to make to move that - I’m sure there’s code writing to the db during presave.
Like creating entities on the fly and updating reference fields or similar.
phenaproxima [12:12 PM]
@alexpott For whatever it's worth, I like @catch's approach. Don't require media items to have thumbnails immediately -- in other words, make queue compatibility the default behavior.
alexpott [12:13 PM]
@catch, @phenaproxima so what we could do is override \Drupal\Core\Entity\Entity::save() in media to do something outside the transaction.
so in Media::preSave() we use a placeholder... get the entity saved... and then update the thumbnail.
Determine the best approach, write a patch, and fix it.
Ideally none.
TBD, ideally none.
None anticipated.
Active
11.0 🔥
Last updated
Not all content is available!
It's likely this issue predates Contrib.social: some issue and comment data are missing.
This issue is mentioned in Media::prepareSave():
public function prepareSave() {
// @todo If the source plugin talks to a remote API (e.g. oEmbed), this code
// might be performing a fair number of HTTP requests. This is dangerously
// brittle and should probably be handled by a queue, to avoid doing HTTP
// operations during entity save. See
// https://www.drupal.org/project/drupal/issues/2976875 for more.