Fix #3379 prevent domain.intercept passing 1st arg to cb

nodejs · Jun 9, 2012 · 569acea · 569acea
1 parent 4b021a3
commit 569acea
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 3 deletions.
diff --git a/doc/api/domain.markdown b/doc/api/domain.markdown
@@ -227,13 +227,16 @@ with a single error handler in a single place.
     var d = domain.create();
 
     function readSomeFile(filename, cb) {
-      fs.readFile(filename, d.intercept(function(er, data) {
+      fs.readFile(filename, d.intercept(function(data) {
+        // note, the first argument is never passed to the
+        // callback since it is assumed to be the 'Error' argument
+        // and thus intercepted by the domain.
+
         // if this throws, it will also be passed to the domain
-        // additionally, we know that 'er' will always be null,
         // so the error-handling logic can be moved to the 'error'
         // event on the domain instead of being repeated throughout
         // the program.
-        return cb(er, JSON.parse(data));
+        return cb(null, JSON.parse(data));
       }));
     }
 

diff --git a/lib/domain.js b/lib/domain.js
@@ -167,6 +167,34 @@ Domain.prototype.bind = function(cb, interceptError) {
       return;
     }
 
+    // remove first-arg if intercept as assumed to be the error-arg
+    if (interceptError) {
+      var len = arguments.length;
+      var args;
+      switch (len) {
+        case 0:
+        case 1:
+          // no args that we care about.
+          args = [];
+          break;
+        case 2:
+          // optimization for most common case: cb(er, data)
+          args = [arguments[1]];
+          break;
+        default:
+          // slower for less common case: cb(er, foo, bar, baz, ...)
+          args = new Array(len - 1);
+          for (var i = 1; i < len; i++) {
+            args[i] = arguments[i - 1];
+          }
+          break;
+      }
+      self.enter();
+      var ret = cb.apply(this, args);
+      self.exit();
+      return ret;
+    }
+
     self.enter();
     var ret = cb.apply(this, arguments);
     self.exit();

diff --git a/test/simple/test-domain.js b/test/simple/test-domain.js
@@ -127,6 +127,13 @@ function fn(er) {
 var bound = d.intercept(fn);
 bound(new Error('bound'));
 
+// intercepted should never pass first argument to callback
+function fn2(data) {
+  assert.equal(data, 'data', 'should not be null err argument')
+}
+
+var bound = d.intercept(fn2);
+bound(null, 'data');
 
 
 // throwing in a bound fn is also caught,