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

Refactor the gateway to properly accept close frames #575

Merged
merged 11 commits into from
Nov 3, 2024

Conversation

kozabrada123
Copy link
Member

also introduces a test for receiving the close codes.

Differentiates the wasm and tungstenite gateways a bit more.

The same thing should be done for the voice gateway, I'll likely add it to this pr if the mod to the normal gateway is okay

Copy link
Member

@bitfl0wer bitfl0wer left a comment

Choose a reason for hiding this comment

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

// Note: The tungstenite backend handles close codes as messages, while the ws_stream_wasm one handles them differently.

Since I have seen this comment in multiple files now: Would it perhaps be possible and feasible to convert wasm-stream and tungstenite messages into the same type? that way, we would not have to differentiate between the two in multiple files

@kozabrada123
Copy link
Member Author

That's how we used to do it; probably no, unless we have another task that just listens for ws_stream_wasm errors

@kozabrada123
Copy link
Member Author

How the messages are is represented currently: non error messages from tungstenite and ws_stream_wasm can both be RawGatewayMessage. GatewayCommunication are all tungstenite messages

@bitfl0wer
Copy link
Member

That's how we used to do it; probably no, unless we have another task that just listens for ws_stream_wasm errors

I meaaaan, that could work!

@kozabrada123
Copy link
Member Author

I think this is nicer
also, multiple files? they are twice in gateway.rs, and then in voice/gateway.rs (which is basically a copy of the former)

Copy link
Member

@bitfl0wer bitfl0wer left a comment

Choose a reason for hiding this comment

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

I like these changes! My only issue with them would be what I already mentioned, but other than that it looks great!

@kozabrada123 kozabrada123 marked this pull request as ready for review November 1, 2024 15:51
@kozabrada123 kozabrada123 merged commit 5adc0bc into dev Nov 3, 2024
9 checks passed
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.

2 participants