added error handler to xhr requests

6 files changed, 107 insertions, 22 deletions

diff --git a/src/Resource.js b/src/Resource.js
index c9bad0c0..724121b7 100644
--- a/src/Resource.js
+++ b/src/Resource.js
@@ -87,16 +87,25 @@ ResourceFactory.prototype = {
         }
 
         var value = action.isArray ? [] : new Resource(data);
-        self.xhr(action.method, route.url(extend({}, action.params || {}, extractParams(data), params)), data, function(status, response) {
-          if (action.isArray) {
-            foreach(response, function(item){
-              value.push(new Resource(item));
-            });
-          } else {
-            copy(response, value);
+        self.xhr(
+          action.method,
+          route.url(extend({}, action.params || {}, extractParams(data), params)),
+          data,
+          function(status, response) {
+            if (status == 200) {
+              if (action.isArray) {
+                foreach(response, function(item){
+                  value.push(new Resource(item));
+                });
+              } else {
+                copy(response, value);
+              }
+              (callback||noop)(value);
+            } else {
+              throw {status: status, response:response, message: status + ": " + response};
+            }
           }
-          (callback||noop)(value);
-        });
+        );
         return value;
       };
 
diff --git a/src/Scope.js b/src/Scope.js
index 7bcf7380..fe0b6ce3 100644
--- a/src/Scope.js
+++ b/src/Scope.js
@@ -174,7 +174,7 @@ function createScope(parent, services, existing) {
   }
 
   function inject(name){
-    var service = getter(servicesCache, name, true), factory, args = [];
+    var service = servicesCache[name], factory, args = [];
     if (isUndefined(service)) {
       factory = services[name];
       if (!isFunction(factory))
@@ -182,7 +182,7 @@ function createScope(parent, services, existing) {
       foreach(factory.inject, function(dependency){
         args.push(inject(dependency));
       });
-      setter(servicesCache, name, service = factory.apply(instance, args));
+      servicesCache[name] = service = factory.apply(instance, args);
     }
     return service;
   }
diff --git a/src/services.js b/src/services.js
index 55e3b2f6..f1dc5c28 100644
--- a/src/services.js
+++ b/src/services.js
@@ -196,7 +196,7 @@ angularService('$route', function(location, params){
   return $route;
 }, {inject: ['$location']});
 
-angularService('$xhr', function($browser){
+angularService('$xhr', function($browser, $error){
   var self = this;
   return function(method, url, post, callback){
     if (isFunction(post)) {
@@ -211,15 +211,27 @@ angularService('$xhr', function($browser){
         if (isString(response) && /^\s*[\[\{]/.exec(response) && /[\}\]]\s*$/.exec(response)) {
           response = fromJson(response);
         }
-        callback(code, response);
+        if (code == 200) {
+          callback(code, response);
+        } else {
+          $error(
+            {method: method, url:url, data:post, callback:callback},
+            {status: code, body:response});
+        }
       } finally {
         self.$eval();
       }
     });
   };
-}, {inject:['$browser']});
+}, {inject:['$browser', '$xhr.error']});
+
+angularService('$xhr.error', function($log){
+  return function(request, response){
+    $log.error(response);
+  };
+}, {inject:['$log']});
 
-angularService('$xhr.bulk', function($xhr){
+angularService('$xhr.bulk', function($xhr, $error){
   var requests = [],
       callbacks = [],
       scope = this;
@@ -254,7 +266,13 @@ angularService('$xhr.bulk', function($xhr){
         $xhr('POST', url, {requests:currentRequests}, function(code, response){
           foreach(response, function(response, i){
             try {
-              (currentCallbacks[i] || noop)(response.status, response.response);
+              if (response.status == 200) {
+                (currentCallbacks[i] || noop)(response.status, response.response);
+              } else {
+                $error(
+                    extend({}, currentRequests[i], {callback: currentCallbacks[i]}),
+                    {status: response.status, body:response.response});
+              }
             } catch(e) {
               scope.$log.error(e);
             }
@@ -267,7 +285,7 @@ angularService('$xhr.bulk', function($xhr){
   };
   this.$onEval(PRIORITY_LAST, bulkXHR.flush);
   return bulkXHR;
-}, {inject:['$xhr']});
+}, {inject:['$xhr', '$xhr.error']});
 
 angularService('$xhr.cache', function($xhr){
   var inflight = {}, self = this;;
diff --git a/test/ResourceSpec.js b/test/ResourceSpec.js
index d2d52d47..2f285bcf 100644
--- a/test/ResourceSpec.js
+++ b/test/ResourceSpec.js
@@ -138,4 +138,19 @@ describe("resource", function() {
     expect(person.name).toEqual('misko');
   });
 
+  describe('failure mode', function(){
+    it('should report error when non 200', function(){
+      xhr.expectGET('/CreditCard/123').respond(500, "Server Error");
+      var cc = CreditCard.get({id:123});
+      try {
+        xhr.flush();
+        fail('expected exception, non thrown');
+      } catch (e) {
+        expect(e.status).toEqual(500);
+        expect(e.response).toEqual('Server Error');
+        expect(e.message).toEqual('500: Server Error');
+      }
+    });
+  });
+
 });
diff --git a/test/angular-mocks.js b/test/angular-mocks.js
index 6ae91596..c5784ac9 100644
--- a/test/angular-mocks.js
+++ b/test/angular-mocks.js
@@ -43,7 +43,7 @@ function MockBrowser() {
       throw "Unexepected request for method '" + method + "' and url '" + url + "'.";
     }
     requests.push(function(){
-      callback(200, response);
+      callback(response.code, response.response);
     });
   };
   self.xhr.expectations = expectations;
@@ -53,8 +53,12 @@ function MockBrowser() {
     if (data && angular.isString(data)) url += "|" + data;
     var expect = expectations[method] || (expectations[method] = {});
     return {
-      respond: function(response) {
-        expect[url] = response;
+      respond: function(code, response) {
+        if (!isNumber(code)) {
+          response = code;
+          code = 200;
+        }
+        expect[url] = {code:code, response:response};
       }
     };
   };
diff --git a/test/servicesSpec.js b/test/servicesSpec.js
index 17f71bdc..4e144dd1 100644
--- a/test/servicesSpec.js
+++ b/test/servicesSpec.js
@@ -1,8 +1,11 @@
 describe("service", function(){
-  var scope;
+  var scope, xhrErrorHandler;
 
   beforeEach(function(){
-    scope = createScope(null, angularService, {});
+    xhrErrorHandler = jasmine.createSpy('$xhr.error');
+    scope = createScope(null, angularService, {
+      '$xhr.error': xhrErrorHandler
+    });
   });
 
   afterEach(function(){
@@ -194,6 +197,17 @@ describe("service", function(){
       expect(log).toEqual('"third";["second"];"first";');
     });
 
+    it('should handle non 200 status codes by forwarding to error handler', function(){
+      xhr.expectPOST('/req', 'MyData').respond(500, 'MyError');
+      scope.$xhr('POST', '/req', 'MyData', callback);
+      xhr.flush();
+      var cb = xhrErrorHandler.mostRecentCall.args[0].callback;
+      expect(typeof cb).toEqual('function');
+      expect(xhrErrorHandler).wasCalledWith(
+          {url:'/req', method:'POST', data:'MyData', callback:cb},
+          {status:500, body:'MyError'});
+    });
+
     describe('bulk', function(){
       it('should collect requests', function(){
         scope.$xhr.bulk.urls["/"] = {match:/.*/};
@@ -211,6 +225,31 @@ describe("service", function(){
         xhr.flush();
         expect(log).toEqual('"first";"second";DONE');
       });
+
+      it('should handle non 200 status code by forwarding to error handler', function(){
+        scope.$xhr.bulk.urls['/'] = {match:/.*/};
+        scope.$xhr.bulk('GET', '/req1', null, callback);
+        scope.$xhr.bulk('POST', '/req2', {post:'data'}, callback);
+
+        xhr.expectPOST('/', {
+          requests:[{method:'GET',  url:'/req1', data: null},
+                    {method:'POST', url:'/req2', data:{post:'data'} }]
+        }).respond([
+          {status:404, response:'NotFound'},
+          {status:200, response:'second'}
+        ]);
+        scope.$xhr.bulk.flush(function(){ log += 'DONE';});
+        xhr.flush();
+
+        expect(xhrErrorHandler).wasCalled();
+        var cb = xhrErrorHandler.mostRecentCall.args[0].callback;
+        expect(typeof cb).toEqual('function');
+        expect(xhrErrorHandler).wasCalledWith(
+            {url:'/req1', method:'GET', data:null, callback:cb},
+            {status:404, body:'NotFound'});
+
+        expect(log).toEqual('"second";DONE');
+      });
     });
 
     describe('cache', function(){