fix($httpBackend): complete the request on timeout

gkalpak · gkalpak · commit 549edc9d0123 · 2016-08-08T19:16:30.000+03:00
When using the [timeout attribute](https://xhr.spec.whatwg.org/#the-timeout-attribute) and an XHR request times out, browsers trigger the `timeout` event (and execute the XHR's `ontimeout` callback). Additionally, Safari 9 handles timed-out requests in the same way, even if no `timeout` has been explicitly set on the XHR. In the above cases, `$httpBackend` would fail to capture the XHR's completing (with an error), so the corresponding `$http` promise would never get fulfilled. Note that using `$http`'s `timeout` configuration option does **not** rely on the XHR's `timeout` property (or its `ontimeout` callback). Fixes angular#14969 Closes angular#14972
diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js
@@ -111,6 +111,7 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
 
       xhr.onerror = requestError;
       xhr.onabort = requestError;
+      xhr.ontimeout = requestError;
 
       forEach(eventHandlers, function(value, key) {
           xhr.addEventListener(key, value);
diff --git a/test/ng/httpBackendSpec.js b/test/ng/httpBackendSpec.js
@@ -155,10 +155,11 @@ describe('$httpBackend', function() {
   });
 
   it('should not try to read response data when request is aborted', function() {
-    callback.and.callFake(function(status, response, headers) {
+    callback.and.callFake(function(status, response, headers, statusText) {
       expect(status).toBe(-1);
       expect(response).toBe(null);
       expect(headers).toBe(null);
+      expect(statusText).toBe('');
     });
     $backend('GET', '/url', null, callback, {}, 2000);
     xhr = MockXhr.$$lastInstance;
@@ -172,6 +173,22 @@ describe('$httpBackend', function() {
     expect(callback).toHaveBeenCalledOnce();
   });
 
+  it('should complete the request on timeout', function() {
+    callback.and.callFake(function(status, response, headers, statusText) {
+      expect(status).toBe(-1);
+      expect(response).toBe(null);
+      expect(headers).toBe(null);
+      expect(statusText).toBe('');
+    });
+    $backend('GET', '/url', null, callback, {});
+    xhr = MockXhr.$$lastInstance;
+
+    expect(callback).not.toHaveBeenCalled();
+
+    xhr.ontimeout();
+    expect(callback).toHaveBeenCalledOnce();
+  });
+
   it('should abort request on timeout', function() {
     callback.and.callFake(function(status, response) {
       expect(status).toBe(-1);
@@ -253,6 +270,7 @@ describe('$httpBackend', function() {
     expect(MockXhr.$$lastInstance.withCredentials).toBe(true);
   });
 
+
   it('should call $xhrFactory with method and url', function() {
     var mockXhrFactory = jasmine.createSpy('mockXhrFactory').and.callFake(createMockXhr);
     $backend = createHttpBackend($browser, mockXhrFactory, $browser.defer, $jsonpCallbacks, fakeDocument);
@@ -391,6 +409,7 @@ describe('$httpBackend', function() {
     // TODO(vojta): test whether it fires "async-end" on both success and error
   });
 
+
   describe('protocols that return 0 status code', function() {
 
     function respond(status, content) {
@@ -400,10 +419,12 @@ describe('$httpBackend', function() {
       xhr.onload();
     }
 
-
-    it('should convert 0 to 200 if content and file protocol', function() {
+    beforeEach(function() {
       $backend = createHttpBackend($browser, createMockXhr);
+    });
+
 
+    it('should convert 0 to 200 if content and file protocol', function() {
       $backend('GET', 'file:///whatever/index.html', null, callback);
       respond(0, 'SOME CONTENT');
 
@@ -412,8 +433,6 @@ describe('$httpBackend', function() {
     });
 
     it('should convert 0 to 200 if content for protocols other than file', function() {
-      $backend = createHttpBackend($browser, createMockXhr);
-
       $backend('GET', 'someProtocol:///whatever/index.html', null, callback);
       respond(0, 'SOME CONTENT');
 
@@ -422,8 +441,6 @@ describe('$httpBackend', function() {
     });
 
     it('should convert 0 to 404 if no content and file protocol', function() {
-      $backend = createHttpBackend($browser, createMockXhr);
-
       $backend('GET', 'file:///whatever/index.html', null, callback);
       respond(0, '');
 
@@ -432,8 +449,6 @@ describe('$httpBackend', function() {
     });
 
     it('should not convert 0 to 404 if no content for protocols other than file', function() {
-      $backend = createHttpBackend($browser, createMockXhr);
-
       $backend('GET', 'someProtocol:///whatever/index.html', null, callback);
       respond(0, '');
 
@@ -460,8 +475,6 @@ describe('$httpBackend', function() {
 
       try {
 
-        $backend = createHttpBackend($browser, createMockXhr);
-
         $backend('GET', '/whatever/index.html', null, callback);
         respond(0, '');
 
@@ -473,10 +486,7 @@ describe('$httpBackend', function() {
       }
     });
 
-
     it('should return original backend status code if different from 0', function() {
-      $backend = createHttpBackend($browser, createMockXhr);
-
       // request to http://
       $backend('POST', 'http://rest_api/create_whatever', null, callback);
       respond(201, '');
