diff --git a/src/media-segment-request.js b/src/media-segment-request.js index a3d0a6b34..5ce2deb0b 100644 --- a/src/media-segment-request.js +++ b/src/media-segment-request.js @@ -298,18 +298,6 @@ const decryptSegment = (decrypter, segment, doneFn) => { ]); }; -/** - * The purpose of this function is to get the most pertinent error from the - * array of errors. - * For instance if a timeout and two aborts occur, then the aborts were - * likely triggered by the timeout so return that error object. - */ -const getMostImportantError = (errors) => { - return errors.reduce((prev, err) => { - return err.code > prev.code ? err : prev; - }); -}; - /** * This function waits for all XHRs to finish (with either success or failure) * before continueing processing via it's callback. The function gathers errors @@ -322,26 +310,39 @@ const getMostImportantError = (errors) => { * downloaded and any decryption completed */ const waitForCompletion = (activeXhrs, decrypter, doneFn) => { - let errors = []; let count = 0; + let didError = false; return (error, segment) => { + if (didError) { + return; + } + if (error) { + didError = true; // If there are errors, we have to abort any outstanding requests abortAll(activeXhrs); - errors.push(error); + + // Even though the requests above are aborted, and in theory we could wait until we + // handle the aborted events from those requests, there are some cases where we may + // never get an aborted event. For instance, if the network connection is lost and + // there were two requests, the first may have triggered an error immediately, while + // the second request remains unsent. In that case, the aborted algorithm will not + // trigger an abort: see https://xhr.spec.whatwg.org/#the-abort()-method + // + // We also can't rely on the ready state of the XHR, since the request that + // triggered the connection error may also show as a ready state of 0 (unsent). + // Therefore, we have to finish this group of requests immediately after the first + // seen error. + return doneFn(error, segment); } + count += 1; if (count === activeXhrs.length) { // Keep track of when *all* of the requests have completed segment.endOfAllRequests = Date.now(); - if (errors.length > 0) { - const worstError = getMostImportantError(errors); - - return doneFn(worstError, segment); - } if (segment.encryptedBytes) { return decryptSegment(decrypter, segment, doneFn); } diff --git a/test/media-segment-request.test.js b/test/media-segment-request.test.js index 766ec46be..e0042a607 100644 --- a/test/media-segment-request.test.js +++ b/test/media-segment-request.test.js @@ -193,6 +193,50 @@ QUnit.test('cancels outstanding key requests on timeout', function(assert) { this.clock.tick(2000); }); +QUnit.test('does not wait for other requests to finish when one request errors', +function(assert) { + let keyReq; + let abortedKeyReq = false; + const done = assert.async(); + + assert.expect(8); + mediaSegmentRequest( + this.xhr, + this.xhrOptions, + this.noop, + this.noop, + { + resolvedUri: '0-test.ts', + key: { + resolvedUri: '0-key.php' + } + }, + this.noop, + (error, segmentData) => { + assert.notOk(keyReq.aborted, 'did not run original abort function'); + assert.ok(abortedKeyReq, 'ran overridden abort function'); + assert.equal(error.code, REQUEST_ERRORS.FAILURE, 'request failed'); + + done(); + }); + assert.equal(this.requests.length, 2, 'there are two requests'); + + keyReq = this.requests.shift(); + // Typically, an abort will run the error algorithm for an XHR, however, in certain + // cases (e.g., if the request is unsent), the error algorithm will not be run and + // the request will never "finish." In order to mimic this behavior, override the + // default abort function so that it doesn't finish. + keyReq.abort = () => { + abortedKeyReq = true; + }; + const segmentReq = this.requests.shift(); + + assert.equal(keyReq.uri, '0-key.php', 'the first request is for a key'); + assert.equal(segmentReq.uri, '0-test.ts', 'the second request is for a segment'); + + segmentReq.respond(500, null, ''); +}); + QUnit.test('the key response is converted to the correct format', function(assert) { let keyReq; const done = assert.async();