Merge pull request #6 from daf-spr/fix/error-callback-issue

Fix callback execution when request fails
jpodwys · Sep 29, 2015 · 471d333 · 471d333
2 parents 98747e3 + cec1937
commit 471d333
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 4 deletions.
diff --git a/superagentCache.js b/superagentCache.js
@@ -123,6 +123,10 @@ module.exports = function(agent, cache){
           else{
             if(curProps.doQuery){
               _this._end(function (err, response){
+                if (err) {
+                  return callbackExecutor(cb, err, response, key);
+                }
+
                 if(!err && response){
                   if(curProps.prune){
                     response = curProps.prune(response);
@@ -156,6 +160,10 @@ module.exports = function(agent, cache){
       }
       else{
         this._end(function (err, response){
+          if (err) {
+            return callbackExecutor(cb, err, response, key);
+          }
+
           if(!err && response){
             superagent.cache.del(key, function (){
               callbackExecutor(cb, err, response, key);

diff --git a/test/server/superagent-cache.js b/test/server/superagent-cache.js
@@ -4,9 +4,9 @@ var superagent = require('superagent');
 var cModule = require('cache-service-cache-module');
 var cacheModule = new cModule({backgroundRefreshInterval: 500});
 require('../../superagentCache')(superagent, cacheModule);
- 
+
 var app = express();
- 
+
 app.get('/one', function(req, res){
   res.send(200, {key: 'one'});
 });
@@ -239,7 +239,7 @@ describe('Array', function(){
   });
 
   describe('superagentCache caching tests', function () {
-    
+
     it('.get() ._end() should bypass all caching logic', function (done) {
       superagent
         .get('localhost:3000/one')
@@ -321,10 +321,23 @@ describe('Array', function(){
       );
     });
 
+    describe('when response is not valid', function() {
+
+      it('.end() should not set \'err\' callback param on error', function (done) {
+        superagent
+          .get('localhost:3000/invalid')
+          .end(function (err, response){
+            expect(err).toExist();
+            done();
+          }
+        );
+      });
+    });
+
   });
 
   describe('superagentCache background refresh tests', function () {
-    
+
     it('.get() .expiration() .end() background refresh should not work if the chainable is not used', function (done) {
       superagent
         .get('localhost:3000/one')