Skip to content

Commit

Permalink
feat: Enable variant failover for BAD_HTTP_STATUS and TIMEOUT (#4769)
Browse files Browse the repository at this point in the history
This enables #4189 for two more error types. As mentioned in #4728 404
status results in `BAD_HTTP_STATUS` rather than `HTTP_ERROR`. I also
think `TIMEOUT` should be included, and another issue was just created
for this (#4764), so I included that as well.

Closes #4728
Closes #4764
  • Loading branch information
friday authored Dec 1, 2022
1 parent c724625 commit b46012d
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 6 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

Adrián Gómez Llorente <[email protected]>
AdsWizz <*@adswizz.com>
Albin Larsson <[email protected]>
Alex Jones <[email protected]>
Alugha GmbH <*@alugha.com>
Alvaro Velad Galvan <[email protected]>
Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTORS
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Aaron Vaage <[email protected]>
Adrián Gómez Llorente <[email protected]>
Agajan Jumakuliyev <[email protected]>
Aidan Ridley <[email protected]>
Albin Larsson <[email protected]>
Alex Jones <[email protected]>
Alvaro Velad Galvan <[email protected]>
Amila Sampath <[email protected]>
Expand Down
8 changes: 6 additions & 2 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -6043,8 +6043,12 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
* @private
*/
tryToRecoverFromError_(error) {
if ((error.code != shaka.util.Error.Code.HTTP_ERROR &&
error.code != shaka.util.Error.Code.SEGMENT_MISSING) ||
if (![
shaka.util.Error.Code.HTTP_ERROR,
shaka.util.Error.Code.SEGMENT_MISSING,
shaka.util.Error.Code.BAD_HTTP_STATUS,
shaka.util.Error.Code.TIMEOUT,
].includes(error.code) ||
error.category != shaka.util.Error.Category.NETWORK) {
return false;
}
Expand Down
26 changes: 22 additions & 4 deletions test/player_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,10 +345,18 @@ describe('Player', () => {
shaka.util.Error.Severity.RECOVERABLE,
shaka.util.Error.Category.NETWORK,
shaka.util.Error.Code.HTTP_ERROR);
const nonHttpError = new shaka.util.Error(
const badHttpStatusError = new shaka.util.Error(
shaka.util.Error.Severity.RECOVERABLE,
shaka.util.Error.Category.NETWORK,
shaka.util.Error.Code.BAD_HTTP_STATUS);
const timeoutError = new shaka.util.Error(
shaka.util.Error.Severity.RECOVERABLE,
shaka.util.Error.Category.NETWORK,
shaka.util.Error.Code.TIMEOUT);
const operationAbortedError = new shaka.util.Error(
shaka.util.Error.Severity.RECOVERABLE,
shaka.util.Error.Category.NETWORK,
shaka.util.Error.Code.OPERATION_ABORTED);
/** @type {?jasmine.Spy} */
let dispatchEventSpy;

Expand All @@ -371,9 +379,9 @@ describe('Player', () => {
dispatchEventSpy.calls.reset();
});

it('does not handle non NETWORK HTTP_ERROR', () => {
onErrorCallback(nonHttpError);
expect(nonHttpError.handled).toBeFalsy();
it('does not handle non recoverable network error', () => {
onErrorCallback(operationAbortedError);
expect(operationAbortedError.handled).toBeFalsy();
expect(player.dispatchEvent).toHaveBeenCalled();
});

Expand Down Expand Up @@ -440,6 +448,16 @@ describe('Player', () => {
expect(httpError.handled).toBeTruthy();
});

it('handles BAD_HTTP_STATUS', () => {
onErrorCallback(badHttpStatusError);
expect(badHttpStatusError.handled).toBeTruthy();
});

it('handles TIMEOUT', () => {
onErrorCallback(timeoutError);
expect(timeoutError.handled).toBeTruthy();
});

it('does not dispatch any error', () => {
onErrorCallback(httpError);
expect(dispatchEventSpy).not.toHaveBeenCalled();
Expand Down

0 comments on commit b46012d

Please sign in to comment.