#1465; don't memoize results that had an error

caolan · Aug 21, 2017 · 1394215 · 1394215
1 parent fa206af
commit 1394215
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 7 deletions.
diff --git a/lib/memoize.js b/lib/memoize.js
@@ -14,6 +14,9 @@ function has(obj, key) {
  * function results against, the callback is omitted from the hash and an
  * optional hash function can be used.
  *
+ * **Note: if the async function errs, the result will not be cached and
+ * subsequent calls will call the wrapped function.**
+ *
  * If no hash function is specified, the first argument is used as a hash key,
  * which may work reasonably if it is a string or a data type that converts to a
  * distinct string. Note that objects and arrays will not behave reasonably.
@@ -63,7 +66,11 @@ export default function memoize(fn, hasher) {
             queues[key] = [callback];
             _fn.apply(null, args.concat(function(/*args*/) {
                 var args = slice(arguments);
-                memo[key] = args;
+                var err = args[0];
+                // #1465 don't memoize if an error occurred
+                if (!err) {
+                    memo[key] = args;
+                }
                 var q = queues[key];
                 delete queues[key];
                 for (var i = 0, l = q.length; i < l; i++) {

diff --git a/mocha_test/memoize.js b/mocha_test/memoize.js
@@ -100,9 +100,8 @@ describe("memoize", function() {
         var fn2 = async.unmemoize(fn);
         fn2(1, 2, function(err, result) {
             expect(result).to.equal(3);
+            done();
         });
-
-        done();
     });
 
     it('error', function(done) {
@@ -112,8 +111,26 @@ describe("memoize", function() {
         };
         async.memoize(fn)(1, 2, function (err) {
             expect(err).to.equal(testerr);
+            done();
+        });
+    });
+
+    it('should not memoize result if error occurs', function(done) {
+        var testerr = new Error('test');
+        var fn = function (arg1, arg2, callback) {
+            callback(testerr, arg1 + arg2);
+        };
+        var memoized = async.memoize(fn);
+        memoized(1, 2, function (err) {
+            expect(err).to.equal(testerr);
+            testerr = null;
+
+            memoized(5, 6, function (err, result) {
+                expect(err).to.equal(null);
+                expect(result).to.equal(11);
+                done();
+            });
         });
-        done();
     });
 
     it('multiple calls', function(done) {
@@ -134,10 +151,8 @@ describe("memoize", function() {
     });
 
     it('custom hash function', function(done) {
-        var testerr = new Error('test');
-
         var fn = function (arg1, arg2, callback) {
-            callback(testerr, arg1 + arg2);
+            callback(null, arg1 + arg2);
         };
         var fn2 = async.memoize(fn, function () {
             return 'custom hash';
@@ -205,4 +220,5 @@ describe("memoize", function() {
             done();
         });
     });
+
 });