matrix-org
`Client::login` should support retries in case of 429 / Too Many Requests
I'm using matrix-rust-sdk both in CI and for non-trivial bots that may require many clients.
Right now, Client::login uses a RequestConfig::short_retry(), ignoring any RequestConfig we may have passed in ClientBuilder::request_config, which makes the method fail if the server responds with a 429. This means that I need to manually retry my logins. It's definitely possible, but it's both ugly code and easy to get wrong.
I'd like the ability to have a Client::login that I can just trust to retry.
One possibility would be to introduce a method
impl ClientBuilder {
/// Specify a `RequestConfig` to use during login, e.g. for retrying.
///
/// By default, `ClientBuilder` assumes that its users wish to be informed quickly if they client
/// cannot connect to a homeserver and will use the specialized `RequestConfig::short_retry()`
/// to fail early.
///
/// Use this method to e.g. enable further retries in case of 429 "Too Many Requests".
pub fn login_request_config(self, RequestConfig) -> Self;
}
Caveat: In terms of API design, it's a bit odd.
I feel like this would end up exploding the API surface.
As I previously mentioned, we could have the short_retry() request config be override-able or, if this really needs to be login specific, add the method to the login builder instead.
I assumed what I wrote was what you meant by override-able. Obviously, I misunderstood :)
What's the login builder?
Anyway, if you tell me what you want the API to look like, I'll be happy to implement it.
What I meant in chat that the short_retry() could be overridden, what you suggest here instead overrides the request config only for the login() call.
The short_retry() config is used in more places than just for the login() method.
The login builder is found here: https://github.com/matrix-org/matrix-rust-sdk/blob/main/crates/matrix-sdk/src/client/login_builder.rs#L58
It has various methods to configure how the login should be done, adding a request config there should be much cleaner than to add it to the ClientBuilder.