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

Question - return response data from server on client publish #390

Open
michDostal opened this issue Jul 10, 2015 · 13 comments
Open

Question - return response data from server on client publish #390

michDostal opened this issue Jul 10, 2015 · 13 comments

Comments

@michDostal
Copy link

Hello,
I'm trying create websocket api for my new project but i cannot solve one thing.

Basic communication is working well (publish/subscribe). But I need in some times on publish to specific channel return response with data to user while user is waiting for those data.

for client side I found:
var resp = client.publish('/channel',data);

resp.then(function() {
alert('Message received by server!');
}, function(error) {
alert('There was a problem: ' + error.message);
});

but I cannot figure out how to pass data from node.js server to this response...

I will need something like this:

fayeAdapter.on('publish', function(clientId,channel,data){
if(channel == 0)
return {data:'some data'};
});

and then at client this

resp.then(function(data) {
alert(data);
}, function(error) {
alert('There was a problem: ' + error.message);
});

Thank you and have a nice day.

@jcoglan
Copy link
Collaborator

jcoglan commented Jul 11, 2015

I will need something like this:

fayeAdapter.on('publish', function(clientId,channel,data){
  if(channel == 0)
    return {data:'some data'};
});

and then at client this

resp.then(function(data) {
  alert(data);
}, function(error) {
  alert('There was a problem: ' + error.message);
});

You can't return anything from an event listener; it will be ignored by the server. The way to 'respond' to a published message is to publish your own message, on a channel the original sender is subscribed to. Or, just make a regular HTTP request if all you want is a request/response semantic.

@michDostal
Copy link
Author

thank you for reply, I don't want use xhr requests because they are slow on our server which means unusable for my application... that's why i'm trying to replace them with websockets. Sometimes I need to wait for data from server... I tried create temporary channel for response but I couldn't figure out how to wait for message on this channel.

@jcoglan
Copy link
Collaborator

jcoglan commented Jul 17, 2015

I'm not sure what you mean by "how to wait for message on this channel" -- could you explain what you're trying to do in more detail?

@RonB
Copy link

RonB commented Aug 12, 2015

One up for this request.
The Bayeux specs allow the server to respond to a published message, so there should be a way in Faye to retrieve the response.

Our message server (java/cometd) implementation supports rooms which we can join by publishing a message on the " /service/room/join" service channel, containing the room id.
If succesfull the response from the server is a message with room id, members and recent messages. I would like to proces this message in the callback function passed with the then function of the returned promise. I.e (ipMessage is the Faye client):

ipMessage.publish('/service/room/join', {
    roomId: id
}).then(function(message) {
      $scope.rooms.push(message.data.room);
});

Unfortunaltely the promise of a subscribe operation shows the same behaviour. The response from the serve ris not passed to the resolved promise:

    ipMessage.subscribe('/chat/' + id, function(message) {
        // process chat messages
    }).then(function(message) {
       // process the subscription response, ie roominfo
       $scope.rooms.push(message.ext.room)
    }); 

@shawn-simon
Copy link

I'm having the same problem @RonB... No idea why you can't do this... The data is even sent down to the client, it's just not passed into the promise. I will have to fork the project and fix this...

@RonB
Copy link

RonB commented Aug 31, 2015

@shawn-simon I'd be glad to help out if you want

@shawn-simon
Copy link

@RonB Ah thanks! But I wound up going with socket.io after trying it out. The API fits my application a bit better.

@thesmart
Copy link

thesmart commented Oct 5, 2015

