Rust: move the redirect option onto the builder — 3.7.0 was breaking

.changeset/rust-redirect-on-builder.md

@@ -0,0 +1,21 @@
+---
+'@smooai/fetch': patch
+---
+
+Rust: move the redirect option off `RequestInit` and onto the builder
+
+3.7.0 added `RequestInit.follow_redirects`. Adding a public field to a struct
+consumers construct is a **breaking change** in Rust semver, shipped under a
+minor — building the SmooAI monorepo against 3.7.0 fails with
+`error[E0063]: missing field` in **129 exhaustive constructors** across ~40 crates.
+
+The option is now `FetchBuilder::with_follow_redirects`, matching the shape Go
+and .NET already use, and `RequestInit` is back to its 3.6.2 fields. Redirect
+policy is per-Client in reqwest anyway, so the builder was the right home from
+the start.
+
+`client::fetch` keeps its exact signature; a new `client::fetch_with_redirect_policy`
+takes the extra argument, so nothing that compiled against 3.6.2 needs changing.
+
+3.7.0 is yanked from crates.io. The other four languages were unaffected —
+Python added a defaulted dataclass field, Go and .NET added builder methods.

rust/fetch/src/builder.rs

@@ -55,6 +55,7 @@ pub struct FetchBuilder<T: DeserializeOwned + Clone + Send + 'static> {
     hooks: LifecycleHooks<T>,
     auth_token_provider: Option<AuthTokenProvider>,
     auth_scheme: String,
+    follow_redirects: Option<bool>,
 }
 
 impl<T: DeserializeOwned + Clone + Send + 'static> FetchBuilder<T> {
@@ -69,6 +70,7 @@ impl<T: DeserializeOwned + Clone + Send + 'static> FetchBuilder<T> {
             container_options: FetchContainerOptions::default(),
             default_init: None,
             hooks: LifecycleHooks::default(),
+            follow_redirects: None,
             auth_token_provider: None,
             auth_scheme: "Bearer".to_string(),
         }
@@ -248,6 +250,31 @@ impl<T: DeserializeOwned + Clone + Send + 'static> FetchBuilder<T> {
         self
     }
 
+    /// Controls whether redirects are followed. Defaults to following, matching
+    /// reqwest.
+    ///
+    /// Set `false` when a redirect must not be followed. Two cases where
+    /// following one is wrong rather than merely surprising:
+    ///
+    /// - A caller that resolved the target hostname and checked it against an
+    ///   SSRF allowlist has that guard defeated by a 302 to an internal
+    ///   address, because the check was performed on the original host.
+    /// - RFC 8461 forbids fetching an MTA-STS policy through a redirect.
+    ///
+    /// With `false` the 3xx is returned as an ordinary response rather than
+    /// raised as an error.
+    ///
+    /// This lives on the builder, not on [`RequestInit`], for two reasons:
+    /// reqwest's redirect policy is per-Client rather than per-request, and a
+    /// new public field on `RequestInit` would break every exhaustive
+    /// constructor downstream — 129 of them in the SmooAI monorepo alone. The
+    /// Go and .NET ports expose the same client-level shape.
+    #[must_use]
+    pub fn with_follow_redirects(mut self, follow: bool) -> Self {
+        self.follow_redirects = Some(follow);
+        self
+    }
+
     /// Build the configured fetch client.
     pub fn build(self) -> FetchClient<T> {
         let rate_limiter = self
@@ -272,6 +299,7 @@ impl<T: DeserializeOwned + Clone + Send + 'static> FetchBuilder<T> {
             hooks: Arc::new(self.hooks),
             auth_token_provider: self.auth_token_provider,
             auth_scheme: self.auth_scheme,
+            follow_redirects: self.follow_redirects,
         }
     }
 }
@@ -293,6 +321,7 @@ pub struct FetchClient<T: DeserializeOwned + Clone + Send + 'static> {
     hooks: Arc<LifecycleHooks<T>>,
     auth_token_provider: Option<AuthTokenProvider>,
     auth_scheme: String,
