-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fix(WebSocketShard): Zombie connection fix #8989
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
Before the uptime was kind of bad thanks to the The logs/tests proved reduced reconnects and better uptime at least on my 60+ Shards bot. Also thanks @shoxcy for providing me with shawarma during the testings |
Blocking for 2 days at @legendhimself's request so they can test the PR for more time. Will unblock afterwards. |
For now, I have excellent results. |
I hear that you were waiting for more information, here are my session_start_limit remaining logins of the GET {
"url": "wss://gateway.discord.gg",
"shards": 368,
"session_start_limit": {
"total": 2000,
"remaining": 1995,
"reset_after": 79901986,
"max_concurrency": 16
}
} That shows that we just used 5 relogins today without any zombie connection, so for me the problem is solved, and this PR should be merged as soon as possible for others devs 🎉 |
Day 2, Uptime ^, No restarts so far. Usually, some of the shards by this time used to silently restart due to the unref on the closetimeout but now, as you saw in the previous login, everything is going fine with this fix.This pr is safe to be merged unless someone else wants to test the fixes first. I think we can get this pr unblocked now and get it merged. |
- Fix backport discordjs#7626 missing changes - Reverted the pull request discordjs#8956 - Removed unref of wsCloseTimeout - We are resuming the connection for zombie instead of starting a new Co-authored-by: DraftMan <[email protected]>
de9060f
to
a7fd2d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good
Fixes #8486, fixes #8984 and supersedes #8981
We are resuming the connection for zombie instead of starting a newTo not mess it up we didn't push this. Will create a separate pr for this.Please describe the changes this PR makes and why it should be merged:
Status and versioning classification: