Skip to content

Commit

Permalink
Request event fixed-length args. Fix #100.
Browse files Browse the repository at this point in the history
- Change Blizzard `request.*` event handlers to be 2-arity.
  The remote argument list as an array is the first argument.
  The reply function is the second argument.

- Update Yeti usage of these events to use the new signature.

- Use the request argument list for incomingBridge events
  as their actual arguments, but without a reply function.

`request.*` events for Blizzard handle remote RPC requests.
Sometimes the event will reply to the caller's side using
a reply function provided as the last argument.

A malformed RPC request with too many or too few arguments
caused the event handler to expect the reply function in
the wrong place in the argument list, yielding a crash.

To maintain compatibility with the majority of Yeti events
which use the incomingBridge API to bridge an EventEmitter event
to a Blizzard `request` event handler, the remote argument list
(first argument) is unpacked and the bridged event is fired
with that argument list. The reply function is not included.

The incomingBridge API is only intended for make a remote event
appear local -- not to handle RPC-specific behavior like a reply.
Yeti already follows this rule, so no further changes are needed.
  • Loading branch information
reid committed Oct 9, 2012
1 parent 08a1905 commit e8f514e
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 29 deletions.
4 changes: 3 additions & 1 deletion lib/blizzard/namespace.js
Expand Up @@ -29,7 +29,9 @@ BlizzardNamespace.prototype.outgoingBridge = function (emitter, event, emitterEv
};

BlizzardNamespace.prototype.incomingBridge = function (emitter, event, emitterEvent) {
this.on("request." + event, emitter.emit.bind(emitter, emitterEvent || event));
this.on("request." + event, function unpackArgs(remoteArgs, reply) {
emitter.emit.apply(emitter, [emitterEvent || event].concat(remoteArgs));
});
};

