Many video providers interact with an external API to retrieve data about a video, like thumbnails. If there's a failure in obtaining that required information, like if the video doesn't exist or the HTTP call fails for some reason, there's nothing a video provider can do to properly communicate that issue. The only recourse is to throw a fatal error and log something.
Seems like there's a better way to handle errors in a graceful way. I think ideally each provider would have a "validate" method which would be used to validate that the input can be used. We already have the static isApplicable
method, but I think this is just an appropriate place to do text string analysis of the input. The "validate" method can do HTTP calls to make sure proper responses are returned.
Then, a field constraint can be added to the VideoEmbedField
field which runs this validation logic for the selected provider.
The main problem I see is that this would be an API break, because you'd need to add a method to the interface. You could a default implementation in the abstract class but I don't think you can assume that every contrib provider extends the abstract base implementation.
Comments
Comment #2
bkosborneComment #3
Sam152 CreditAttribution: Sam152 as a volunteer commentedActually, additions are allowed to interfaces, see Drupal 8 backwards compatibility and internal API policy.
Long story short, we can add methods but not remove existing ones. Also, this is a case where I can't see someone going it alone and implementing the interface directly.
With regards to the issue itself, I like it! Once isApplicable has decided a URL matches the plugin, being given the opportunity to provide per-provider validations seems like a good idea.
Comment #4
Sam152 CreditAttribution: Sam152 as a volunteer commentedWith the advent of Media in core, the Video Embed Field module has moved to being minimally maintained. Only issues which assist in the migration to Media in core will be committed. To read more about this decision, please see: #3089599: Maintenance status for Video Embed Field now that media is in core.