Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Please check your URL" bug is back? #129

Closed
bartrylant opened this issue Dec 13, 2022 · 20 comments
Closed

"Please check your URL" bug is back? #129

bartrylant opened this issue Dec 13, 2022 · 20 comments

Comments

@bartrylant
Copy link

bartrylant commented Dec 13, 2022

I noticed this error comes back quite a lot (many issues on this github). So here we go again:

Suddenly, out embedded YouTube video's didn't show up anymore. The render code just displays a white rectangle. This is what I found so far:

  • When pasting a YouTube url in the oEmbed field, it says Please check your URL.
  • When saving the entry anyway, the url gets saved, but the oembed object = null
  • The entry.field.media.providerName is set to google

Not sure what is going on here but it's extremely annoying. It causes my clients website - which uses YouTube embeds quite a lot - to not display any embedded video from time to time. I have no idea what causes this. This all happens on production, not on dev. And when I embed the same video hardcoded (i.e. just use the iframe code generated by YouTube), the video displays without any problem. So it's an issue when converting the pasted url (in the field) to rendering.

PHP version | 8.1.12
Craft Pro 4.3.4
oEmbed 2.2.2

@denisyilmaz
Copy link

denisyilmaz commented Jan 11, 2023

same issue here. I got it working by disabling the cache on production (was disabled on dev by default).

/admin/oembed/preview?url=https://issuu.com/iscience/docs/issue23 as an example is failing when cache is on. works with cache disabled.

@kelenakamura
Copy link

@denisyilmaz what cache did you disable?

@denisyilmaz
Copy link

@kelenakamura
inside config/oembed.php you can define if oembed should cache the responses for the embeds (which is causing this issue):

<?php
return [
    '*' => [
        'enableGdpr' => true,
        'enableNotifications' => false,
        'enableCache' => false,
    ],
    'production' => [
        'enableCache' => true, // set this to false if the caching issue appears
    ],
];

@kelenakamura
Copy link

@denisyilmaz thanks for quick response. yes strange we never had it enabled on staging or prod, but still have the same issue.

@reganlawton
Copy link
Member

I think it's time for me to revisit the embed package upgrade

@kelenakamura
Copy link

kelenakamura commented Oct 23, 2023

we found the issue - it was a php curl issue - we were getting a curl 77 error - so following this fixed it - guzzle/guzzle#1935 (comment) - or could have been just restarting php-fpm....

@denisyilmaz
Copy link

@kelenakamura we run into that issue now ourself (even with disabled cache) and the newest oembed plugin version. Restarting php-fpm did not work.

@yoannisj
Copy link

yoannisj commented Dec 6, 2023

hi @reganlawton ! I work with @denisyilmaz and it looks like this is related to some oEmbed providers not returning an <iframe> in the HTML by default –in our case Issuu.com.

In those cases, a "Call to a member function getAttribute() on null" error is thrown https://github.com/wrav/oembed/blob/master/src/services/OembedService.php#L121 but that error is not handled and we don't get a FallbackAdapter.


The Issuu.com oEmbed endpoint supports an iframe=true query argument which forces it to return the embed code as an <iframe>. Looking at the source code (here and here) it seems I should be able to pass that query argument to the Issuu oEmbed endpoint like so:

{% set media = craft.oembed.embed(url, {
    `oembed:query_parameters`: {
        iframe: true,
     }, 
 }) %}
 
 {{ media.code |raw }} {# AFAIK this should now include an iframe #}

However, that does not work and I can't figure out why...

To my greatest surprise: I get an empty array when I dump $this->extractor->getSettings() here, although I can see the 'oembed:query_parameters' option when I dump $infos->getSettings() here).

@reganlawton reganlawton reopened this Dec 6, 2023
@reganlawton
Copy link
Member

reganlawton commented Dec 6, 2023

@yoannisj and @denisyilmaz I believe this is the issue from #141 where the TRY CATCH was getting the wrong exception and I wasn't fallback.

If you can try v2.3.2 Im just about to tag it. You can also use v2.x-dev

@yoannisj
Copy link

yoannisj commented Dec 7, 2023

Hmmm, thanks but we are currently running v3.0.3 which already catches the correct/broader exception type, so I don't think it is related.

@yoannisj
Copy link

yoannisj commented Dec 7, 2023

@reganlawton can you confirm that passing a map with key 'oembed:query_parametersas 2nd argument$options.oembed:query_parameterskey forwrav\embed\OembedService::embed($url, $options)` should include that map as query parameters to the oembed endpoint?

If that's the expected behaviour, it does not seem to be working. Any idea?

@reganlawton
Copy link
Member

@yoannisj it should pass it though yes. Do you have a URL I can use to test. It's currently bed time but I can look into it tomorrow morning.

@yoannisj
Copy link

yoannisj commented Dec 7, 2023

Of course, this is the URL I have been testing with: https://issuu.com/deignismedia/docs/64_deignis_magazin_web. Thanks, and have a good night!

@reganlawton
Copy link
Member

Awesome thanks for that @yoannisj that'll great help my testing.

Fingers crossed we have this resolved tomorrow 🤞

@reganlawton
Copy link
Member

@yoannisj Seems the embed/embed v3.x doesn't issuu.com, I can definitely resolved the issue in v3.0.5. So seeing your using the news version track (v3) you should be fine.

Im about to tag it so please update and let me know. This is the code I used to test.

{{
    craft.oembed.render(
        'https://issuu.com/deignismedia/docs/64_deignis_magazin_web',
    )
}}

@yoannisj
Copy link

yoannisj commented Dec 13, 2023

@reganlawton did you manage to pass on query parameters to the oEmbed endpoint called by embed/embed using the 2nd argument for the wrav\embed\OembedService::embed($url, $options) method?

We are retrieving the oEmbed information via GraphQL, which uses the wrav/oembed field value (i.e. craft\base\Element::getFieldValue('oembedFieldHandle'), which in turn relies on wrav\embed\OembedService::embed() so we would need a way to modify the underlying oembed request.

How would you feel about triggering an event in the wrav\embed\OembedService::embed method so custom modules/plugins can modify the options given to embed/embed programmatically? That sounds very useful in case other oEmbed services (not just issuu) rely on query arguments to return iframes in the oembed html code.

Note that Issuu used to work for us, and stopped so I suspect they changed the behaviour of their oEmbed endpoint and that could happen to other services in the future. An event could mitigate this risk, and provide a way for projects to fix it without depending on a new wrav/oembed release.

@reganlawton
Copy link
Member

@yoannisj the code I provided in previous comment worked as intended without need for params / options.

@reganlawton
Copy link
Member

Try using the latest version if you haven't already

@reganlawton
Copy link
Member

reganlawton commented Dec 13, 2023

@yoannisj Oh shit sorry I completely over looked the GQL bit 😅 I'll have to test that.

As for adding custom adapters, there is definitely a way I can do that I just haven't cos it's more code I have to manage. If creating a custom adapter based on options is something you'd like me to look into I definitely can. I would probably allow you to be able pass a class that would then be able to be used given you ability to completely customise your code.

@reganlawton
Copy link
Member

@yoannisj Lots of updates out with v3.0.6 this should fix issues with GraphQL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants