diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index 31a594421e50..ae629d0b16fa 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -695,6 +695,8 @@ angular.module('ngResource', ['ng']). defaultResponseInterceptor; var responseErrorInterceptor = action.interceptor && action.interceptor.responseError || undefined; + var hasError = !!error; + var hasResponseErrorInterceptor = !!responseErrorInterceptor; var timeoutDeferred; var numericTimeoutPromise; @@ -776,13 +778,19 @@ angular.module('ngResource', ['ng']). (success || noop)(value, response.headers); return value; }, - responseErrorInterceptor || error ? + (hasError || hasResponseErrorInterceptor) ? function(response) { - (error || noop)(response); - (responseErrorInterceptor || noop)(response); - return response; - } - : undefined); + 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); + } if (!isInstanceCall) { // we are creating instance / collection diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js index a848f06d1244..f3f83682ea55 100644 --- a/test/ngResource/resourceSpec.js +++ b/test/ngResource/resourceSpec.js @@ -30,7 +30,7 @@ describe("basic usage", function() { } }); - callback = jasmine.createSpy(); + callback = jasmine.createSpy('callback'); })); @@ -907,6 +907,7 @@ describe("basic usage", function() { expect(cc.url).toBe('/new-id'); }); + it('should pass the same transformed value to success callbacks and to promises', function() { $httpBackend.expect('GET', '/CreditCard').respond(200, { value: 'original' }); @@ -1024,6 +1025,7 @@ describe("basic usage", function() { }); }); + it('should allow per action response interceptor that gets full response', function() { CreditCard = $resource('/CreditCard', {}, { query: { @@ -1079,6 +1081,46 @@ describe("basic usage", function() { expect(response.status).toBe(404); expect(response.config).toBeDefined(); }); + + + it('should fulfill the promise with the value returned by the responseError interceptor', + inject(function($q) { + CreditCard = $resource('/CreditCard', {}, { + test1: { + method: 'GET', + interceptor: {responseError: function() { return 'foo'; }} + }, + test2: { + method: 'GET', + interceptor: {responseError: function() { return $q.resolve('bar'); }} + }, + test3: { + method: 'GET', + interceptor: {responseError: function() { return $q.reject('baz'); }} + } + }); + + $httpBackend.whenGET('/CreditCard').respond(404); + + callback.calls.reset(); + CreditCard.test1().$promise.then(callback); + $httpBackend.flush(); + + expect(callback).toHaveBeenCalledOnceWith('foo'); + + callback.calls.reset(); + CreditCard.test2().$promise.then(callback); + $httpBackend.flush(); + + expect(callback).toHaveBeenCalledOnceWith('bar'); + + callback.calls.reset(); + CreditCard.test3().$promise.then(null, callback); + $httpBackend.flush(); + + expect(callback).toHaveBeenCalledOnceWith('baz'); + }) + ); }); @@ -1414,6 +1456,102 @@ describe('errors', function() { }); }); +describe('handling rejections', function() { + var $exceptionHandler; + var $httpBackend; + var $resource; + + beforeEach(module('ngResource')); + beforeEach(module(function($exceptionHandlerProvider) { + $exceptionHandlerProvider.mode('log'); + })); + + beforeEach(inject(function(_$exceptionHandler_, _$httpBackend_, _$resource_) { + $exceptionHandler = _$exceptionHandler_; + $httpBackend = _$httpBackend_; + $resource = _$resource_; + + $httpBackend.whenGET('/CreditCard').respond(404); + })); + + + it('should reject the promise even when there is an error callback', function() { + var errorCb1 = jasmine.createSpy('errorCb1'); + var errorCb2 = jasmine.createSpy('errorCb2'); + var CreditCard = $resource('/CreditCard'); + + CreditCard.get(noop, errorCb1).$promise.catch(errorCb2); + $httpBackend.flush(); + + expect(errorCb1).toHaveBeenCalledOnce(); + expect(errorCb2).toHaveBeenCalledOnce(); + }); + + + it('should report a PUR when no error callback or responseError interceptor is provided', + function() { + var CreditCard = $resource('/CreditCard'); + + CreditCard.get(); + $httpBackend.flush(); + + expect($exceptionHandler.errors.length).toBe(1); + expect($exceptionHandler.errors[0]).toMatch(/^Possibly unhandled rejection/); + } + ); + + + it('should not report a PUR when an error callback or responseError interceptor is provided', + function() { + var CreditCard = $resource('/CreditCard', {}, { + test1: { + method: 'GET' + }, + test2: { + method: 'GET', + interceptor: {responseError: function() { return {}; }} + } + }); + + // With error callback + CreditCard.test1(noop, noop); + $httpBackend.flush(); + + expect($exceptionHandler.errors.length).toBe(0); + + // With responseError interceptor + CreditCard.test2(); + $httpBackend.flush(); + + expect($exceptionHandler.errors.length).toBe(0); + + // With error callback and responseError interceptor + CreditCard.test2(noop, noop); + $httpBackend.flush(); + + expect($exceptionHandler.errors.length).toBe(0); + } + ); + + + it('should report a PUR when the responseError interceptor returns a rejected promise', + inject(function($q) { + var CreditCard = $resource('/CreditCard', {}, { + test: { + method: 'GET', + interceptor: {responseError: function() { return $q.reject({}); }} + } + }); + + CreditCard.test(); + $httpBackend.flush(); + + expect($exceptionHandler.errors.length).toBe(1); + expect($exceptionHandler.errors[0]).toMatch(/^Possibly unhandled rejection/); + }) + ); +}); + describe('cancelling requests', function() { var httpSpy; var $httpBackend;