Skip to content
New issue

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

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

Already on GitHub? # to your account

Live reload level switch and error handling improvements #5317

Merged
merged 1 commit into from
Mar 22, 2023

Conversation

robwalch
Copy link
Collaborator

This PR will...

  • Maintain live reload pace after level switch
  • Only define deliveryDirectives when when blocking reload is available (correct error handling)
  • Retry main playlist on status 0 when navigator.onLine is false (this policy is not extended to audio and subtitle track media playlists)

Why is this Pull Request needed?

On level switch the reload interval was being added to the already scheduled time for next reload, which could skip reloads after level switch.

Related issues:

#5282
#5306

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md

- Maintain reload pace after level switch
- Only define deliveryDirectives when when blocking reload is available (correct error handling)
- Retry on status 0 when navigator.onLine is false
@kevleyski
Copy link
Collaborator

Quick test with 3 ABR variants seems ok for me with canBlockReload

@robwalch robwalch merged commit 98b38ab into master Mar 22, 2023
@robwalch robwalch deleted the bugfix/live-reload-switch-and-http-status-0 branch March 22, 2023 00:52
robwalch added a commit that referenced this pull request Aug 31, 2023
@robwalch robwalch mentioned this pull request Sep 1, 2023
@@ -61,5 +61,8 @@ export function shouldRetry(

export function retryForHttpStatus(httpStatus: number | undefined) {
// Do not retry on status 4xx, status 0 (CORS error), or undefined (decrypt/gap/parse error)
return !!httpStatus && (httpStatus < 400 || httpStatus > 499);
return (
(httpStatus === 0 && navigator.onLine === false) ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @robwalch I recent meet this issue. I don't get the point. Are we going to retry under user's good network? I means

(httpStatus === 0 && navigator.onLine)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It means retry later when network is disabled or completely unavailable.

A status of 0 where online equals true is typically a CORS error and should not be retried.

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

Successfully merging this pull request may close these issues.

4 participants