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

Stop silently closing socket on window beforeunload event #661

Closed
cenzovit opened this issue Apr 27, 2021 · 1 comment
Closed

Stop silently closing socket on window beforeunload event #661

cenzovit opened this issue Apr 27, 2021 · 1 comment
Labels
enhancement New feature or request

Comments

@cenzovit
Copy link

Would it be possible to not have this fail silently and/or disconnect with a specific unload reason? We use a socket as a heartbeat to clients and we formerly used the window unload event to fire off an intentional close of the socket to inform our server when it could begin cleaning up in-memory artifacts of the client sooner (rather than waiting for our socket disconnected retry timeouts to handle cleanup).

Describe the solution you'd like
Make the socket's beforeunload silent close either configurable/disable-able or make the "silent" close emit a disconnect with a new reason.

Describe alternatives you've considered
As it stand now, we would have to either offload this server side cleanup logic to a custom endpoint (somewhat defeating one of the reasons we use sockets in the first place) or add our own event listener to beforeunload to try to emit the intentional close before the silent close occurs (which will probably lead to race conditions?). Realistically, this is currently just preventing us from bumping our socket.io version past 3.1.1

@cenzovit cenzovit added the enhancement New feature or request label Apr 27, 2021
darrachequesne added a commit that referenced this issue May 4, 2021
Since [1], the socket is now closed when receiving the "beforeunload"
event in the browser.

This change was meant to fix a discrepancy between Chrome and Firefox
when the user reloads/closes a browser tab: Firefox would close the
connection (and emit a "disconnect" event, at the Socket.IO level), but
not Chrome (see [2]).

But it also closes the connection when there is another "beforeunload"
handler, for example when the user is prompted "are you sure you want
to leave this page?".

Note: calling "stopImmediatePropagation()" was a possible workaround:

```js
window.addEventListener('beforeunload', (event) => {
  event.preventDefault();
  event.stopImmediatePropagation();
  event.returnValue = 'are you sure you want to leave this page?';
});
```

This commit adds a "closeOnBeforeunload" option, which controls whether
a handler is registered for the "beforeunload" event.

Syntax:

```js
const socket = require('engine.io-client')('ws://localhost', {
  closeOnBeforeunload: false // defaults to true
});
```

[1]: ed48b5d
[2]: socketio/socket.io#3639

Related:

- #661
- #658
- socketio/socket.io-client#1451

Reference: https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event
@darrachequesne
Copy link
Member

The closeOnBeforeunload option was added in engine.io-client@5.1.0, in order to mitigate this issue.

Syntax:

const socket = require('engine.io-client')('ws://localhost', {
  closeOnBeforeunload: false // defaults to true
});

Thanks for the heads-up 👍

darrachequesne added a commit that referenced this issue Jun 28, 2023
Silently closing the connection when receiving a "beforeunload" event
is problematic, because it is emitted:

- when downloading a file from another host

Related: socketio/socket.io#4436

- when the user already has a listener for the "beforeunload" event
(i.e. "are you sure you want to leave this page?")

Related:

- #661
- #658
- socketio/socket.io#4065

That's why the `closeOnBeforeunload` option will now default to false.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants