Clean up namespaces after client is rejected in middleware #4773
Conversation
| @@ -758,9 +758,6 @@ export class Socket< | |||
| } | |||
|
|
|||
| this._cleanup(); | |||
There was a problem hiding this comment.
_cleanup is called here and in two more places: https://github.com/socketio/socket.io/blob/main/lib/namespace.ts#L327-L333
I believe the this.nsp._remove(this); should be called in all these cases
lib/socket.ts
Outdated
| @@ -772,6 +769,9 @@ export class Socket< | |||
| */ | |||
| _cleanup() { | |||
| this.leaveAll(); | |||
| this.nsp._remove(this); | |||
| this.client._remove(this); | |||
| this.connected = false; | |||
There was a problem hiding this comment.
I don't think this.client._remove(this); and this.connected = false; are needed here, since the socket is not actually connected if it's rejected by the middleware. What do you think?
There was a problem hiding this comment.
I agree, I just thought it does no harm since client._remove has a guard check and no-ops if the socket is not actually connected, but yeah, let me move it out
|
Thanks a lot 👍 |
|
Thank you for merging. What's the release process of this repo? Do I need to do something for this to roll out? |
|
@carera we have a few other bug fixes in progress, I'll keep you updated. |
|
Update: released in version 4.7.2 🚀 |
The kind of change this PR does introduce
Change is fixing bug described here: #4772