Handling media element errors

[danielr18]

We added track error handling on #352 however media element errors aren't being handled, which isn't a big deal since you can create your own media element with createMediaElement prop, however I think the library might benefit handling one error that can easily happen and gives a bad user experience, which is that when internet connection isn't very stable, the player might not play again until you load another track.

Steps to reproduce in Chrome (mobile/desktop):

• Play a track on https://benwiley4000.github.io/cassette/styleguide/.
• Take your browser offline and wait until no more requests are made (or media element throws an error).
• Get back online and try to play, and track won't play again.
• The media element should have the following error: {code: 2, message: "PIPELINE_ERROR_READ: FFmpegDemuxer: data source error"}.

Possible solutions within library:

• Adding a onPlaybackFailure prop, something similar to onTrackPlaybackFailure but for media element errors, and making mediaCannotPlay true or another boolean if it's more appropriate.
• Automatically trying to get media element out of the error state by reloading the track. Since we support passing a startingTime, if a network error is thrown for example, we could reload and seek to the time you were on as if nothing had happened, however startingTime is set in the playlist, so I'm not sure that can be used. This also bring questions like which errors would try to be solved by reloading, should max retries be set and others you might have.

Would like to know your thoughts and can help you making the PR if we agree to do something!

[benwiley4000]

Hmmmm this is a good thing to tackle. Is the currentTime unset before the error is thrown? Because if not we could just save it before reloading the track. I am not sure about max retries.. it seems like something that you would not want to retry until you are actually back online, right?

I am wondering if we would want to just reuse the onTrackPlaybackFailure callback, if we wanted to handle it that way. Having two callbacks seems confusing and I'm not sure how great the benefit would be of separating them.

What do you think?

[danielr18]

Hmmmm this is a good thing to tackle. Is the currentTime unset before the error is thrown? Because if not we could just save it before reloading the track.

Yes, the currentTime is still set, so we can do that.

I am not sure about max retries.. it seems like something that you would not want to retry until you are actually back online, right?

Yes, retrying when you are back online makes sense for network errors, the max retries was related to a non network error like MEDIA_ERR_DECODE (if we wanted to reload if that error is ever thrown).

I am wondering if we would want to just reuse the onTrackPlaybackFailure callback, if we wanted to handle it that way. Having two callbacks seems confusing and I'm not sure how great the benefit would be of separating them.

What do you think?

This is a good idea, guess you can use event.target to check if the error comes from a source or the media element.

[benwiley4000]

I think we could justify retrying playback if the cause of failure was the network, but a retry makes less sense for a decode failure since I would assume that failure would occur again even if we retry. Same for unsupported source. For the abort error, it's usually meaningless and I would ignore it.

Le lun. 3 juin 2019 16 h 49, Daniel Reinoso notifications@github.com a écrit :

Hmmmm this is a good thing to tackle. Is the currentTime unset before the error is thrown? Because if not we could just save it before reloading the track.

Yes, the currentTime is still set, so we can do that.

I am not sure about max retries.. it seems like something that you would not want to retry until you are actually back online, right?

Yes, retrying when you are back online makes sense for network errors, the max retries was related to a non network error like MEDIA_ERR_DECODE http://w3c.github.io/html/semantics-embedded-content.html#dom-mediaerror-media_err_decode (if we wanted to reload if that error is ever thrown).

I am wondering if we would want to just reuse the onTrackPlaybackFailure callback, if we wanted to handle it that way. Having two callbacks seems confusing and I'm not sure how great the benefit would be of separating them.

What do you think?

This is a good idea, guess you can use event.target to check if the error comes from a source or the media element.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/benwiley4000/cassette/issues/425?email_source=notifications&email_token=ADHOD3NXAGVHJL3NE64MA2LPYV7VZA5CNFSM4HSRFD22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW2U3ZQ#issuecomment-498421222, or mute the thread https://github.com/notifications/unsubscribe-auth/ADHOD3L3V7AUY3PCLBH3COTPYV7VZANCNFSM4HSRFD2Q .

[danielr18]

Yes, I agree we should just retry on network error.

[benwiley4000]

Here's what my approach would be:

1. Include these errors in the playback failure callback (assume the user will handle them appropriately and not throw an error in every case)
2. If we have a network failure, although we should still emit this event, we should also try to recover: a. Save the necessary play state (maybe? do we need to save anything?) and set an interval (not sure the duration) to continue checking if we're online. Once we are then retry playing (once). b. Avoid setting the paused state automatically to true c. If the user initiates a pause action, cancel the interval

[benwiley4000]

If you want to try a PR for this I would be glad to review and and try to get it released in the beta!