This does look like an issue. Or at least I think there is an issue here, but I’m not sure it’s your issue.
The documentation is clear:
Wrap a function that takes a callback function as its final parameter. On the server, the wrapped function can be used either synchronously (without passing a callback) or asynchronously (when a callback is passed). On the client, a callback is always required; errors will be logged if there is no callback.
So, we have the possibility of no callback in the final parameter, but if there is a callback it must be the final parameter. Or, to turn it on its head, “if the final parameter is a function, then it’s the callback”
My analysis of the code runs as follows (code section first, explanation following):
wrapAsync: function (fn, context) {
return function (/* arguments */) {
var self = context || this;
var newArgs = _.toArray(arguments);
var callback;
So, we begin with a copy of the arguments in an array (indexed from [0]) and an undefined
callback
variable. Then:
for (var i = newArgs.length - 1; i >= 0; --i) {
var arg = newArgs[i];
var type = typeof arg;
if (type !== "undefined") {
if (type === "function") {
callback = arg;
}
break;
}
}
We iterate over the arguments in reverse order (starting with the last and going to the first). As soon as we find a function, we break out of the loop with i at the position of the function and callback set to that function reference. If we don’t find a function, the loop runs to the end, and i has the final value of -1, with callback still undefined. Then:
if (! callback) {
if (Meteor.isClient) {
callback = logErr;
} else {
var fut = new Future();
callback = fut.resolver();
}
++i; // Insert the callback just after arg.
}
If we did find a function (callback), skip the above section (i will be at the position of the function in the newArgs array).
Otherwise, i will be -1 by this time and we could have any number of non-function parameters. Clients must use a callback, so we need to register an error. On the server, no callback means use a future. In all cases increment i (which means set to 0). Then:
newArgs[i] = Meteor.bindEnvironment(callback);
var result = fn.apply(self, newArgs);
return fut ? fut.wait() : result;
};
}
What’s going on here? The i-th parameter is overwritten with the callback (may be the actual callback, or the registered error callback, or a future). That’s fine if there were no parameters or if the final parameter was the callback. But what if this is on the server and there are several non-function parameters? In that scenario we don’t need to specify a callback, because a future will be provided for us. However, it looks as if newArgs[0]
will be given the future - but newArgs[0]
is one of our non-functional parameters and has just been destroyed. I think that line ++i; // Insert the callback just after arg.
is incorrect and should be: i = newArgs.length; // Insert the callback just after final parameter
.
What aren’t I seeing?