socket.emit to invoke ack callback with an Error if the transport closes #1546
Comments
darrachequesne
added a commit
that referenced
this issue
Mar 14, 2024
Previously, getting disconnected while waiting for an acknowledgement
would create a memory leak, as the acknowledgement was never received
and the handler would stay in memory forever.
This commit fixes the issue:
- handlers that do accept an error as first argument, such as:
* `socket.emit("test", (err, value) => { ... })` with `ackTimeout` option
* `socket.timeout(5000).emit("test", (err, value) => { ... })`
* `const value = await socket.emitWithAck("test")`
will now properly resolve with an error and get discarded.
- handlers that don't like `socket.emit("test", (value) => { ... });`
will simply be discarded upon disconnection
Note: the structure of the 'acks' attribute has been left untouched, in
order to prevent any breaking change.
Related:
- #1546
- socketio/socket.io#4964
darrachequesne
added
bug
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
Our app operates in some areas with poor internet connectivity, and network interruptions are quite common. When our client's call
socket.emit, often the underlaying websocket will close before the message reaches the server. Since the socket was connected at the time of theemit, the messages aren't added to thethis.sendBufferqueue. So, effectively, they are lost. And the application code waits indefinitely for the ack callabck.Describe the solution you'd like
I would like a feature similar to the
timeout()feature, but for transport close events.When a "close" event is received on a transport, any remaining entries in
this.acksshould be invoked with an Error, and removed. Since the transport is closed, there is no hope of an actual ack coming from the server. This should also prevent some memory leak issues, becausethis.acksis holding references to functions for acks that will never come.When the application receives the Error callback, it can decide if it wants to retry the emit, or to inform the user of the failure.
Describe alternatives you've considered
This functionality could probably be built into the application layer above socketio. But, it would fit more cleanly into the
this.acksdatastructure. The application also doesn't have the ability to clean out thethis.acksdata structure, which could lead to memory leaks.Additional context
I'm happy to put together a PR for this.
The text was updated successfully, but these errors were encountered: