refactor(ngResource): handle success and error callbacks using the same promise

Author: lgalfaso

src/ngResource/resource.js

@@ -701,19 +701,15 @@ angular.module('ngResource', ['ng']).
               response.resource = value;
 
               return response;
-            }, function(response) {
-              (error || noop)(response);
-              return $q.reject(response);
             });
 
-            promise = promise['finally'](function(response) {
+            promise = promise['finally'](function() {
               value.$resolved = true;
               if (!isInstanceCall && cancellable) {
                 value.$cancelRequest = angular.noop;
                 $timeout.cancel(numericTimeoutPromise);
                 timeoutDeferred = numericTimeoutPromise = httpConfig.timeout = null;
               }
-              return response;
             });
 
             promise = promise.then(
@@ -722,21 +718,32 @@ angular.module('ngResource', ['ng']).
                 (success || noop)(value, response.headers);
                 return value;
               },
-              responseErrorInterceptor);
+              responseErrorInterceptor || error ?
+              function(response) {
+                (error || noop)(response);
+                (responseErrorInterceptor || noop)(response);
+                return response;
+              }
+              : undefined);
 
             if (!isInstanceCall) {
               // we are creating instance / collection
               // - set the initial promise
               // - return the instance / collection
               value.$promise = promise;
               value.$resolved = false;
-              if (cancellable) value.$cancelRequest = timeoutDeferred.resolve;
+              if (cancellable) value.$cancelRequest = cancelRequest;
 
               return value;
             }
 
             // instance call
             return promise;
+
+            function cancelRequest(value) {
+              promise.catch(noop);
+              timeoutDeferred.resolve(value);
+            }
           };
 
 

test/ng/animateRunnerSpec.js

@@ -206,7 +206,7 @@ describe("$$AnimateRunner", function() {
         var animationComplete = false;
         runner.finally(function() {
           animationComplete = true;
-        }).then(noop, noop);
+        }).catch(noop);
         runner[method]();
         $rootScope.$digest();
         expect(animationComplete).toBe(true);

test/ngResource/resourceSpec.js

@@ -1052,8 +1052,7 @@ describe("basic usage", function() {
     it('should call the error callback if provided on non 2xx response', function() {
       $httpBackend.expect('GET', '/CreditCard/123').respond(ERROR_CODE, ERROR_RESPONSE);
 
-      var ccs = CreditCard.get({id:123}, callback, errorCB);
-      ccs.$promise.then(noop, noop);
+      CreditCard.get({id:123}, callback, errorCB);
       $httpBackend.flush();
       expect(errorCB).toHaveBeenCalledOnce();
       expect(callback).not.toHaveBeenCalled();
@@ -1063,8 +1062,7 @@ describe("basic usage", function() {
     it('should call the error callback if provided on non 2xx response (without data)', function() {
       $httpBackend.expect('GET', '/CreditCard').respond(ERROR_CODE, ERROR_RESPONSE);
 
-      var ccs = CreditCard.get(callback, errorCB);
-      ccs.$promise.then(noop, noop);
+      CreditCard.get(callback, errorCB);
       $httpBackend.flush();
       expect(errorCB).toHaveBeenCalledOnce();
       expect(callback).not.toHaveBeenCalled();
@@ -1564,7 +1562,6 @@ describe('cancelling requests', function() {
     });
 
     var ccs = CreditCard.get();
-    ccs.$promise.catch(noop);
     ccs.$cancelRequest();
     expect($httpBackend.flush).toThrow(new Error('No pending request to flush !'));
 
@@ -1584,7 +1581,6 @@ describe('cancelling requests', function() {
     });
 
     var ccs = CreditCard.get();
-    ccs.$promise.catch(noop);
     ccs.$cancelRequest();
     expect($httpBackend.flush).toThrow(new Error('No pending request to flush !'));