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

Fix Heartbeat gateway event handling #3105

Open
wants to merge 1 commit into
base: current
Choose a base branch
from

Conversation

DPlayer234
Copy link

Currently, serenity deserializes the Heartbeat gateway event (opcode 1) incorrectly (Discord->Bot heartbeats):

[WARN  serenity::gateway::ws] Err deserializing text: Json(Error("missing field `s`", line: 0, column: 0)); text: {"t":null,"s":null,"op":1,"d":null}

It attempts to deserialize a sequence number for it, however it is null in the payload and that matches Discord's documentation on gateway events: https://discord.com/developers/docs/events/gateway-events#payload-structure
(Heartbeat is not opcode 0, so s and t are null.)

What serenity intends to happen after is forcing a heartbeat immediately. This is correct according to the documentation: https://discord.com/developers/docs/events/gateway#sending-heartbeats
However, the deserialization error leads to this code being unreachable in practice.

This patch leaves the Heartbeat event's sequence field in, as it's technically part of the crate's public API, but sets it to 0 and ignores it during handling.

Some cursory testing indicates that this leads to the correct behavior:

[INFO  serenity::gateway::shard] handle_event; event=Ok(Heartbeat(0))
[INFO  serenity::gateway::shard] [ShardInfo { id: ShardId(0), total: 1 }] Received shard heartbeat
[INFO  serenity::gateway::ws] send_heartbeat; shard_info=ShardInfo { id: ShardId(0), total: 1 } seq=Some(4)
[TRACE serenity::gateway::ws] [ShardInfo { id: ShardId(0), total: 1 }] Sending heartbeat d: Some(4)
[TRACE serenity::gateway::bridge::shard_runner] [ShardRunner ShardInfo { id: ShardId(0), total: 1 }] loop iteration reached the end.
[TRACE serenity::gateway::bridge::shard_runner] [ShardRunner ShardInfo { id: ShardId(0), total: 1 }] loop iteration started.
[INFO  serenity::gateway::shard] handle_event; event=Ok(HeartbeatAck)
[TRACE serenity::gateway::shard] [ShardInfo { id: ShardId(0), total: 1 }] Received heartbeat ack

The current running theory is that this was never noticed because this gateway event very rarely happens.
In my testing, Discord only sends this event when a heartbeat is overdue. So to test this, I disabled periodic heartbeats, except once after the Hello event.

@github-actions github-actions bot added model Related to the `model` module. gateway Related to the `gateway` module. labels Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gateway Related to the `gateway` module. model Related to the `model` module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant