xhr-polling: disconnect -> connect

[ftrippel]

The behavior with (connect ->) disconnect -> connect in xhr-polling is undesirable/error-prone. It once also produced an error on the server-side (sth. like session not found).

I don't know how facebook does it, but I think they implement their chat application foremost with long polling (even getting rid of the annoying pageload animation of the cursor on chrome). So far, I only found websocket/flashsocket to be stable enough for production.

[ftrippel]

My mistake: forget about the "session not found". I was using nginx to tcp/level 4 load balance between two node instances and that broke the server-side. However, the main issue remains and it would be great if the xhr-polling/multicast would be stable/fault tolerant.

[bgrieder]

Agreed, xhr-polling is not really usable as it is.
I have been playing with xhr-polling (firefox 4.0.1 OSX) and found that:

1- on the client side: calling socket.disconnect() does NOT stop the polling when it should.

2-every new call/poll to the server, creates a new session on the server and a memory leak by piling up listeners.

3-if the connection is "closed" (I know this not officially supported) on the server using something like try {this.socket._onDisconnect(); } catch(e) {}, the session is removed, thus avoiding the leak, and listener.js logs "Couldnt find client with session id xxxx" on each new poll.

4-closing the connection on the server side does not stop polling though, as there is no detection on the client side that a connection has been closed on the server side [lack of detection is true for all transports as far as I can tell and should be improved IMHO]

5-about xhr-polling, an improvement to the 'check' method in 'listener.js' would be the ability to send back an HTTP 400 status when the connection is destroyed server side. Hopefully this would make the client polling fail and provide a 'detection' of a failed connection server side.

bruno

[rdzar]

The connection and session needs to be kept far away from each other. It would be great to be able to just disconnect it and connect it again, with the session still remaining (id based, lock/unlock). If the user disconnects for real (no heartbeat lost, timeout or xhr polling failure) then the session needs to be deleted (disconnect event emitted), but otherwise it needs to "pick it up" again (delay disconnect event, so they are able to reconnect on current session?). Like you just lost your connection for a second, no worries, you may lost some chat messages, but we can handle that with queue's on clients/sessions.

With 3G test on the iPad in mind, the current way is not as stable as desired (websocket-like).

[3rd-Eden]

@ftrippel

The behavior with (connect ->) disconnect -> connect in xhr-polling is undesirable/error-prone.

How is undesirable? That is how long polling works.. Create a XHR request, to the server and the server holds it as long as possible. Once the request has been answered the clients starts a new XHR request.

(even getting rid of the annoying pageload animation of the cursor on chrome)
That was solved in the latest release.

@bgrieder

1- on the client side: calling socket.disconnect() does NOT stop the polling when it should.
I'm unaware of this issue, is reproducible?

2-every new call/poll to the server, creates a new session on the server and a memory leak by piling up listeners.
This is already solved on the server.

4-closing the connection on the server side does not stop polling though, as there is no detection on the client side that a connection has been closed on the server side [lack of detection is true for all transports as far as I can tell and should be improved IMHO]

Will be fixed in 0.7

@renedx

The connection and session needs to be kept far away from each other. It would be great to be able to just disconnect it and connect it again, with the session still remaining (id based, lock/unlock).

This would cause memory leaks, as we need to keep the user's details in memory while he refreshes the connection. I don't think this would be right way to solve this..

[rdzar]

@3rd-Eden

This would cause memory leaks, as we need to keep the user's details in memory while he refreshes the connection. I don't think this would be right way to solve this..

You keep it until.. timeout time? Like 8000 ms, no reconnect > garbage. I don't think that is such a huge issue. But at the end I'm not seeing how that will cause memory leaks either. The memory is still used the right way, because you keep sessions from connections. The session is still awaiting new connection, if no connection becomes available, the session go's right into garbage.

So, could je explain me were you see the memory leaking in my idea? Maybe I'm not seeing it the right way :)

[bgrieder]

Thanks for the prompt reply.

@bgrieder

1- on the client side: calling socket.disconnect() does NOT stop the polling when it should.
I'm unaware of this issue, is reproducible?

If I have time, I will try to build a small case, but looking at the stack trace and the code in xhr-polling.js, a call to self_.get() triggers a readyState change on this._xhr which calls self_get(), etc... Since the server is always returning 200 - whatever the state of the connection (is this what you will fix in 0.7?) - the code goes into an infinite loop.

Should not the test be something like if (status == 200 && (true == self.base.connected || self.base.connecting) ){

original_code

XHRPolling.prototype._get = function() {
    var self = this;
    this._xhr = this._request(+new Date, 'GET');
    this._xhr.onreadystatechange = function() {
        var status;
        if (self._xhr.readyState == 4) {
            self._xhr.onreadystatechange = empty;
            try {
                status = self._xhr.status;
            } catch (e) {
            }
            if (status == 200) {
                self._onData(self._xhr.responseText);
                self._get();
            } else {
                self._onDisconnect();
            }
        }
    };
    this._xhr.send(null);
};

2-every new call/poll to the server, creates a new session on the server and a memory leak by piling up listeners.
This is already solved on the server.

Is that available in Git Hub ?

4-closing the connection on the server side does not stop polling though, as there is no detection on the client side that a connection has been closed on the server side [lack of detection is true for all transports as far as I can tell and should be improved IMHO]

Will be fixed in 0.7

How far is 0.7 ?

@renedx

The connection and session needs to be kept far away from each other. It would be great to be able to just disconnect it and connect it again, with the session still remaining (id based, lock/unlock).

This would cause memory leaks, as we need to keep the user's details in memory while he refreshes the connection. I don't think this would be right way to solve this..

renedx replied before I had time to answer, but I believe he is right: connections and sessions are two different animals and sessions should be GCed when they expire. This would match 'standard' session management where you can reconnect to a non expired session by passing one way or another the session id (+ usual tricks to avoid session hijacking).

bruno