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

fix(FEC-11734): player error handling works incorrect #171

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yairans
Copy link
Contributor

@yairans yairans commented Dec 6, 2021

Description of the Changes

This is a shaka issue - shaka-project/shaka-player#3794
As a WA - set the error severity to critical when it's HTTP_ERROR

Solves FEC-11734

CheckLists

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • test are passing in local environment
  • Travis tests are passing (or test results are not worse than on master branch :))
  • Docs have been updated

@yairans yairans requested a review from a team December 6, 2021 11:44
@yairans yairans self-assigned this Dec 6, 2021
@OrenMe
Copy link
Collaborator

OrenMe commented Dec 6, 2021

@yairans the proper way to handle this for Shaka, which is basically what you did in our code is to use failureCallback method in streaming config.
It is connected to the deprecated option shaka once had called infiniteRetriesForLiveStreams which from the name you can understand what is the default behaviour for shaka for live manifests reloads and why it internally sets this error to non fatal.

this is the default handler for them and you can see it sets for live a few errors to be non fatal https://github.com/google/shaka-player/blob/master/lib/player.js#L4960

@yairans
Copy link
Contributor Author

yairans commented Dec 6, 2021

@OrenMe The question is why this behavior is only in SegmentTimeline and not in SegmentTemplate?
see the shaka issue I opened for more details.
anyway, I tested the failureCallback config and it doesn't work, seems because the error is not caught as I wrote in the shaka ticket.

@yairans yairans closed this Dec 6, 2021
@yairans yairans reopened this Dec 6, 2021
@OrenMe
Copy link
Collaborator

OrenMe commented Dec 6, 2021

so this will happen also for VOD if we use SegmentTimeline? although in VOD manifest won't ever reload so not a real issue.

I just suspect doing this in error handler for all playback types might be an issue cause this will cause any HTTP error to get us into critical error, meaning error handling will be void

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

Successfully merging this pull request may close these issues.

None yet

2 participants