Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Open Chain #1

Merged
merged 2 commits into from

2 participants

@schloerke

Ran into a situation where asynchronous functions added to a open groupie.chain are called excessively.

The "add()" method was calling "callNext()" if the group "hasMore()" functions to process. Since this was always true after adding a new function, "callNext()" was executed. In the example below, the first function has not returned yet (incrementing 'pos'), so it is called 3 more times.

I'm thinking the logic within the "add()" method is trying to check if it is processing anything at the current time. If it is processing something, "callNext()" will be called when the processing function is done, otherwise start the new function with "callNext()".

All of this only comes into play when it's an open groupie.chain and a function is asynchronous.

Example code:

group = groupie.chain(function(err, results) {
  if (err != null) {
    console.log("Error: ", err);
  } else {
    console.log("Results: ", results);
  }
});

add_num = function(num, time) {
  group.add(function(done) {
    setTimeout(function() {
      console.log("Done " + num + " - " + (new Date()).toISOString());
      return done(null, num);
    }, time);
  });
};

add_num(1, 3000);
add_num(2, 2000);
add_num(3, 1000);
add_num(4, 4000);

group.finalize();

Output with current groupie:

Done 1 - 2012-03-13T23:38:00.130Z
Done 1 - 2012-03-13T23:38:00.132Z
Done 1 - 2012-03-13T23:38:00.132Z
Done 1 - 2012-03-13T23:38:00.132Z
Results:  [ 1, 1, 1, 1 ]
Done 3 - 2012-03-13T23:38:01.132Z
Results:  [ 1, 1, 1, 1, 3 ]
Done 2 - 2012-03-13T23:38:02.131Z
Results:  [ 1, 1, 1, 1, 3, 2 ]
Done 4 - 2012-03-13T23:38:04.131Z
Results:  [ 1, 1, 1, 1, 3, 2, 4 ]

Output with pull request:

Done 1 - 2012-03-13T23:36:50.348Z
Done 2 - 2012-03-13T23:36:52.349Z
Done 3 - 2012-03-13T23:36:53.350Z
Done 4 - 2012-03-13T23:36:57.351Z
Results:  [ 1, 2, 3, 4 ]

Best,
Barret

@alexkwolfe alexkwolfe merged commit e761168 into alexkwolfe:master
@alexkwolfe
Owner

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 23 additions and 20 deletions.
  1. +23 −20 lib/groupie.js
View
43 lib/groupie.js
@@ -2,13 +2,13 @@
var sys = require('sys');
/**
- * Executes a group of functions concurrently, invoking a callback when all have completed
+ * Executes a group of functions concurrently, invoking a callback when all have completed
* or when an error occurs. Errors occur when an executed function throws an unhandled
* Error or when an error is passed to the callback.
*
- * Results are returned to the callback in the order that they are declared. The results
+ * Results are returned to the callback in the order that they are declared. The results
* of functions that complete after an error have occurred are discarded.
- *
+ *
* group([function(done){ done(null, 1); }, function(done){ done(null, 2); }], function(err, results){});
* or
* var g = group(function(err, results) {});
@@ -31,9 +31,9 @@ exports.group = function () {
var callGroupFunction = function(i, fxn) {
var done = function(err, result) {
if (errOccurred) return;
- if (err) {
+ if (err) {
errOccurred = true;
- return cb(err, results);
+ return cb(err, results);
}
results[i] = result;
items_left_to_execute--;
@@ -41,7 +41,7 @@ exports.group = function () {
cb(null, results);
}
};
-
+
try {
fxn(done);
} catch (err) {
@@ -49,14 +49,14 @@ exports.group = function () {
}
};
-
+
if (fxns.length === 0 && !open)
cb(null, []);
-
+
for ( var i = 0; i < fxns.length; i++) {
callGroupFunction(i, fxns[i]);
}
-
+
if (open)
return {
add: function(fxn) {
@@ -85,33 +85,36 @@ exports.chain = function () {
var finalized = !open;
fxns = fxns || [];
-
+
var pos = 0;
+ var isThinking = false;
var results = [];
-
+
var hasMore = function() {
return fxns.length > pos;
};
-
+
var callNext = function() {
var done = function(err, result) {
+ isThinking = false;
if (err) return cb(err, results);
-
- pos++;
+
+ pos++;
results.push(result);
-
- if (hasMore())
+
+ if (hasMore())
callNext();
else if (finalized)
cb(null, results);
};
try {
+ isThinking = true;
fxns[pos](done);
} catch (err) {
cb(err, results);
}
};
-
+
if (fxns.length > 0)
callNext();
else if (!open)
@@ -121,11 +124,11 @@ exports.chain = function () {
return {
add: function(fxn) {
fxns.push(fxn);
- if (hasMore()) callNext();
+ if (! isThinking) callNext();
},
finalize: function() {
finalized = true;
- if (!hasMore()) cb(null, results);
+ if (! isThinking) cb(null, results);
}
}
-}
+}
Something went wrong with that request. Please try again.