Skip to content

Commit

Permalink
Fix nodejs#3379 prevent domain.intercept passing 1st arg to cb
Browse files Browse the repository at this point in the history
  • Loading branch information
wavded authored and isaacs committed Jun 9, 2012
1 parent 4b021a3 commit 36b5b02
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 3 deletions.
9 changes: 6 additions & 3 deletions doc/api/domain.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}));
}

Expand Down
28 changes: 28 additions & 0 deletions lib/domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 7 additions & 0 deletions test/simple/test-domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 36b5b02

Please sign in to comment.