Reject ready promise on 403 #264
Conversation
|
@tenacex can you commit a compiled dist/player.js so I can test it? |
|
@wesbos previously we have not committed dist fils. Please run |
|
Ah! Easy okay thanks - will try it |
|
I'm not sure if I have access to https://api-2489.ci.vimeows.com/api/oembed.json?url=https%3A%2F%2Fvimeo.com%2F184581834&domain=localhost&id=184581834&width=640&loop=true It's timing out after about 72 seconds. Good news is that the .catch is catching this network error so I'd say it's working :D thanks for this! |
|
@wesbos ah yes sorry I forgot you would have to be within our VPN. API team told me it would be released this morning so I will hook into it as soon as that happens. |
src/lib/embed.js
Outdated
|
|
||
| // Check api response for 403 on oembed | ||
| if (json['domain_status_code'] === 403) { | ||
|
|
There was a problem hiding this comment.
Remove whitespace.
src/lib/embed.js
Outdated
| @@ -52,13 +52,13 @@ export function getOEmbedParameters(element, defaults = {}) { | |||
| * @param {Object} [params] Parameters to pass to oEmbed. | |||
| * @return {Promise} | |||
|
@dylandove does this also take in account iframe setups? I think the oEmbed data is only used when setting up a div element with the Vimeo.Player constructor or no? |
|
@luwes I am not entirely sure what you mean? In the player.js codebase we get back an iframe element and embed it directly into the dom. I don't think we ever refer to a specific div in this setup. Does that answer your question? |
|
@tenacex Not sure what's going on but videos that were previously embedding fine on my end are now spitting out the new 403 errors instead of loading. The videos I'm referencing have domain specific embed/privacy settings. I fixed by rolling back to 2.6.1 (I was previously using the CDN). This was on a live server so unfortunately I can't do much digging at the moment. Could it be something with this line? https://github.com/vimeo/player.js/blob/master/src/lib/embed.js#L87 |
|
@simplystuart could you please send me the video ID's and I will investigate? Thanks |
|
@tenacex Thanks! Here's one: 152183192 |
|
@simplystuart sorry also could you send me an example domain you are attempting to embed on? |
|
@tenacex classicalacademicpress.com |
|
Hey @simplystuart could you do me a favor and do some debugging on this? Since this error is dependent on behavior on your site, I can't do it just by myself. Could you add a console.log(json) here: https://github.com/vimeo/player.js/blob/master/src/lib/embed.js#L111. Also could you add a console.log(url) here: https://github.com/vimeo/player.js/blob/master/src/lib/embed.js#L88 If you get back to me with those logs I think that will lead me in the right direction. Thanks! |
|
@tenacex sure:
and:
Clarification from earlier... the first video above loads fine, but then when I go to switch videos with |
|
Thanks @simplystuart. There was an issue with our api being case-sensitive even though domains are case insensitive. This is being fixed right now, stand by. |
|
@tenacex any E.T.A. on this? |
|
Hi Stuart have you tried with the most recent build of player.js. It didn’t require a change to our code just the API repo. I believe that has gone out and it should be working for you, but if not let me know and I’ll check with their team. |
|
@tenacex It works! Thanks! |
|
Good to hear! |
This PR gives users the ability to properly detect errors on player instantiation. We detect errors as follows:
var player = new Vimeo.Player('made-in-ny', options); player.ready().catch((error) => { console.log('ERROR', error); });Before this change, a 403 (privacy settings error) wouldn't be caught. Now, we throw the error to allow devs to give their own visual feedback. This currently is using a CI link, which will be updated to production before being merged.
Fixes #165