I assumed Faye would solve this as well. @jcoglan Faye docs say "Faye is based on the Bayeux protocol, and is compatible with the reference implementation of Bayeux provided by the cometD project." Bayeux spec from cometD says that /service/** should be usable for request/response purpose: http://docs.cometd.org/reference/bayeux_service_channel_operation.html

I don't see code in the Faye repo for handling req/resp to service channels. Is this a supported feature, or are patches needed to support this?

@jcoglan
Copy link
Collaborator

jcoglan commented Oct 18, 2015

@House-MD said:

Sometimes I need to wait for data from server... I tried create temporary channel for response but I couldn't figure out how to wait for message on this channel.

@RonB said:

The Bayeux specs allow the server to respond to a published message, so there should be a way in Faye to retrieve the response.

These comments touch on part of why the publish() promise fulfils with no value: it's not clear what a 'response' from the server means.

The spec says that the server may respond to a published message with a publish event acknowledgement containing the fields channel, successful, id, error and ext.

This is not a new message with its own application data; it has no data field. It is just an acknowledgement that the server processed the message.

Given this, it's not clear how a server should respond to a message published by a client. Should it include a data field in its acknowledgement, or publish to some other channel on which the client is subscribed? If the former, then a Bayeux client can automatically map the response to the request; if the latter then it's up to the application developer.

It's also not clear that 'response' means one message. Given pub/sub semantics it's possible for any peer to send many or no logical responses to some message it receives. If the server is allowed to respond many or no times, then a promise is not a good model for this response; it makes more sense as an event listener or stream.

Finally it's not clear that the server must respond immediately. Say a client publishes to a channel and it takes the server a very long time to compose a response, say, a time longer than the client's connection timeout. Should this delay the sending of the acknowledgement? If the Faye client doesn't receive timely confirmation of a published message, it will retry it. If we need to wait a long time to create the data field for a response then the client will become confused.

Because of the above, when I have wanted any semantics where one peer Alice (including the server) needs to 'respond' to another Bob, I have had Alice publish messages to some channel Bob is subscribed to. It allows for zero or multiple responses, and responses that take a long time, and doesn't rely on non-standard fields being added to protocol messages.

Even if we ignore all the above and constrain publish() to just yielding the acknowledgement message, whatever that is, that causes an inconsistency in the API. If the publication fails, Client will reject the promise with the parsed error. That is, if the server responds with:

{
  "id": "1",
  "channel": "/foo",
  "successful": false,
  "error": "405:/foo:Invalid channel"
}

Then the promise is rejected with an error object with the fields:

{
  code: 405,
  params: ['/foo'],
  message: 'Invalid channel'
}

This is the parsed error field from the acknowledgement. This is one way in which Client does not expose any protocol or serialization details to the user. It does not yield the entire acknowledgement. But if the promise did yield the entire acknowledgement on success, it would be confusing for it not to
do so on failure. Making them consistent would require a breaking change, and it would hand users the serialized copy of the error field rather than the parsed object they get now. This breaking change could be avoided if we allow acknowledgements to contain a data field, and only expose that via the promise rather than the whole message, which would be consistent with subscribe() listeners.

For reasons given in #385 and #400 and elsewhere (and the above comments are relevant to #407 which I've not got to yet) the promises returned by Client event listeners and promises do not expose Bayeux protocol messages. Client only exposes a high-level interface and application data, and hides protocol data, and if you need access to protocol data you should use extensions.

@jcoglan
Copy link
Collaborator

jcoglan commented Oct 18, 2015

@RonB said:

Our message server (java/cometd) implementation supports rooms which we can join by publishing a message on the " /service/room/join" service channel, containing the room id. If succesfull the response from the server is a message with room id, members and recent messages.

Could you clarify why it's done this way? Presumably to join a room, a client also needs to subscribe to a channel. How does it do that; does it subscribe itself or does the server subscribe it on receipt of the /service/room/join message? Why doesn't joining a room happen by a client simply subscribing to its channel, and having the server attach the member list (which would also need a subscription to track people joining/leaving) and latest messages to the /meta/subscribe response?

If the client has to do all this in multiple steps there's a greater chance of race conditions that cause the client to miss messages between requesting the room details and subscribing for updates.

@jcoglan
Copy link
Collaborator

jcoglan commented Nov 7, 2015

Did anyone have any feedback on my comments and questions above?

@thesmart
Copy link

All good points. I think specs have their place, and are great for galvanizing the development of clients for a common purpose. However, when the spec falls short, the implementation should take liberties. For example, I think its fine that a published event (i.e. "request") on the /service/** channels return a data attribute to the promise resolver. This data could be returned on the ext node, but I don't see a problem with adding a new attribute because it seems like an oversight of the spec. I also don't have issue with the error handlers not being great at handling request errors. In my extensions, I just wrote some additional error codes for the error states I cared about most.

When dealing with requests that don't have an immediate outcome, I just had the server publish events on the channels. In these "slow response" cases, I consider the request to be saying to the server "do something eventually" and the response to that request to be "yes, I'll considering doing something eventually" and relying on my inbound extensions to handle the rest.

Regarding joining channels versus having presence in a room, the two have different use cases. A channel is used for scoping the broadcast of messages to particular a subscriber set. Presence in a room is slightly different, in that my request to receive messages on a channel doesn't necessary imply that I wish to be known as present to all other clients. Is see the two as potentially over-lapping domains, but not 1:1.

I ended up writing a session extension and presence extension that maps clientIds and session tokens (users) and what I call "spaces" together. It was quite difficult tracking client connections / disconnections. For example, I noticed that clients that disconnect without broadcasting disconnect are are silently dropped from channels without an outbound disconnect message. So in fact, using channels for presence isn't obvious nor is the spec clear about how to handle client timeouts due to disconnection.

@jcoglan
Copy link
Collaborator

jcoglan commented Jun 21, 2016

Sorry I've not got back to this issue in a while. I'm now catching up before shipping 1.2.0.

For reasons I gave in #400, I'm not going to add support for the subscribe() promise yielding a data field from the response. Briefly, it's because that call doesn't correspond to a single call to the server, especially in the event of disconnection, and the proposed API in that issue would be misleading to users. But see my comment there for full details.

Regarding yielding data from a response to publish(), @thesmart said:

I think specs have their place, and are great for galvanizing the development of clients for a common purpose. However, when the spec falls short, the implementation should take liberties. For example, I think its fine that a published event (i.e. "request") on the /service/** channels return a data attribute to the promise resolver.

This is a fair point but I want to know what other servers actually do. Faye does not return a data field in any server responses and so the client won't see any difference when talking to a Faye server. But I also want to keep the client compatible with other Bayeux implementations, that is, it should follow the spirit of the spec.

Do other servers return data in response messages? If so, we have a stronger case for having the Faye client yield this to users. Do you have any examples of servers that do this?

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

No branches or pull requests

5 participants