Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feedback wanted] Move method call events under single event: 'methodCall' #57

Open
baalexander opened this issue Aug 24, 2012 · 3 comments
Labels

Comments

@baalexander
Copy link
Owner

This came out of the discussion from Pull Request #56.

There was a need to be able to respond to a listening and close event from the XML-RPC server. However, server.on('listening') could cause a conflict if the XML-RPC method call's name was listening. One proposed solution is to change the API to something like:

server.on('methodCall', function(method, args) {
  // switch based on method name
});
server.on('listening', function() {
})
server.on('close', function() {
});
server.on('error', function(error) {
});

The disadvantage is a big API change. The advantage is other events can be emitted (listening, close, etc.) and not be confused with method names.

@timoxley
Copy link
Contributor

timoxley commented Sep 4, 2012

Hm, this isn't what I was thinking at all. That switch you propose is messy.

This is what I was thinking:

server.on('listening', function() {
})
server.on('close', function() {
});
server.on('error', function(error) {
});

// api calls are properties
server.doSomething = function(param1, param2, callback) {
// note lack of single params argument

}

This solves 3 gripes I have with the api at the moment:

  1. Triggering methods off events, as discussed in Add listening/close events, and a close() function #56
  2. the err as first param in every method doesn't add much value (as far as I can see) since it appears to be fixed to null.
  3. params as a single array in the arguments. They should come in as actual params to the method, with callback always being last param. This is a pretty common convention for node.

@baalexander
Copy link
Owner Author

The plan is for the API for 2.0 to look very similar to @timoxley's protocol above.

One tweak, from pull request #60, is there should be a way to listen to a method call without knowing the name. It could be an unhandled event if no property function, a methodCall that's always fired (but still handled via setting server.doSomething), or something else.

Edit: This unhandled-like event could allow for a 404 response or a custom response if needed.

@mbush92
Copy link

mbush92 commented Oct 30, 2016

Any update on when 2.0 will be coming out or of there are still plans for a 2.0? We are building a rather large RPC library for use with industrial robots and like what I see in the posts above regarding a V2.0 as it would give us more control when we have multiple robots communicating to the RPC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants