fix: ensure sync errors are thrown, and don't callback twice

Author: mbroadst

lib/core/utils.js

@@ -131,30 +131,29 @@ function isPromiseLike(maybePromise) {
  * @param {function} callback The callback called after every item has been iterated
  */
 function eachAsync(arr, eachFn, callback) {
-  if (arr.length === 0) {
-    callback(null);
+  arr = arr || [];
+
+  let idx = 0;
+  let awaiting = 0;
+  for (idx = 0; idx < arr.length; ++idx) {
+    awaiting++;
+    eachFn(arr[idx], eachCallback);
+  }
+
+  if (awaiting === 0) {
+    callback();
     return;
   }
 
-  const length = arr.length;
-  let completed = 0;
   function eachCallback(err) {
+    awaiting--;
     if (err) {
-      callback(err, null);
+      callback(err);
       return;
     }
 
-    if (++completed === length) {
-      callback(null);
-    }
-  }
-
-  for (let idx = 0; idx < length; ++idx) {
-    try {
-      eachFn(arr[idx], eachCallback);
-    } catch (err) {
-      callback(err);
-      return;
+    if (idx === arr.length && awaiting <= 0) {
+      callback();
     }
   }
 }

test/unit/utils.test.js

@@ -0,0 +1,36 @@
+'use strict';
+const eachAsync = require('../../lib/core/utils').eachAsync;
+const expect = require('chai').expect;
+
+describe('utils', function() {
+  describe('eachAsync', function() {
+    it('should callback with an error', function(done) {
+      eachAsync(
+        [{ error: false }, { error: true }],
+        (item, cb) => {
+          cb(item.error ? new Error('error requested') : null);
+        },
+        err => {
+          expect(err).to.exist;
+          done();
+        }
+      );
+    });
+
+    it('should propagate a synchronously thrown error', function(done) {
+      expect(() =>
+        eachAsync(
+          [{}],
+          () => {
+            throw new Error('something wicked');
+          },
+          err => {
+            expect(err).to.not.exist;
+            done(err);
+          }
+        )
+      ).to.throw(/something wicked/);
+      done();
+    });
+  });
+});