fix($resource): do not swallow errors in success callback

Previously, errors thrown inside the `success` callback would be swallowed by a
noop `catch()` handler. The `catch()` handler was added in order to avoid an
unnecessary "Possibly Unhandled Rejection" error, in case the user provided an
`error` callback (which would handle request errors).

The handler was added too "eagrly" and as a result would swallow errors thrown
in the `success` callback, despite the fact that those errors would _not_ be
handled by the `error` callback.

This commit fixes this, by adding the `catch()` handler "lazily", only when it
is certain that a rejection will be handled by the `error` callback.

Fixes #15624

Closes #15628

Author: kylewuolle

src/ngResource/resource.js

@@ -785,18 +785,18 @@ angular.module('ngResource', ['ng']).
                 return value;
               },
               (hasError || hasResponseErrorInterceptor) ?
-              function(response) {
-                if (hasError) error(response);
-                return hasResponseErrorInterceptor ?
+                function(response) {
+                  if (hasError && !hasResponseErrorInterceptor) {
+                    // Avoid `Possibly Unhandled Rejection` error,
+                    // but still fulfill the returned promise with a rejection
+                    promise.catch(noop);
+                  }
+                  if (hasError) error(response);
+                  return hasResponseErrorInterceptor ?
                     responseErrorInterceptor(response) :
                     $q.reject(response);
-              } :
-              undefined);
-            if (hasError && !hasResponseErrorInterceptor) {
-              // Avoid `Possibly Unhandled Rejection` error,
-              // but still fulfill the returned promise with a rejection
-              promise.catch(noop);
-            }
+                } :
+                undefined);
 
             if (!isInstanceCall) {
               // we are creating instance / collection

test/ngResource/resourceSpec.js

@@ -1721,6 +1721,76 @@ describe('handling rejections', function() {
       expect($exceptionHandler.errors[0]).toMatch(/^Possibly unhandled rejection/);
     })
   );
+
+
+  it('should not swallow exceptions in success callback when error callback is provided',
+    function() {
+      $httpBackend.expect('GET', '/CreditCard/123').respond(null);
+      var CreditCard = $resource('/CreditCard/:id');
+      var cc = CreditCard.get({id: 123},
+          function(res) { throw new Error('should be caught'); },
+          function() {});
+
+      $httpBackend.flush();
+      expect($exceptionHandler.errors.length).toBe(1);
+      expect($exceptionHandler.errors[0]).toMatch(/^Error: should be caught/);
+    }
+  );
+
+
+  it('should not swallow exceptions in success callback when error callback is not provided',
+    function() {
+      $httpBackend.expect('GET', '/CreditCard/123').respond(null);
+      var CreditCard = $resource('/CreditCard/:id');
+      var cc = CreditCard.get({id: 123},
+          function(res) { throw new Error('should be caught'); });
+
+      $httpBackend.flush();
+      expect($exceptionHandler.errors.length).toBe(1);
+      expect($exceptionHandler.errors[0]).toMatch(/^Error: should be caught/);
+    }
+  );
+
+
+  it('should not swallow exceptions in success callback when error callback is provided and has responseError interceptor',
+    function() {
+      $httpBackend.expect('GET', '/CreditCard/123').respond(null);
+      var CreditCard = $resource('/CreditCard/:id:', null, {
+        get: {
+          method: 'GET',
+          interceptor: {responseError: function() {}}
+        }
+      });
+
+      var cc = CreditCard.get({id: 123},
+          function(res) { throw new Error('should be caught'); },
+          function() {});
+
+      $httpBackend.flush();
+      expect($exceptionHandler.errors.length).toBe(1);
+      expect($exceptionHandler.errors[0]).toMatch(/^Error: should be caught/);
+    }
+  );
+
+
+  it('should not swallow exceptions in success callback when error callback is not provided and has responseError interceptor',
+    function() {
+      $httpBackend.expect('GET', '/CreditCard/123').respond(null);
+      var CreditCard = $resource('/CreditCard/:id', null, {
+        get: {
+          method: 'GET',
+          interceptor: {responseError: function() {}}
+        }
+      });
+
+      var cc = CreditCard.get({id: 123},
+          function(res) { throw new Error('should be caught'); });
+
+      $httpBackend.flush();
+      expect($exceptionHandler.errors.length).toBe(1);
+      expect($exceptionHandler.errors[0]).toMatch(/^Error: should be caught/);
+    }
+  );
 });
 
 describe('cancelling requests', function() {