From b46012df647d0fd6f1b6209a324171ab86f9fa80 Mon Sep 17 00:00:00 2001 From: Albin Larsson Date: Thu, 1 Dec 2022 22:16:58 +0100 Subject: [PATCH] feat: Enable variant failover for BAD_HTTP_STATUS and TIMEOUT (#4769) 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 https://github.com/shaka-project/shaka-player/issues/4728 Closes https://github.com/shaka-project/shaka-player/issues/4764 --- AUTHORS | 1 + CONTRIBUTORS | 1 + lib/player.js | 8 ++++++-- test/player_unit.js | 26 ++++++++++++++++++++++---- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/AUTHORS b/AUTHORS index 9b701a38d1..1db255c8bc 100644 --- a/AUTHORS +++ b/AUTHORS @@ -15,6 +15,7 @@ Adrián Gómez Llorente AdsWizz <*@adswizz.com> +Albin Larsson Alex Jones Alugha GmbH <*@alugha.com> Alvaro Velad Galvan diff --git a/CONTRIBUTORS b/CONTRIBUTORS index aa611ec22e..4488e1ffbc 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -26,6 +26,7 @@ Aaron Vaage Adrián Gómez Llorente Agajan Jumakuliyev Aidan Ridley +Albin Larsson Alex Jones Alvaro Velad Galvan Amila Sampath diff --git a/lib/player.js b/lib/player.js index 4d6e20e5c3..24db39ccdc 100644 --- a/lib/player.js +++ b/lib/player.js @@ -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; } diff --git a/test/player_unit.js b/test/player_unit.js index 1e8171a710..db5db5d874 100644 --- a/test/player_unit.js +++ b/test/player_unit.js @@ -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; @@ -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(); }); @@ -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();