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

Gateway cleanup #3099

Merged
merged 5 commits into from
Jan 27, 2025
Merged

Gateway cleanup #3099

merged 5 commits into from
Jan 27, 2025

Conversation

mkrasnitski
Copy link
Collaborator

This PR refactors how the gateway connection being closed gets handled, and also reworks how resuming is performed. If a resume fails, or if the session id is invalid/doesn't exist, the shard will fall back to restart + reidentify after a 1 second delay. This behavior was only present in some circumstances before.

Also, cleaned up the loop in ShardRunner::run by adding a ShardAction::Dispatch variant, since event dispatch was already mutually exclusive to hearbeating, identifying, and restarting. The overall effect is less interleaving of control flow.

Plus, removed the Shard::{reconnect, reset} functions as they were unused.

A notable change is that 4006 is no longer considered a valid close code as it is undocumented, and neither is 1000, which tungstenite assigns as Normal or "clean". We should stick to the table of close codes provided by Discord.

Also, a close code of 1000 will now warn as unknown.
Always check for a session id before resuming, and fall back to a
reidentify if the resume fails. In all cases, wait 1 second after
resuming fails in order to not spam log messages.
@github-actions github-actions bot added the gateway Related to the `gateway` module. label Jan 25, 2025
@arqunis arqunis added the enhancement An improvement to Serenity. label Jan 25, 2025
@arqunis arqunis added the breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users label Jan 27, 2025
@arqunis arqunis merged commit e71723e into serenity-rs:next Jan 27, 2025
24 checks passed
@mkrasnitski mkrasnitski deleted the gateway-cleanup branch January 28, 2025 01:41
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Feb 1, 2025
This commit refactors how the gateway connection being closed gets handled,
and also reworks how resuming is performed. If a resume fails, or if the
session id is invalid/doesn't exist, the shard will fall back to restart
+ reidentify after a 1 second delay. This behavior was only present in
some circumstances before.

Also, cleaned up the loop in `ShardRunner::run` by adding a
`ShardAction::Dispatch` variant, since event dispatch was already
mutually exclusive to hearbeating, identifying, and restarting. The
overall effect is less interleaving of control flow.

Plus, removed the `Shard::{reconnect, reset}` functions as they were
unused.

A notable change is that 4006 is no longer considered a valid close code
as it is undocumented, and neither is 1000, which tungstenite assigns as
`Normal` or "clean". We should stick to the [table of close
codes](https://discord.com/developers/docs/topics/opcodes-and-status-codes#gateway-gateway-close-event-codes)
provided by Discord.
jamesbt365 added a commit to jamesbt365/serenity that referenced this pull request Feb 2, 2025
arqunis pushed a commit that referenced this pull request Feb 3, 2025
A regression introduced by #3099 was that successful resumes will break
out of the loop inside `ShardRunner::run`, but they shouldn't (or
rather, didn't before). Therefore, only break out of the loop if the
resume failed and we had to fallback to reidentifying.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users enhancement An improvement to Serenity. gateway Related to the `gateway` module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants