SocketCluster / socketcluster-client

JavaScript client for SocketCluster
MIT License
291 stars 92 forks source link

Perform disconnect on unload rather than beforeunload #62

Closed Kequc closed 6 years ago

Kequc commented 7 years ago

Ref: #49

Frees up beforeunload event for use by developer. For example to display a warning to the user that they are leaving the page and will lose their connection.

jondubois commented 7 years ago

@Kequc Hmmm, that solution is not ideal. The unload event is not as reliable as beforeunload (in several browsers. See http://stackoverflow.com/questions/4376596/jquery-unload-or-beforeunload) - This means that the server would get a lot of 'Socket Hung Up' issues because the clients would not disconnect correctly when the user navigates to a different page (for example).

Socket.io uses the same technique as we do currently. See https://github.com/socketio/socket.io-client/blob/master/socket.io.js#L3663

To solve your use case, we could add an extra option when creating the socket client:

socketCluster.connect({
  // ...
  disconnectOnUnload: true
});

true would be the default, but in your case you could set it to false... Then, it would be your responsibility to call socket.disconnect() when the user leaves the page.

Kequc commented 7 years ago

I see. It seemed that because the attachEvent method was using unload, it made more sense and was a typo. I guess I was wrong. That feature would be desirable.

jondubois commented 7 years ago

Yep, the attachEvent is for IE, which is better at handling the unload event. When I tested it (a while ago) I think the issue with the unload event affected Firefox.

jondubois commented 7 years ago

I'll make the fix for you now.

Kequc commented 7 years ago

ooh! I was gonna go do it. Thanks.

jondubois commented 7 years ago

@Kequc Ok, v5.0.9 has the fix https://www.npmjs.com/package/socketcluster-client - You can just pass the disconnectOnUnload: false option.

Kequc commented 7 years ago

How do I know if the user blocked page unload? In your opinion where should I place my disconnect statement here?

window.addEventListener('beforeunload', (e) => {
    let message = "Your connection will be interrupted.";
    e.returnValue = message;
    return message;
});

Thanks for the code update!

jondubois commented 7 years ago

@Kequc Not sure, it depends how you want to implement it.

Kequc commented 7 years ago

It seems... Not possible with the current solution. There isn't a way to know whether the user clicked "ok" in the resulting dialog. They are simply and immediately taken off the page, this is in order to prevent abuse by websites that would rather you are unable to navigate away.

Therefore, with disconnectonunload disabled, navigating away from the page always results in socket hung up errors unfortunately.

For reference: (beforeunload)[https://developer.mozilla.org/en/docs/Web/Events/beforeunload]