+    follow_redirects: Option<bool>,
 }
 
 impl<T: DeserializeOwned + Clone + Send + 'static> FetchClient<T> {
@@ -305,14 +334,15 @@ impl<T: DeserializeOwned + Clone + Send + 'static> FetchClient<T> {
         // Merge default init with per-request init
         let merged_init = self.apply_auth(self.merge_init(init)).await;
 
-        crate::client::fetch::<T>(
+        crate::client::fetch_with_redirect_policy::<T>(
             url,
             merged_init,
             Some(self.fetch_options.clone()),
             self.rate_limiter.as_ref(),
             self.rate_limit_retry.as_ref(),
             self.circuit_breaker.as_ref(),
             Some(self.hooks.as_ref()),
+            self.follow_redirects,
         )
         .await
     }
@@ -326,14 +356,15 @@ impl<T: DeserializeOwned + Clone + Send + 'static> FetchClient<T> {
     ) -> Result<FetchResponse<T>, FetchError> {
         let merged_init = self.apply_auth(self.merge_init(init)).await;
 
-        crate::client::fetch::<T>(
+        crate::client::fetch_with_redirect_policy::<T>(
             url,
             merged_init,
             Some(options),
             self.rate_limiter.as_ref(),
             self.rate_limit_retry.as_ref(),
             self.circuit_breaker.as_ref(),
             Some(self.hooks.as_ref()),
+            self.follow_redirects,
         )
         .await
     }
@@ -371,10 +402,6 @@ impl<T: DeserializeOwned + Clone + Send + 'static> FetchClient<T> {
                     method: init.method,
                     headers: merged_headers,
                     body: init.body.or_else(|| default.body.clone()),
-                    // Per-request wins, but only when it SAYS something —
-                    // `None` inherits, so a client default of `Some(false)`
-                    // survives a per-request `..Default::default()`.
-                    follow_redirects: init.follow_redirects.or(default.follow_redirects),
                 }
             }
             None => init,

rust/fetch/src/client.rs

