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(ws): Songbird would fail if it could not deserialize ws payload. #170

Merged
merged 1 commit into from
Apr 9, 2023

Conversation

Erk-
Copy link
Member

@Erk- Erk- commented Apr 8, 2023

Discord started sending a new event on the voice websocket with opcode 18:

{"op":18,"d":{"user_id":"...","flags":0}}

This caused songbird to crash and disconnect.

This patch makes it so that failing to deserialize a event is not a fatal error.

@Erk- Erk- requested a review from FelixMcFelix April 8, 2023 10:24
@Erk- Erk- force-pushed the do-not-fail-if-new-opcode branch from 1d9d135 to 0618112 Compare April 8, 2023 10:30
@Skarlett
Copy link

Skarlett commented Apr 8, 2023

updated my project with pr-branch, appears to work great.

Skarlett/coggie-bot@1986fea

Copy link
Member

@FelixMcFelix FelixMcFelix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is effectively moving the check from the main ws runner loop to the WS stream itself, which I think is a better solution. I do need to clean up the main WS loop to remove the check there but pushing this through for now.

@FelixMcFelix FelixMcFelix merged commit 752cae7 into serenity-rs:current Apr 9, 2023
Erk- added a commit to Erk-/songbird that referenced this pull request Apr 10, 2023
FelixMcFelix pushed a commit to FelixMcFelix/songbird that referenced this pull request Apr 10, 2023
…erenity-rs#170)

Receiving a new opcode while connecting to a voice channel was causing the connection to fail, while this was handled correctly elsewhere. Unfortunately, Discord added such a payload during every connection.

This PR moves the logging and conversion to no-op (and log) to catch both locations.
FelixMcFelix pushed a commit to FelixMcFelix/songbird that referenced this pull request Nov 20, 2023
…erenity-rs#170)

Receiving a new opcode while connecting to a voice channel was causing the connection to fail, while this was handled correctly elsewhere. Unfortunately, Discord added such a payload during every connection.

This PR moves the logging and conversion to no-op (and log) to catch both locations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants