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

No way to catch errors on instantiation #165

Closed
rijk opened this issue Sep 25, 2017 · 31 comments
Closed

No way to catch errors on instantiation #165

rijk opened this issue Sep 25, 2017 · 31 comments
Assignees
Labels
bug Something isn't working

Comments

@rijk
Copy link

rijk commented Sep 25, 2017

This was first mentioned in #57, but note that it is a separate issue hence the new ticket.

When instantiating a player with new Vimeo.Player() with a URL that triggers a 404 (not found) or 403 (privacy settings) error, there is no way to catch this error.

Expected Behavior

player = new Vimeo.Player( ... ) // with private video ID
player.on( 'error', ... )

Error handler is called.

Actual Behavior

Error handler is not called.

Steps to Reproduce

See above


This makes for pretty terrible UX when the player is embedded in a webapp. The user gets no visual feedback from the player; it just doesn't load. And since no error handler is called it's also impossible to give feedback via the app interface about it.

The only way is around it is to use loadVideo, which does give you a way to intercept errors (although this doesn't work sometimes either, that's what #57 is about). The problem here is, there is no way to create a player without first setting a video url and therefore loading it, as the player on instantiation checks the iframe for a valid source and blows up if it doesn't find one. So we have to do a workaround of initially set a url to a small dummy video to get the player instantiated, and then call loadVideo to load the actual video and check for errors.

Please give us some way to catch errors during instantiation. 🙏

@rijk rijk changed the title Error handler not called on instantiation No way to catch errors on instantiation Sep 25, 2017
@rijk
Copy link
Author

rijk commented Sep 25, 2017

That workaround of loading a blank video doesn't work for me either; after that the duration is fixed at 1 sec (at least in the react-player library I'm using). So please please fix this :)

@LoveIsGrief
Copy link
Contributor

LoveIsGrief commented Sep 30, 2017

Hmm... something else is working for me (don't know any private videos, so just passed an invalid id)

player = new Vimeo.Player( ... ).ready().catch(...)

Technically, it's catching at instantiation 🤔

@rijk
Copy link
Author

rijk commented Oct 2, 2017

That works, awesome! Even though I had to change it slightly to:

player = new Vimeo.Player( ... )
player.ready().catch(...)

Otherwise the player variable would not be a reference to the actual player in case of a successful instantiation.

Unfortunately, the error you receive here does not contain any status codes, the only thing you have is the message, which is variable as well (“https://vimeo.com/...” is not embeddable.). So you'd be stuck having to do a regex check on the error message (and pray they never change it) facepalm

What I do now is 'simply' call the API myself, and check the HTTP status code of the return.

@LoveIsGrief
Copy link
Contributor

What I do now is 'simply' call the API myself, and check the HTTP status code of the return.

That's a good workaround I hadn't thought of! 👍 I'll use that in my code until this issue is resolved.

@luwes luwes added the bug Something isn't working label Mar 2, 2018
@fredlee0109
Copy link

fredlee0109 commented Mar 22, 2018

403 videos are no longer caught using .ready() even though code says it would https://github.com/vimeo/player.js/blob/master/src/lib/embed.js#L69-L76 ... major :(
https://jsfiddle.net/Lk2zycn8/25/
chrome console: player.js:2 GET https://player.vimeo.com/video/234555653?app_id=122963 403 (Forbidden)

@wesbos
Copy link

wesbos commented Apr 9, 2018

I'm getting this too - I can't find a way to detect if the user is blocking my private videos with Privacy Badger

Anyone have some ideas?

@fredlee0109
Copy link

@wesbos for now, you can request HTTP header of https://player.vimeo.com/video/... and check status code
ex: curl -I https://player.vimeo.com/video/234555653

@fisherinnovation
Copy link
Contributor

Thanks for the feedback everyone. We are actively working to solve this issue. Thanks for your patience!

@wesbos
Copy link

wesbos commented Apr 9, 2018

Is that header supposed to be caught anywhere? I’m not able to catch this error

@tenacex tenacex self-assigned this Apr 10, 2018
@tenacex
Copy link
Contributor

tenacex commented Apr 18, 2018

After checking this out and playing around, I don't think the way described in the issue ticket will work:

player = new Vimeo.Player( ... ) // with private video ID
player.on( 'error', ... )

Even if I dispatch an error event on the player object when we 404 or 403, we don't catch it because we error before we ever attach the event handler.

I think the best way to handle this will be to use the ready() promise. I am going to adjust it to give more detailed information on whether it is a 404 or 403 so that you can handle the issues accordingly.

@wesbos
Copy link

wesbos commented May 22, 2018

Any update on this? I'm getting 5-10 emails a day where users complain about the privacy error and I have no current way to alert them that it's their privacy badger extension.

@tenacex
Copy link
Contributor

tenacex commented May 22, 2018

Hey Sorry @wesbos I had almost finished working on this and got caught up in something else and forgot to wrap it up. I'll have this hopefully by EOD, and if not by tomorrow early on in the day.

@wesbos
Copy link

wesbos commented May 22, 2018

😘 thank you!

@fredlee0109
Copy link

@tenacex thank you! +1

@tenacex
Copy link
Contributor

tenacex commented May 23, 2018

Still working on this guys, ran into some issues with my old implementation, but regardless this will be finished soon. Will ping the channel when ready.

@tenacex
Copy link
Contributor

tenacex commented May 23, 2018

@wesbos and @fredlee0109 Sorry for the delay. I totally spaced about having my CORS browser plugin enabled when I wrote the fix for this. Unfortunately, there is no possible way to fix this until the api team builds in the functionality. I reached out to them and they assured me that they will implement the feature that I need to patch this by next week, so I will let you know when thats up so that I can post my fix. Sorry for jumping the gun on this, I know you have been waiting for a while.

@wesbos
Copy link

wesbos commented May 23, 2018

thanks for the update - looking forward to getting this implemented 👌

@wesbos
Copy link

wesbos commented May 29, 2018

Any movement on this?

@tenacex
Copy link
Contributor

tenacex commented May 29, 2018

@wesbos API team has it in their weekly pipeline so just waiting on them! Thanks for checking in.

@Silviu-Marian
Copy link

here is a private video id for testing. using through react-player I can confirm the error event never fires

https://player.vimeo.com/video/196716639

@tenacex
Copy link
Contributor

tenacex commented May 31, 2018

@wesbos API team has it in CI now, testing it. Should have something out tomorrow.

@wesbos
Copy link

wesbos commented May 31, 2018

:D Thanks! CC @lfades

@tenacex
Copy link
Contributor

tenacex commented Jun 4, 2018

Monday update: PR for API is still in PR. Once that is out this can go out as well.

@Silviu-Marian
Copy link

Still in PR?

@tenacex
Copy link
Contributor

tenacex commented Jun 13, 2018

Hey guys, the fix is out in: https://github.com/vimeo/player.js/releases/tag/v2.6.2

Sorry for the delay. If you have any questions, please don't hesitate to reach out!

@Silviu-Marian
Copy link

Guys thank you for the fix

One more question though

image

onError does not fire in this situations. Is this intended or are there other events fired when a video cannot be played for any reason?

@Silviu-Marian
Copy link

pen for the above https://codepen.io/anon/pen/rKJPdX

@tenacex
Copy link
Contributor

tenacex commented Jun 20, 2018

@Silviu-Marian this is not intended and we are aware. However, it may take some time to be addressed given our current priorities on the player. If its possible to do without an API change, I'm happy to review and approve any PR around this. if not, I will try and get to it when our workload dies down!

@jimkeecn
Copy link

player.ready().catch(...) I tried to pass in one invalid id, this function will get keep calling and until the browser hanging. Is it anyway while in the catch function destroy the player?

@LoveIsGrief
Copy link
Contributor

Open a new bug @jimkeecn . It'll be more visible

@bomber555
Copy link

thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests