Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Ensure that polling transports work with Connection: close

  • Loading branch information...
commit 2075307f23f630f973e4c7fe9bf5b29216da0db7 1 parent d7b06ed
@rauchg rauchg authored
Showing with 12 additions and 0 deletions.
  1. +12 −0 lib/transports/http-polling.js
View
12 lib/transports/http-polling.js
@@ -44,6 +44,18 @@ HTTPPolling.prototype.__proto__ = HTTPTransport.prototype;
HTTPPolling.prototype.name = 'httppolling';
/**
+ * Override setHandlers
+ *
+ * @api private
+ */
+
+HTTPPolling.prototype.setHandlers = function () {
+ HTTPTransport.prototype.setHandlers.call(this);
+ this.socket.removeListener('end', this.bound.end);
+ this.socket.removeListener('close', this.bound.close);
+};
+
+/**
* Removes heartbeat timeouts for polling.
*/

9 comments on commit 2075307

@crickeys

This breaks disconnectSync from ever working.

@crickeys

specifically line 55

@rauchg
Owner

Can you elaborate please ?

@crickeys

Sorry, I posted about this a while ago in a lot of places, so here's my summary. If you use one of the HTTPPolling transports and you close a browser tab or the browser the disconnect event never registers on the server. For details please see this where I did as much research as possible:

#461

and this for a test
Automattic/socket.io-client#431

@rauchg
Owner

I see, so you narrowed it down to this commit. We definitely need to add tests for disconnectSync

@crickeys

@guille the fix I added in socket.io-client for properly sending disconnectSync only works if line 55 above is commented out. I'm happy to add a pull request with that line commented out but I don't know enough about why it was there in the first place. In production I currently HAVE to have it commented out to work properly. Any ideas what that line does or why it was added?

@rauchg
Owner

Yeah basically what's happening in your tests (methinks) is that the disconnectSync is being sent over a different TCP socket from the long-poll requests, and the transport is (intentionally) ignoring them. This was a protection to avoid client-side race conditions, but I'm pretty sure we can safely remove it.

@utan

I am sorry if I am reopening an old bug, but i can confirm that disconnect event works on IE <= 9 if line 55 in this commit is commented out (http-polling.js ).. using xhr-polling on the mentioned IE versions doesn't fire at all.. Only flashsocket works fine....

@ivesbrito

hey people, i have the same problem, i ll try comment the line 55, ty

Please sign in to comment.
Something went wrong with that request. Please try again.