@@ -173,12 +173,13 @@ async fn do_single_request<T: DeserializeOwned>(
     url: &str,
     init: &RequestInit,
     connect_timeout_ms: Option<u64>,
+    follow_redirects: Option<bool>,
 ) -> Result<FetchResponse<T>, FetchError> {
     let mut client_builder = reqwest::Client::builder();
     if let Some(ms) = connect_timeout_ms {
         client_builder = client_builder.connect_timeout(Duration::from_millis(ms));
     }
-    if init.follow_redirects == Some(false) {
+    if follow_redirects == Some(false) {
         // Per-client, not per-request, in reqwest — which is why a caller could
         // not previously express this without building its own client.
         client_builder = client_builder.redirect(reqwest::redirect::Policy::none());
@@ -255,7 +256,7 @@ async fn do_single_request<T: DeserializeOwned>(
     // without this leaves the option useless: reqwest returns the 302, and it
     // is then raised as an `HttpResponse` error because it is not 2xx.
     let is_unfollowed_redirect =
-        init.follow_redirects == Some(false) && (300..400).contains(&(status as u32));
+        follow_redirects == Some(false) && (300..400).contains(&(status as u32));
 
     if fetch_response.ok || is_unfollowed_redirect {
         Ok(fetch_response)
@@ -285,6 +286,39 @@ pub async fn fetch<T: DeserializeOwned + Clone + Send + 'static>(
     rate_limit_retry: Option<&RateLimitRetryOptions>,
     circuit_breaker: Option<&CircuitBreaker>,
     hooks: Option<&LifecycleHooks<T>>,
+) -> Result<FetchResponse<T>, FetchError> {
+    fetch_with_redirect_policy(
+        url,
+        init,
+        options,
+        rate_limiter,
+        rate_limit_retry,
+        circuit_breaker,
+        hooks,
+        None,
+    )
+    .await
+}
+
+/// [`fetch`], plus explicit control over redirect following.
+///
+/// A separate entry point rather than an eighth parameter on [`fetch`], because
+/// changing that signature would break every existing caller. `None` follows,
+/// which is the default everywhere else.
+///
+/// `Some(false)` returns a 3xx as an ordinary response instead of following it —
+/// see [`crate::builder::FetchBuilder::with_follow_redirects`] for why that
+/// matters (SSRF guards, RFC 8461).
+#[allow(clippy::too_many_arguments)]
+pub async fn fetch_with_redirect_policy<T: DeserializeOwned + Clone + Send + 'static>(
+    url: &str,
+    init: RequestInit,
+    options: Option<FetchOptions>,
+    rate_limiter: Option<&SlidingWindowRateLimiter>,
+    rate_limit_retry: Option<&RateLimitRetryOptions>,
+    circuit_breaker: Option<&CircuitBreaker>,
+    hooks: Option<&LifecycleHooks<T>>,
+    follow_redirects: Option<bool>,
 ) -> Result<FetchResponse<T>, FetchError> {
     let opts = options.unwrap_or_default();
 
@@ -348,7 +382,7 @@ pub async fn fetch<T: DeserializeOwned + Clone + Send + 'static>(
         async move {
             timeout::with_timeout(
                 timeout_ms,
-                do_single_request::<T>(&url, &init, connect_timeout_ms),
+                do_single_request::<T>(&url, &init, connect_timeout_ms, follow_redirects),
             )
             .await
         }
@@ -361,7 +395,7 @@ pub async fn fetch<T: DeserializeOwned + Clone + Send + 'static>(
         // No retry, just execute once with timeout
         timeout::with_timeout(
             timeout_ms,
-            do_single_request::<T>(&url, &init, connect_timeout_ms),
+            do_single_request::<T>(&url, &init, connect_timeout_ms, follow_redirects),
         )
         .await
     };

rust/fetch/src/types.rs

@@ -215,6 +215,12 @@ impl Method {
 }
 
 /// Request initialization options, analogous to the JS `RequestInit`.
+///
+/// Redirect behaviour is NOT here: it is a client-level concern in reqwest
+/// (the policy lives on the Client, not the request), and adding a public
+/// field to this struct would break every exhaustive constructor downstream.
+/// Use [`crate::builder::FetchBuilder::with_follow_redirects`], which is what
+/// the Go and .NET ports expose too.
 #[derive(Debug, Clone, Default)]
 pub struct RequestInit {
     /// HTTP method.
@@ -223,23 +229,4 @@ pub struct RequestInit {
     pub headers: HashMap<String, String>,
     /// Request body (serialized as a string).
     pub body: Option<String>,
-    /// Whether to follow HTTP redirects. `None` inherits the client default,
-    /// which is to follow.
-    ///
-    /// Set `Some(false)` when a redirect must not be followed automatically.
-    /// Two cases where following one is wrong rather than merely surprising:
-    ///
-    /// - The caller resolved the target hostname and checked it against an
-    ///   SSRF allowlist. A 302 to an internal address defeats that guard
-    ///   entirely, because the check was performed on the original host.
-    /// - RFC 8461 forbids fetching an MTA-STS policy through a redirect.
-    ///
-    /// With `Some(false)` the 3xx is returned as an ordinary response.
-    ///
-    /// `Option` rather than `bool` for two reasons: it keeps
-    /// `#[derive(Default)]` correct (a bare `bool` would default to FALSE and
-    /// silently flip behaviour for every `..Default::default()` caller), and it
-    /// lets [`crate::builder`]'s `merge_init` tell "unset" from "explicitly
-    /// true" so a client-level default is not clobbered by a per-request init.
-    pub follow_redirects: Option<bool>,
 }

rust/fetch/tests/builder_tests.rs

@@ -76,7 +76,6 @@ async fn test_builder_with_default_headers() {
             method: Method::GET,
             headers: default_headers,
             body: None,
-            follow_redirects: None,
         })
         .build();
 
@@ -219,7 +218,6 @@ async fn test_builder_merge_headers() {
             method: Method::GET,
             headers: default_headers,
             body: None,
-            follow_redirects: None,
         })
         .build();
 
@@ -231,7 +229,6 @@ async fn test_builder_merge_headers() {
         method: Method::GET,
         headers: request_headers,
         body: None,
-        follow_redirects: None,
     };
 
     let response = client.fetch(&url, init).await.unwrap();
@@ -265,7 +262,6 @@ async fn test_builder_post_with_body() {
         method: Method::POST,
         headers,
         body: Some(r#"{"name":"test"}"#.to_string()),
-        follow_redirects: None,
     };
 
     let response = client.fetch(&url, init).await.unwrap();

rust/fetch/tests/fetch_tests.rs

@@ -77,7 +77,6 @@ async fn test_basic_post_request_with_body() {
         method: Method::POST,
         headers,
         body: Some(r#"{"key":"value"}"#.to_string()),
-        follow_redirects: None,
     };
     let options = FetchOptions {
         connect_timeout_ms: None,
@@ -346,12 +345,13 @@ async fn test_default_method_is_get() {
 // `Option<bool>` so that `None` inherits and a client-level default is not
 // clobbered by a per-request `..Default::default()`.
 #[test]
-fn request_init_defaults_to_following_redirects() {
+fn request_init_has_no_redirect_field() {
+    // Deliberate: a public field here would break every exhaustive
+    // `RequestInit { .. }` downstream. Redirect policy is per-Client in reqwest
+    // anyway, so it belongs on the builder — as it does in Go and .NET.
     let init = RequestInit::default();
-    assert_eq!(
-        init.follow_redirects, None,
-        "unset must mean inherit, not 'do not follow'"
-    );
+    assert_eq!(init.method, Method::GET);
+    assert!(init.body.is_none());
 }
 
 #[tokio::test]
@@ -368,24 +368,13 @@ async fn a_redirect_is_not_followed_when_the_caller_opts_out() {
         .mount(&server)
         .await;
 
-    let init = RequestInit {
-        follow_redirects: Some(false),
-        ..Default::default()
-    };
-    let result = client::fetch::<serde_json::Value>(
-        &format!("{}/start", server.uri()),
-        init,
-        Some(FetchOptions {
-            connect_timeout_ms: None,
-            timeout: Some(TimeoutOptions { timeout_ms: 5000 }),
-            retry: None,
-        }),
-        None,
-        None,
-        None,
-        None,
-    )
-    .await;
+    let client = smooai_fetch::builder::FetchBuilder::<serde_json::Value>::new()
+        .with_follow_redirects(false)
+        .without_retry()
+        .build();
+    let result = client
+        .fetch(&format!("{}/start", server.uri()), RequestInit::default())
+        .await;
 
     let response = result.expect("a 3xx is a response, not a transport error");
     assert_eq!(
@@ -406,21 +395,13 @@ async fn a_redirect_is_followed_by_default() {
         .mount(&server)
         .await;
 
-    let response = client::fetch::<serde_json::Value>(
-        &format!("{}/start", server.uri()),
-        RequestInit::default(),
-        Some(FetchOptions {
-            connect_timeout_ms: None,
-            timeout: Some(TimeoutOptions { timeout_ms: 5000 }),
-            retry: None,
-        }),
-        None,
-        None,
-        None,
-        None,
-    )
-    .await
-    .expect("follows by default");
+    let client = smooai_fetch::builder::FetchBuilder::<serde_json::Value>::new()
+        .without_retry()
+        .build();
+    let response = client
+        .fetch(&format!("{}/start", server.uri()), RequestInit::default())
+        .await
+        .expect("follows by default");
 
     assert_eq!(response.status, 200, "default behaviour must be unchanged");
 }