fix: don't wait for requests to finish when encountering an error in …

…media-segment-request (#286)

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);
  }

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();