Don't retry network requests on CRITICAL errors.

Issue #620 Change-Id: I6b659acdccf5d893022f3474b45020482f543550
shaka-project · Apr 5, 2017 · f5cabea · f5cabea
1 parent b4b1453
commit f5cabea
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 14 deletions.
diff --git a/lib/net/networking_engine.js b/lib/net/networking_engine.js
@@ -275,9 +275,7 @@ shaka.net.NetworkingEngine.prototype.request = function(type, request) {
   var p = Promise.resolve();
   this.requestFilters_.forEach(function(requestFilter) {
     // Request filters are resolved sequentially.
-    p = p.then(function() {
-      return Promise.resolve(requestFilter(type, request));
-    }.bind(this));
+    p = p.then(requestFilter.bind(null, type, request));
   });
 
   // Catch any errors thrown by request filters, and substitute
@@ -303,8 +301,11 @@ shaka.net.NetworkingEngine.prototype.request = function(type, request) {
     var p = this.send_(type, request, 0, filterTimeMs);
     for (var i = 1; i < maxAttempts; i++) {
       var index = i % request.uris.length;
-      p = p.catch(
-          this.resend_.bind(this, type, request, delay, index, filterTimeMs));
+      p = p.catch(function(delay, index, err) {
+        if (err && err.severity == shaka.util.Error.Severity.RECOVERABLE)
+          return this.resend_(type, request, delay, index, filterTimeMs);
+        throw err;
+      }.bind(this, delay, index));
       delay *= backoffFactor;
     }
 
@@ -395,8 +396,12 @@ shaka.net.NetworkingEngine.prototype.send_ = function(
     // Catch any errors thrown by response filters, and substitute
     // them with a Shaka-native error.
     p = p.catch(function(e) {
+      var severity = shaka.util.Error.Severity.CRITICAL;
+      if (e instanceof shaka.util.Error)
+        severity = e.severity;
+
       throw new shaka.util.Error(
-          shaka.util.Error.Severity.CRITICAL,
+          severity,
           shaka.util.Error.Category.NETWORK,
           shaka.util.Error.Code.RESPONSE_FILTER_ERROR, e);
     });

diff --git a/test/net/networking_engine_unit.js b/test/net/networking_engine_unit.js
@@ -229,6 +229,24 @@ describe('NetworkingEngine', /** @suppress {accessControls} */ function() {
             done();
           });
     });
+
+    it('won\'t retry for CRITICAL error', function(done) {
+      var request = createRequest('reject://foo', {
+        maxAttempts: 5,
+        baseDelay: 0,
+        backoffFactor: 0,
+        fuzzFactor: 0,
+        timeout: 0
+      });
+
+      error.severity = shaka.util.Error.Severity.CRITICAL;
+      networkingEngine.request(requestType, request)
+          .then(fail)
+          .catch(function() {
+            expect(rejectScheme.calls.count()).toBe(1);
+            done();
+          });
+    });
   });
 
   describe('request', function() {
@@ -391,6 +409,7 @@ describe('NetworkingEngine', /** @suppress {accessControls} */ function() {
       networkingEngine.request(requestType, createRequest('resolve://foo'))
           .then(fail)
           .catch(function(e) {
+            expect(e.severity).toBe(shaka.util.Error.Severity.CRITICAL);
             expect(e.code).toBe(shaka.util.Error.Code.REQUEST_FILTER_ERROR);
             expect(e.data).toEqual([fakeError]);
             done();
@@ -664,9 +683,11 @@ describe('NetworkingEngine', /** @suppress {accessControls} */ function() {
         fuzzFactor: 0,
         timeout: 0
       });
+      error.severity = shaka.util.Error.Severity.RECOVERABLE;
       filter.and.callFake(function() {
         if (filter.calls.count() == 1) throw error;
       });
+
       networkingEngine.request(requestType, request)
           .catch(fail)
           .then(function() {
@@ -696,11 +717,15 @@ describe('NetworkingEngine', /** @suppress {accessControls} */ function() {
       expect(r1.status).toBe('pending');
       expect(r2.status).toBe('pending');
 
-      var d = networkingEngine.destroy();
-      Util.capturePromiseStatus(d);
-      expect(d.status).toBe('pending');
-
+      var d;
       Util.delay(0.1).then(function() {
+        d = networkingEngine.destroy();
+        Util.capturePromiseStatus(d);
+        expect(d.status).toBe('pending');
+        expect(r1.status).toBe('pending');
+        expect(r2.status).toBe('pending');
+        return Util.delay(0.1);
+      }).then(function() {
         expect(d.status).toBe('pending');
         p.resolve({});
         return d;
@@ -759,11 +784,14 @@ describe('NetworkingEngine', /** @suppress {accessControls} */ function() {
       expect(r1.status).toBe('pending');
       expect(r2.status).toBe('pending');
 
-      var d = networkingEngine.destroy();
-      Util.capturePromiseStatus(d);
-      expect(d.status).toBe('pending');
-
+      var d;
       Util.delay(0.1).then(function() {
+        d = networkingEngine.destroy();
+        Util.capturePromiseStatus(d);
+        expect(d.status).toBe('pending');
+
+        return Util.delay(0.1);
+      }).then(function() {
         expect(d.status).toBe('pending');
         p.reject(error);
         return d;