BlizzardNamespace.prototype.bind = function () {
Expand Down
29 changes: 20 additions & 9 deletions lib/blizzard/session.js
Expand Up @@ -197,13 +197,25 @@ BlizzardSession.prototype.outgoingBridge = function (emitter, event, emitterEven
/**
* Bridge an remote RPC event to the given EventEmitter.
*
* The remote RPC event's params (arguments) are unpacked
* instead of given as an array in the first argument.
* This makes an RPC event act like a local event.
*
* Unlike listeners to `request.*` events, the provided event
* will not recieve a reply callback because the listener argument
* list is variable. If you need to reply to a RPC request, listen
* to the event on this object's request namespace instead,
* taking care to unpack the first argument.
*
* @method incomingBridge
* @param {EventEmitter} emitter Target EventEmitter.
* @param {String} event Event name.
* @param {String} emitterEvent Optional event name for the target EventEmitter, if different.
*/
BlizzardSession.prototype.incomingBridge = function (emitter, event, emitterEvent) {
this.on("request." + event, emitter.emit.bind(emitter, emitterEvent || event));
this.on("request." + event, function unpackArgs(remoteArgs, reply) {
emitter.emit.apply(emitter, [emitterEvent || event].concat(remoteArgs));
});
};

/**
Expand Down Expand Up @@ -512,9 +524,11 @@ BlizzardSession.prototype.onIncomingJSON = function (id, jsonString) {
* @event request.**
* @description Fires when an RPC message is received from the remote side.
* Fired by `onMessage`.
* @param {Object} n Argument for the RPC. May be repeated.
* @param {Function} reply Reply function. Optional. Must be the last argument.
* Reply function takes two arguments: `err` and `data`.
* @param {Array} params Arguments from the remote event.
* @param {Function} reply Reply function to respond to the RPC.
* Note: the reply will only be sent if the remote side asked for a reply.
* @param {Boolean} reply.err Error response, null otherwise.
* @param {Object} reply.data Data response.
*/

/**
Expand Down Expand Up @@ -554,13 +568,10 @@ BlizzardSession.prototype.onMessage = function (id, message) {
// Params will become the final arguments to
// the `request.{message.method}` event.

// Push the reply function to the end of the args.
params.push(requestCompleter);

// Any listeners on request.{message.method}?
if (self.listeners(event).length) {
// Emit the event with the params as arguments.
self.emit.apply(self, [event].concat(params));
// Emit the request event with a fixed argument list.
self.emit.call(self, event, params, requestCompleter);
} else {
self.emit("fail", id,
BlizzardSession.ERROR_METHOD,
Expand Down
4 changes: 3 additions & 1 deletion lib/client.js
Expand Up @@ -144,7 +144,9 @@ ClientBatch.prototype.onAck = function (err, id) {
ClientBatch.prototype.provideTests = function () {
var self = this;

self.batchSession.on("request.clientFile", function (file, reply) {
self.batchSession.on("request.clientFile", function (args, reply) {
var file = args[0];

if (!self.basedir) {
reply("Not permitted.");
return;
Expand Down
6 changes: 3 additions & 3 deletions lib/hub/index.js
Expand Up @@ -130,11 +130,11 @@ Hub.prototype._setupEvents = function () {
self.agentManager.on("agentSeen", self.sessions.emit.bind(self.sessions, "rpc.agentSeen"));
self.agentManager.on("agentDisconnect", self.sessions.emit.bind(self.sessions, "rpc.agentDisconnect"));

self.sessions.on("request.batch", function (tests, reply) {
self.emit("batch", this.child, tests, reply);
self.sessions.on("request.batch", function (args, reply) {
self.emit("batch", this.child, args[0], reply);
});

self.sessions.on("request.agents", function (reply) {
self.sessions.on("request.agents", function (args, reply) {
var agentNames = [];
self.agentManager.getAvailableAgents().forEach(function (agent) {
agentNames.push(agent.getName());
Expand Down
46 changes: 31 additions & 15 deletions test/blizzard.js
Expand Up @@ -25,8 +25,8 @@ function blizzardEventTests(ns) {
topic: blizzard.rpcTopic({
method: "echo",
request: "foo"
}, function (data, reply) {
reply(null, data);
}, function (params, reply) {
reply(null, params[0]);
}),
"the response should be the same as the request": function (topic) {
assert.strictEqual(topic.req, topic.res);
Expand All @@ -37,8 +37,12 @@ function blizzardEventTests(ns) {
method: "binary",
request: "bar",
fixture: new Buffer(7068)
}, function (fixture, data, reply) {
reply(null, fixture);
}, function (fixture, params, reply) {
var err = null;
if (params[0] !== "bar") { // the request
err = new Error("Bad request, expected bar, got: " + params[0]);
}
reply(err, fixture);
}),
"the response should be a Buffer": function (topic) {
assert.ok(Buffer.isBuffer(topic.res));
Expand All @@ -50,35 +54,47 @@ function blizzardEventTests(ns) {
assert.deepEqual(topic.res, topic.fixture);
}
},
"with events responded to from the incomingBridge API": {
"with events from the incomingBridge API": {
topic: function (lastTopic) {
var vow = this,
context = {},
ee = new EventEmitter2();
ee.once("bridge1", function (data, reply) {
reply(null, data);
});
lastTopic.server.incomingBridge(ee, "bridge1");
lastTopic.client.emit("rpc.bridge1", "quux", function (err, res) {
vow.callback(err, {

// Note: the reply callback is no longer passed
// to event bridges with the incomingBridge API.

ee.once("bridge1", function (first, second) {
vow.callback(null, {
context: context,
event: this.event,
res: res
res: first,
reply: second // should be undefined
});
});
lastTopic.server.incomingBridge(ee, "bridge1");
lastTopic.client.emit("rpc.bridge1", "quux", function () {
// no-op callback, used to indicate that a reply
// is requested. We want to test that the reply
// callback, which normally is presented to request.*
// events, is not passed when bridged.
throw new Error("Unexpected reply.");
});
},
"the response is correct": function (topic) {
assert.strictEqual(topic.event, rpcPrefix + "bridge1");
assert.strictEqual(topic.event, "bridge1");
assert.strictEqual(topic.res, "quux");
},
"only one argument was given to the bridged event": function (topic) {
assert.isUndefined(topic.reply);
}
},
"with events sent by the outgoingBridge API": {
topic: function (lastTopic) {
var vow = this,
context = {},
ee = new EventEmitter2();
lastTopic.server.on("request.bridge2", function (data, reply) {
reply(null, data);
lastTopic.server.on("request.bridge2", function (params, reply) {
reply(null, params[0]); // data
});
lastTopic.client.outgoingBridge(ee, "bridge2");
ee.emit("bridge2", "dogcow", function (err, res) {
Expand Down

0 comments on commit e8f514e

Please sign in to comment.