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

bkosborne created an issue. See original summary.

bkosborne’s picture

Issue summary: View changes
Sam152’s picture

Actually, additions are allowed to interfaces, see Drupal 8 backwards compatibility and internal API policy.

Interfaces follow a similar pattern as above with respect to @api, @internal, or neither. However, in case of neither tag, the interface is treated as an API for callers but not for implementers.

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.

Sam152’s picture

Status: Active » Closed (won't fix)

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