js/core: Refactored and shared code between the cb and promise case
for client and async-call.
Change-Id: If26f53f7a0b9468b505d6c4569f30274ce3328d1
diff --git a/src/lib/async-call.js b/src/lib/async-call.js
index 7fd8052..ee9b92c 100644
--- a/src/lib/async-call.js
+++ b/src/lib/async-call.js
@@ -5,11 +5,16 @@
// This file defines a async calling convention intended used to call
// user-defined functions.
-var verror = require('../gen-vdl/v.io/v23/verror');
var logger = require('../lib/vlog').logger;
+var makeError = require('../verror/make-errors');
+var actions = require('../verror/actions');
module.exports = asyncCall;
+var IncorrectResultCountError = makeError(
+ 'v.io/core/javascript.IncorrectResultCount',
+ actions.NO_RETRY,
+ '{1:}{2:} IncorrectResultCount: Expected {3} results, but got {4}{:_}');
/**
* asyncCall performs a call and calls a callback with the result.
*
@@ -34,58 +39,30 @@
logger.error('Callback called multiple times');
return;
}
- inputCb.apply(this, arguments);
+ inputCb.apply(self, arguments);
cbCalled = true;
}
- // Call the callback and log the error. Used for internal errors.
- function asyncFailedCb(err) {
+ function handleResult(err, res) {
+ if (err) {
+ // Error case
+ return callOnceCb(err);
+ }
+ // Results case
+ var numResults = res.length;
+ if (numResults === numOutArgs) {
+ // Correct number of out args given
+ return callOnceCb(null, res);
+ }
+ // Internal error: incorrect number of out args
+ err = new IncorrectResultCountError(ctx, numOutArgs, numResults);
logger.error(err);
callOnceCb(err);
}
-
// Callback we are injecting into the user's function
function injectedCb(err /*, args */) {
- var res = Array.prototype.slice.call(arguments, 1);
-
- if (err) {
- // Error case
- callOnceCb(err);
- } else {
- // Results case
- var numResults = res.length;
- if (numResults === numOutArgs) {
- // Correct number of out args given
- callOnceCb(null, res);
- } else {
- // Internal error: incorrect number of out args
- asyncFailedCb(makeIncorrectArgCountError(true, outArgs, numResults));
- }
- }
+ handleResult(err, Array.prototype.slice.call(arguments, 1));
}
- // Creates an error when there are an incorrect number of arguments.
- // TODO(bjornick): Generate a real verror for this so it can be
- // internationalized.
- function makeIncorrectArgCountError(isCb, expectedArgs, numGiven) {
- var delta = numGiven - expectedArgs.length;
- var prefix;
- if (isCb) {
- prefix = 'Callback of form cb(err,' + expectedArgs + ')';
- } else {
- prefix = 'Expected out args ' + expectedArgs + ' but';
- }
-
- var suffix;
- if (delta < 0) {
- suffix = 'was missing ' + expectedArgs.slice(numGiven);
- } else {
- suffix = 'got ' + delta + ' extra arg(s)';
- }
-
- return new verror.InternalError(ctx, prefix, suffix);
- }
-
-
if (fn.hasCallback()) {
args.push(injectedCb);
}
@@ -113,35 +90,31 @@
//
// Convert the results to always be in array style:
// [], [a], [a, b], etc
- var resAsArray;
+ // Note that the arity checking isn't done here, but at a later point
+ // sharing the logic between the callback and promise case.
switch (numOutArgs) {
case 0:
if (res !== undefined) {
- return asyncFailedCb(new verror.InternalError(ctx,
- 'Non-undefined value returned from function with 0 out args'));
+ return Promise.reject(
+ new IncorrectResultCountError(ctx, 0, 1,
+ 'expected undefined result ' +
+ 'for void function'));
}
- resAsArray = [];
- break;
+ return [];
case 1:
// Note: If res is undefined, the result is [undefined].
- resAsArray = [res];
- break;
+ return [res];
default:
if (!Array.isArray(res)) {
- asyncFailedCb(new verror.InternalError(
- ctx,
- 'Expected multiple out arguments to be returned in an array.'));
+ return Promise.reject(
+ new IncorrectResultCountError(ctx, numOutArgs, 1));
}
- resAsArray = res;
- break;
+ return res;
}
- var numResults = resAsArray.length;
- if (numResults !== numOutArgs) {
- asyncFailedCb(makeIncorrectArgCountError(false, outArgs, numResults));
- }
- callOnceCb(null, resAsArray);
- }).catch(function error(err) {
- callOnceCb(wrapError(err));
+ }).then(function(res) {
+ handleResult(null, res);
+ }).catch(function(err) {
+ handleResult(wrapError(err));
});
}
@@ -150,13 +123,12 @@
* This is used in cases where values are known to be errors even if they
* are not of error type such as if they are thrown or rejected.
* @private
- * @param {Error} err The error or other value.
+ * @param {*} err The error or other value.
* @return {Error} An error or type Error.
*/
function wrapError(err) {
- if (!(err instanceof Error)) {
- return new Error(err);
- } else {
+ if (err instanceof Error) {
return err;
}
+ return new Error(err);
}
diff --git a/src/rpc/client.js b/src/rpc/client.js
index 840b3ae..e816fb5 100644
--- a/src/rpc/client.js
+++ b/src/rpc/client.js
@@ -99,68 +99,61 @@
var ctx = this._ctx;
var self = this;
- var cb;
var outArgTypes = this._outArgTypes;
- if (this._cb) {
- // Wrap the callback to call with multiple arguments cb(err, a, b, c)
- // rather than cb(err, [a, b, c]).
- var origCb = this._cb;
- cb = function convertToMultiArgs(err, results) { // jshint ignore:line
- // If called from a deferred, the results are undefined.
+ var def = new Deferred();
+ var cb = this._cb;
+ var promise = def.promise.then(function(args) {
+ if (!Array.isArray(args)) {
+ throw new verror.InternalError(
+ ctx, 'Internal error: incorrectly formatted out args in client');
+ }
- if (err) {
- origCb(err);
- return;
+ return Promise.all(args.map(function(outArg, i) {
+ return convertOutArg(ctx, outArg, outArgTypes[i], self._controller);
+ }));
+ }).then(function(results) {
+ if (cb) {
+ // Make a copy the results to so we can push a null for the
+ // error onto the front of the arg list.
+ var cbArgs = results.slice();
+ cbArgs.unshift(null);
+ try {
+ cb.apply(null, cbArgs);
+ } catch (e) {
+ process.nextTick(function() {
+ throw e;
+ });
}
-
- // Each out argument should also be unwrapped. (results was []any)
- results = results || [];
- var resultPromises = results.map(function(res, i) {
- return convertOutArg(ctx, res, outArgTypes[i], self._controller);
- });
- Promise.all(resultPromises)
- .then(function(results) {
- results.unshift(null);
- origCb.apply(null, results);
- }).catch(origCb);
- };
- }
-
- var def = new Deferred(cb);
-
- if (!this._cb) {
+ }
// If we are using a promise, strip single args out of the arg array.
// e.g. [ arg1 ] -> arg1
- def.promise = def.promise.then(function(args) {
- if (!Array.isArray(args)) {
- throw new verror.InternalError(ctx,
- 'Internal error: incorrectly formatted out args in client');
+ switch(results.length) {
+ // We expect:
+ // 0 args - return; // NOT return [];
+ // 1 args - return a; // NOT return [a];
+ // 2 args - return [a, b] ;
+ //
+ // Convert the results from array style to the expected return style.
+ // undefined, a, [a, b], [a, b, c] etc
+ case 0:
+ return undefined;
+ case 1:
+ return results[0];
+ default:
+ return results;
+ }
+ });
+
+ if (cb) {
+ promise.catch(function(err) {
+ try {
+ cb(err);
+ } catch(e) {
+ process.nextTick(function() {
+ throw e;
+ });
}
-
-
- // Each out argument should also be unwrapped. (args was []any)
- var unwrappedArgPromises = args.map(function(outArg, i) {
- return convertOutArg(ctx, outArg, outArgTypes[i], self._controller);
- });
-
- return Promise.all(unwrappedArgPromises).then(function(unwrappedArgs) {
- // We expect:
- // 0 args - return; // NOT return [];
- // 1 args - return a; // NOT return [a];
- // 2 args - return [a, b] ;
- //
- // Convert the results from array style to the expected return style.
- // undefined, a, [a, b], [a, b, c] etc
- switch(unwrappedArgs.length) {
- case 0:
- return undefined;
- case 1:
- return unwrappedArgs[0];
- default:
- return unwrappedArgs;
- }
- });
});
}
@@ -171,7 +164,7 @@
// inStreamingType.
def.stream = new Stream(this._id, streamingDeferred.promise, true,
this._outStreamingType, this._inStreamingType, this._typeEncoder);
- def.promise.stream = def.stream;
+ promise.stream = def.stream;
}
var message = this.constructMessage();
@@ -187,7 +180,7 @@
});
}
- return def.promise;
+ return promise;
};
OutstandingRPC.prototype.handleResponse = function(type, data) {
diff --git a/test/unit/test-async-call.js b/test/unit/test-async-call.js
index e92ead6..f17e96b 100644
--- a/test/unit/test-async-call.js
+++ b/test/unit/test-async-call.js
@@ -250,7 +250,7 @@
var inspectFn = new InspectableFunction(returnNonUndefined);
asyncCall(null, null, inspectFn, [], [], function(err) {
t.ok((''+err).indexOf(
- 'Non-undefined value returned from function with 0 out args') !== -1,
+ 'Expected 0 results, but got 1') !== -1,
'Expected error when non-undefined value returned');
t.end();
});
diff --git a/test/unit/test-caveat-validator-registry.js b/test/unit/test-caveat-validator-registry.js
index f43a275..27f105b 100644
--- a/test/unit/test-caveat-validator-registry.js
+++ b/test/unit/test-caveat-validator-registry.js
@@ -136,7 +136,7 @@
testCaveats.CaveatDoesntValidateExpectedData), function(err) {
t.ok(err, 'Got an error');
t.ok((''+err).indexOf(
- 'Non-undefined value returned from function with 0 out args') !== -1,
+ 'Expected 0 results, but got 1') !== -1,
'Error due to non-undefined value returned from 0-arg function');
t.end();
});
diff --git a/test/unit/test-invoker-invoke.js b/test/unit/test-invoker-invoke.js
index 7239316..db3f3c1 100644
--- a/test/unit/test-invoker-invoke.js
+++ b/test/unit/test-invoker-invoke.js
@@ -633,9 +633,9 @@
context: context
}
}, function cb(err, results) {
- t.ok(err instanceof verror.InternalError, 'should error');
+ t.ok(err instanceof verror.VanadiumError, 'should error');
t.equal(err.message,
- 'app:op: Internal error: Expected out args a,b,c,d,e but was missing e');
+ 'app:op: IncorrectResultCount: Expected 5 results, but got 4');
t.end();
});
@@ -661,10 +661,9 @@
context: context
}
}, function cb(err, results) {
- t.ok(err instanceof verror.InternalError, 'should error');
+ t.ok(err instanceof verror.VanadiumError, 'should error');
t.equal(err.message,
- 'app:op: Internal error: ' +
- 'Callback of form cb(err,a,b,c,d,e) was missing e');
+ 'app:op: IncorrectResultCount: Expected 5 results, but got 4');
t.end();
});
@@ -690,10 +689,9 @@
context: context
}
}, function cb(err, results) {
- t.ok(err instanceof verror.InternalError, 'should error');
+ t.ok(err instanceof verror.VanadiumError, 'should error');
t.equal(err.message,
- 'app:op: Internal error: ' +
- 'Expected out args a,b,c,d,e but got 1 extra arg(s)');
+ 'app:op: IncorrectResultCount: Expected 5 results, but got 6');
t.end();
});
@@ -719,10 +717,9 @@
context: context
}
}, function cb(err, results) {
- t.ok(err instanceof verror.InternalError, 'should error');
+ t.ok(err instanceof verror.VanadiumError, 'should error');
t.equal(err.message,
- 'app:op: Internal error: ' +
- 'Callback of form cb(err,a,b,c,d,e) got 1 extra arg(s)');
+ 'app:op: IncorrectResultCount: Expected 5 results, but got 6');
t.end();
});
