refactor: Add read_timeout for HTTP requests, make timeout optional

bindings/matrix-sdk-ffi/src/client_builder.rs

@@ -133,6 +133,11 @@ pub struct ClientBuilder {
     threads_enabled: bool,
 }
 
+/// The timeout applies to each read operation, and resets after a successful
+/// read. This is more appropriate for detecting stalled connections when the
+/// size isn’t known beforehand.
+const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(60);
+
 #[matrix_sdk_ffi_macros::export]
 impl ClientBuilder {
     #[uniffi::constructor]
@@ -539,6 +544,7 @@ impl ClientBuilder {
             if let Some(timeout) = config.timeout {
                 updated_config = updated_config.timeout(Duration::from_millis(timeout));
             }
+            updated_config = updated_config.read_timeout(DEFAULT_READ_TIMEOUT);
             if let Some(max_concurrent_requests) = config.max_concurrent_requests {
                 if max_concurrent_requests > 0 {
                     updated_config = updated_config.max_concurrent_requests(NonZeroUsize::new(

crates/matrix-sdk/CHANGELOG.md

@@ -31,6 +31,13 @@ All notable changes to this project will be documented in this file.
 - [**breaking**] The MSRV has been bumped to Rust 1.88.
   ([#5431](https://github.com/matrix-org/matrix-rust-sdk/pull/5431))
 
+### Bugfix
+
+- All HTTP requests now have a default `read_timeout` of 60s, which means they'll disconnect if the connection stalls.
+ `RequestConfig::timeout` is now optional and can be disabled on a per-request basis. This will be done for
+ the requests used to download media, so they don't get cancelled after the default 30s timeout for no good reason. 
+ ([#5437](https://github.com/matrix-org/matrix-rust-sdk/pull/5437))
+
 ## [0.13.0] - 2025-07-10
 
 ### Security Fixes

crates/matrix-sdk/src/client/mod.rs

@@ -20,6 +20,7 @@ use std::{
     future::{ready, Future},
     pin::Pin,
     sync::{Arc, Mutex as StdMutex, RwLock as StdRwLock, Weak},
+    time::Duration,
 };
 
 use caches::ClientCaches;
@@ -2312,7 +2313,8 @@ impl Client {
         });
         let mut request_config = self.request_config();
         if let Some(timeout) = sync_settings.timeout {
-            request_config.timeout += timeout;
+            let base_timeout = request_config.timeout.unwrap_or(Duration::from_secs(30));
+            request_config.timeout = Some(base_timeout + timeout);
         }
 
         let response = self.send(request).with_request_config(request_config).await?;

crates/matrix-sdk/src/config/request.rs

@@ -43,7 +43,8 @@ use crate::http_client::DEFAULT_REQUEST_TIMEOUT;
 /// ```
 #[derive(Copy, Clone)]
 pub struct RequestConfig {
-    pub(crate) timeout: Duration,
+    pub(crate) timeout: Option<Duration>,
+    pub(crate) read_timeout: Option<Duration>,
     pub(crate) retry_limit: Option<usize>,
     pub(crate) max_retry_time: Option<Duration>,
     pub(crate) max_concurrent_requests: Option<NonZeroUsize>,
@@ -56,6 +57,7 @@ impl Debug for RequestConfig {
     fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         let Self {
             timeout,
+            read_timeout,
             retry_limit,
             max_retry_time: retry_timeout,
             force_auth,
@@ -65,6 +67,7 @@ impl Debug for RequestConfig {
 
         let mut res = fmt.debug_struct("RequestConfig");
         res.field("timeout", timeout)
+            .maybe_field("read_timeout", read_timeout)
             .maybe_field("retry_limit", retry_limit)
             .maybe_field("max_retry_time", retry_timeout)
             .maybe_field("max_concurrent_requests", max_concurrent_requests)
@@ -81,7 +84,8 @@ impl Debug for RequestConfig {
 impl Default for RequestConfig {
     fn default() -> Self {
         Self {
-            timeout: DEFAULT_REQUEST_TIMEOUT,
+            timeout: Some(DEFAULT_REQUEST_TIMEOUT),
+            read_timeout: None,
             retry_limit: Default::default(),
             max_retry_time: Default::default(),
             max_concurrent_requests: Default::default(),
@@ -132,8 +136,22 @@ impl RequestConfig {
 
     /// Set the timeout duration for all HTTP requests.
     #[must_use]
-    pub fn timeout(mut self, timeout: Duration) -> Self {
-        self.timeout = timeout;
+    pub fn timeout(mut self, timeout: impl Into<Option<Duration>>) -> Self {
+        self.timeout = timeout.into();
+        self
+    }
+
+    /// Set the read timeout duration for all HTTP requests.
+    ///
+    /// The timeout applies to each read operation, and resets after a
+    /// successful read. This is more appropriate for detecting stalled
+    /// connections when the size isn’t known beforehand.
+    ///
+    /// **IMPORTANT**: note this value can only be applied when the HTTP client
+    /// is instantiated, it won't have any effect on a per-request basis.
+    #[must_use]
+    pub fn read_timeout(mut self, timeout: impl Into<Option<Duration>>) -> Self {
+        self.read_timeout = timeout.into();
         self
     }
 
@@ -180,12 +198,14 @@ mod tests {
             .force_auth()
             .max_retry_time(Duration::from_secs(32))
             .retry_limit(4)
-            .timeout(Duration::from_secs(600));
+            .timeout(Duration::from_secs(600))
+            .read_timeout(Duration::from_secs(10));
 
         assert!(cfg.force_auth);
         assert_eq!(cfg.retry_limit, Some(4));
         assert_eq!(cfg.max_retry_time, Some(Duration::from_secs(32)));
-        assert_eq!(cfg.timeout, Duration::from_secs(600));
+        assert_eq!(cfg.timeout, Some(Duration::from_secs(600)));
+        assert_eq!(cfg.read_timeout, Some(Duration::from_secs(10)));
     }
 
     #[test]

crates/matrix-sdk/src/http_client/native.rs

@@ -149,7 +149,8 @@ pub(crate) struct HttpSettings {
     pub(crate) disable_ssl_verification: bool,
     pub(crate) proxy: Option<String>,
     pub(crate) user_agent: Option<String>,
-    pub(crate) timeout: Duration,
+    pub(crate) timeout: Option<Duration>,
+    pub(crate) read_timeout: Option<Duration>,
     pub(crate) additional_root_certificates: Vec<Certificate>,
     pub(crate) disable_built_in_root_certificates: bool,
 }
@@ -161,7 +162,8 @@ impl Default for HttpSettings {
             disable_ssl_verification: false,
             proxy: None,
             user_agent: None,
-            timeout: DEFAULT_REQUEST_TIMEOUT,
+            timeout: Some(DEFAULT_REQUEST_TIMEOUT),
+            read_timeout: None,
             additional_root_certificates: Default::default(),
             disable_built_in_root_certificates: false,
         }
@@ -175,11 +177,18 @@ impl HttpSettings {
         let user_agent = self.user_agent.clone().unwrap_or_else(|| "matrix-rust-sdk".to_owned());
         let mut http_client = reqwest::Client::builder()
             .user_agent(user_agent)
-            .timeout(self.timeout)
             // As recommended by BCP 195.
             // See: https://datatracker.ietf.org/doc/bcp195/
             .min_tls_version(tls::Version::TLS_1_2);
 
+        if let Some(timeout) = self.timeout {
+            http_client = http_client.timeout(timeout);
+        }
+
+        if let Some(read_timeout) = self.read_timeout {
+            http_client = http_client.read_timeout(read_timeout);
+        }
+
         if self.disable_ssl_verification {
             warn!("SSL verification disabled in the HTTP client!");
             http_client = http_client.danger_accept_invalid_certs(true)
@@ -213,7 +222,7 @@ impl HttpSettings {
 pub(super) async fn send_request(
     client: &reqwest::Client,
     request: &http::Request<Bytes>,
-    timeout: Duration,
+    timeout: Option<Duration>,
     send_progress: SharedObservable<TransmissionProgress>,
 ) -> Result<http::Response<Bytes>, HttpError> {
     use std::convert::Infallible;
@@ -251,7 +260,7 @@ pub(super) async fn send_request(
             reqwest::Request::try_from(request)?
         };
 
-        *request.timeout_mut() = Some(timeout);
+        *request.timeout_mut() = timeout;
         request
     };
 

crates/matrix-sdk/src/media.rs

@@ -434,18 +434,24 @@ impl Media {
             }
         }
 
+        let request_config = self
+            .client
+            .request_config()
+            // Downloading a file should have no timeout as we don't know the network connectivity
+            // available for the user or the file size
+            .timeout(None);
+
         // Use the authenticated endpoints when the server supports Matrix 1.11 or the
         // authenticated media stable feature.
         let (use_auth, request_config) =
             if self.client.server_versions().await?.contains(&MatrixVersion::V1_11) {
-                (true, None)
+                (true, request_config)
             } else if self.client.unstable_features().await?.contains(&FeatureFlag::Msc3916Stable) {
                 // We need to force the use of the stable endpoint with the Matrix version
                 // because Ruma does not handle stable features.
-                let request_config = self.client.request_config();
-                (true, Some(request_config.force_matrix_version(MatrixVersion::V1_11)))
+                (true, request_config.force_matrix_version(MatrixVersion::V1_11))
             } else {
-                (false, None)
+                (false, request_config)
             };
 
         let content: Vec<u8> = match &request.source {