Skip to content

Commit

Permalink
fix: Allow overriding special handling of 404s (#4635)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@joeyparrish
joeyparrish committed Nov 8, 2022
1 parent 9e8d5a8 commit d927cec
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 30 deletions.
15 changes: 3 additions & 12 deletions lib/media/streaming_engine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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);
}
}

Expand Down
30 changes: 20 additions & 10 deletions lib/player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <code>false</code>.
*
* @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;
}

Expand Down Expand Up @@ -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);
}
}

Expand Down
13 changes: 13 additions & 0 deletions test/media/streaming_engine_integration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
31 changes: 23 additions & 8 deletions test/media/streaming_engine_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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!
Expand Down Expand Up @@ -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!
Expand Down Expand Up @@ -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);
}
});

Expand Down
87 changes: 87 additions & 0 deletions test/player_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down

0 comments on commit d927cec

Please sign in to comment.