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

feature: Handle error 429 when connecting websocket #213

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

Jonnern
Copy link
Contributor

@Jonnern Jonnern commented Sep 18, 2024

A RateLimitGuard can be used to limit (re)connection of sockets.
This works well until the server returns 429 and the client continues to use the configured client rate limit.
If the client is too eager to reconnect, the client may be stuck in a 429 rate limit loop.

This feature enables setting SocketExchangeOptions.ConnectDelayAfterRateLimited which will set a RetryAfterGuardon the connection when the server returns 429.

  • Parse error message thrown from ClientWebSocket.Connect when connecting the socket fails
    • Check if "429" is in the error message
  • Add RateLimitItemType to RetryAfterGuard
  • Add SocketExchangeOptions.ConnectDelayAfterRateLimited
    • Works as a one-time rate limit to avoid subsequent connection attempts that pass the regular connection rate limiting

Note: I could not create a unit test for this behavior.


if (e is WebSocketException we)
{
// ClientWebSocket.HttpStatusCode is only available in .NET6+ https://learn.microsoft.com/en-us/dotnet/api/system.net.websockets.clientwebsocket.httpstatuscode?view=net-8.0
Copy link
Owner

Choose a reason for hiding this comment

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

Would it be good to use a compiler directive here checking for .NET 6 or higher which uses the new HttpStatusCode property and if lower version fallback to checking for 429?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that idea

@JKorf JKorf merged commit 5d3de52 into JKorf:master Sep 24, 2024
1 check 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