From d927cec58921928da9a82be8234ac091e3f18322 Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Tue, 1 Nov 2022 11:12:22 -0700 Subject: [PATCH] fix: Allow overriding special handling of 404s (#4635) In general, streaming.failureCallback is meant to give applications control over error handling at the level of streaming. However, there was a special case for HTTP 404s built into StreamingEngine in a way that applications could not override. This was in spite of the fact that the default failureCallback would already check for and retry on the error code BAD_HTTP_STATUS. This removes the special case in StreamingEngine and refactors failureCallback and retryStreaming to preserve the special delay imposed in the old 404 handler. With this, applications can override failureCallback to have complete control over 404 handling. Closes #4548 --- lib/media/streaming_engine.js | 15 +--- lib/player.js | 30 +++++--- test/media/streaming_engine_integration.js | 13 ++++ test/media/streaming_engine_unit.js | 31 ++++++-- test/player_unit.js | 87 ++++++++++++++++++++++ 5 files changed, 146 insertions(+), 30 deletions(-) diff --git a/lib/media/streaming_engine.js b/lib/media/streaming_engine.js index 24e309cfc2..f6fe19bf42 100644 --- a/lib/media/streaming_engine.js +++ b/lib/media/streaming_engine.js @@ -1366,16 +1366,6 @@ shaka.media.StreamingEngine = class { this.mediaStates_.delete(ContentType.TEXT); } else if (error.code == shaka.util.Error.Code.QUOTA_EXCEEDED_ERROR) { this.handleQuotaExceeded_(mediaState, error); - } else if (error.code == shaka.util.Error.Code.BAD_HTTP_STATUS && - error.data && error.data[1] == 404) { - // The segment could not be found, does not exist, or is not available. - // In any case just try again. - // The current segment is not available. Schedule another update to - // fetch the segment again. - shaka.log.v2(logPrefix, 'segment not available.'); - mediaState.performingUpdate = false; - mediaState.updateTimer = null; - this.scheduleUpdate_(mediaState, 1); } else { shaka.log.error(logPrefix, 'failed fetch and append: code=' + error.code); @@ -1390,9 +1380,10 @@ shaka.media.StreamingEngine = class { /** * Clear per-stream error states and retry any failed streams. + * @param {number} delaySeconds * @return {boolean} False if unable to retry. */ - retry() { + retry(delaySeconds) { if (this.destroyer_.destroyed()) { shaka.log.error('Unable to retry after StreamingEngine is destroyed!'); return false; @@ -1409,7 +1400,7 @@ shaka.media.StreamingEngine = class { if (mediaState.hasError) { shaka.log.info(logPrefix, 'Retrying after failure...'); mediaState.hasError = false; - this.scheduleUpdate_(mediaState, 0.1); + this.scheduleUpdate_(mediaState, delaySeconds); } } diff --git a/lib/player.js b/lib/player.js index 69f1eef4c0..93a4a92802 100644 --- a/lib/player.js +++ b/lib/player.js @@ -4958,12 +4958,13 @@ shaka.Player = class extends shaka.util.FakeEventTarget { * If the player has loaded content, and streaming seen an error, but the * could not resume streaming, this will return false. * + * @param {number=} retryDelaySeconds * @return {boolean} * @export */ - retryStreaming() { + retryStreaming(retryDelaySeconds = 0.1) { return this.loadMode_ == shaka.Player.LoadMode.MEDIA_SOURCE ? - this.streamingEngine_.retry() : + this.streamingEngine_.retry(retryDelaySeconds) : false; } @@ -5059,17 +5060,26 @@ shaka.Player = class extends shaka.util.FakeEventTarget { * @private */ defaultStreamingFailureCallback_(error) { - const retryErrorCodes = [ - shaka.util.Error.Code.BAD_HTTP_STATUS, - shaka.util.Error.Code.HTTP_ERROR, - shaka.util.Error.Code.TIMEOUT, - ]; + // For live streams, we retry streaming automatically for certain errors. + // For VOD streams, all streaming failures are fatal. + if (!this.isLive()) { + return; + } - if (this.isLive() && retryErrorCodes.includes(error.code)) { - error.severity = shaka.util.Error.Severity.RECOVERABLE; + let retryDelaySeconds = null; + if (error.code == shaka.util.Error.Code.BAD_HTTP_STATUS || + error.code == shaka.util.Error.Code.HTTP_ERROR) { + // These errors can be near-instant, so delay a bit before retrying. + retryDelaySeconds = 1; + } else if (error.code == shaka.util.Error.Code.TIMEOUT) { + // We already waited for a timeout, so retry quickly. + retryDelaySeconds = 0.1; + } + if (retryDelaySeconds != null) { + error.severity = shaka.util.Error.Severity.RECOVERABLE; shaka.log.warning('Live streaming error. Retrying automatically...'); - this.retryStreaming(); + this.retryStreaming(retryDelaySeconds); } } diff --git a/test/media/streaming_engine_integration.js b/test/media/streaming_engine_integration.js index 1221383f97..a394cfb2b7 100644 --- a/test/media/streaming_engine_integration.js +++ b/test/media/streaming_engine_integration.js @@ -154,6 +154,19 @@ describe('StreamingEngine', () => { /* presentationDuration= */ Infinity); setupPlayhead(); + // Retry on failure for live streams. + config.failureCallback = () => streamingEngine.retry(0.1); + + // Ignore 404 errors in live stream tests. + onError.and.callFake((error) => { + if (error.code == shaka.util.Error.Code.BAD_HTTP_STATUS && + error.data[1] == 404) { + // 404 error + } else { + fail(error); + } + }); + createStreamingEngine(); } diff --git a/test/media/streaming_engine_unit.js b/test/media/streaming_engine_unit.js index f7b1327c97..4daa53138c 100644 --- a/test/media/streaming_engine_unit.js +++ b/test/media/streaming_engine_unit.js @@ -857,7 +857,7 @@ describe('StreamingEngine', () => { const config = shaka.util.PlayerConfiguration.createDefault().streaming; config.bufferingGoal = 60; - config.failureCallback = () => streamingEngine.retry(); + config.failureCallback = () => streamingEngine.retry(0.1); createStreamingEngine(config); // Make requests for different types take different amounts of time. @@ -1716,8 +1716,23 @@ describe('StreamingEngine', () => { beforeEach(() => { setupLive(); mediaSourceEngine = new shaka.test.FakeMediaSourceEngine(segmentData, 0); - createStreamingEngine(); + + // Retry on failure for live streams. + const config = shaka.util.PlayerConfiguration.createDefault().streaming; + config.failureCallback = () => streamingEngine.retry(0.1); + + createStreamingEngine(config); presentationTimeInSeconds = 100; + + // Ignore 404 errors in live stream tests. + onError.and.callFake((error) => { + if (error.code == shaka.util.Error.Code.BAD_HTTP_STATUS && + error.data[1] == 404) { + // 404 error + } else { + fail(error); + } + }); }); it('outside segment availability window', async () => { @@ -1753,9 +1768,9 @@ describe('StreamingEngine', () => { if (startTime >= 100) { // Ignore a possible call for the first Period. expect(Util.invokeSpy(timeline.getSegmentAvailabilityStart)) - .toBe(100); + .not.toBeLessThan(100); expect(Util.invokeSpy(timeline.getSegmentAvailabilityEnd)) - .toBe(120); + .not.toBeLessThan(120); playing = true; mediaSourceEngine.appendBuffer.and.callFake( originalAppendBuffer); @@ -1980,7 +1995,7 @@ describe('StreamingEngine', () => { mediaSourceEngine = new shaka.test.FakeMediaSourceEngine(segmentData); const config = shaka.util.PlayerConfiguration.createDefault().streaming; - config.failureCallback = () => streamingEngine.retry(); + config.failureCallback = () => streamingEngine.retry(0.1); createStreamingEngine(config); presentationTimeInSeconds = 100; @@ -2141,7 +2156,7 @@ describe('StreamingEngine', () => { netEngine.request.calls.reset(); // Retry streaming. - expect(streamingEngine.retry()).toBe(true); + expect(streamingEngine.retry(0.1)).toBe(true); }); // Here we go! @@ -2171,7 +2186,7 @@ describe('StreamingEngine', () => { // Retry streaming, which should fail and return false. netEngine.request.calls.reset(); - expect(streamingEngine.retry()).toBe(false); + expect(streamingEngine.retry(0.1)).toBe(false); }); // Here we go! @@ -2223,7 +2238,7 @@ describe('StreamingEngine', () => { // Retry streaming, which should fail and return false. netEngine.request.calls.reset(); - expect(streamingEngine.retry()).toBe(false); + expect(streamingEngine.retry(0.1)).toBe(false); } }); diff --git a/test/player_unit.js b/test/player_unit.js index 23fd35c3a2..2650bf7937 100644 --- a/test/player_unit.js +++ b/test/player_unit.js @@ -3735,6 +3735,93 @@ describe('Player', () => { }); }); + describe('config streaming.failureCallback default', () => { + /** @type {jasmine.Spy} */ + let retryStreaming; + /** @type {jasmine.Spy} */ + let isLive; + + /** + * @suppress {accessControls} + * @param {!shaka.util.Error} error + */ + function defaultStreamingFailureCallback(error) { + player.defaultStreamingFailureCallback_(error); + } + + beforeEach(() => { + retryStreaming = jasmine.createSpy('retryStreaming'); + isLive = jasmine.createSpy('isLive'); + + player.retryStreaming = Util.spyFunc(retryStreaming); + player.isLive = Util.spyFunc(isLive); + }); + + it('ignores VOD failures', () => { + isLive.and.returnValue(false); + + const error = new shaka.util.Error( + shaka.util.Error.Severity.CRITICAL, + shaka.util.Error.Category.NETWORK, + shaka.util.Error.Code.BAD_HTTP_STATUS, + /* url= */ '', + /* http_status= */ 404); + + defaultStreamingFailureCallback(error); + expect(retryStreaming).not.toHaveBeenCalled(); + }); + + it('retries live on HTTP 404', () => { + isLive.and.returnValue(true); + + const error = new shaka.util.Error( + shaka.util.Error.Severity.CRITICAL, + shaka.util.Error.Category.NETWORK, + shaka.util.Error.Code.BAD_HTTP_STATUS, + /* url= */ '', + /* http_status= */ 404); + + defaultStreamingFailureCallback(error); + expect(retryStreaming).toHaveBeenCalled(); + }); + + it('retries live on generic HTTP error', () => { + isLive.and.returnValue(true); + + const error = new shaka.util.Error( + shaka.util.Error.Severity.CRITICAL, + shaka.util.Error.Category.NETWORK, + shaka.util.Error.Code.HTTP_ERROR); + + defaultStreamingFailureCallback(error); + expect(retryStreaming).toHaveBeenCalled(); + }); + + it('retries live on HTTP timeout', () => { + isLive.and.returnValue(true); + + const error = new shaka.util.Error( + shaka.util.Error.Severity.CRITICAL, + shaka.util.Error.Category.NETWORK, + shaka.util.Error.Code.TIMEOUT); + + defaultStreamingFailureCallback(error); + expect(retryStreaming).toHaveBeenCalled(); + }); + + it('ignores other live failures', () => { + isLive.and.returnValue(true); + + const error = new shaka.util.Error( + shaka.util.Error.Severity.CRITICAL, + shaka.util.Error.Category.MEDIA, + shaka.util.Error.Code.VIDEO_ERROR); + + defaultStreamingFailureCallback(error); + expect(retryStreaming).not.toHaveBeenCalled(); + }); + }); + /** * Gets the currently active variant track. * @return {shaka.extern.Track}