feat(libsy-llm-client): harden upstream request handling

crates/libsy-llm-client/src/backend.rs

@@ -3,7 +3,7 @@
 
 //! Per-provider backend configuration: wire format, upstream URL, and auth.
 
-use std::{collections::BTreeMap, fmt};
+use std::{collections::BTreeMap, fmt, time::Duration};
 
 use reqwest::RequestBuilder;
 use serde_json::Value;
@@ -35,6 +35,19 @@ const ANTHROPIC_OVERFLOW_PHRASES: &[&str] = &[
     "context length",
 ];
 
+// Phrases marking a request the model cannot serve at all, whatever its size:
+// multimodal content sent to a text-only deployment, or a server started without
+// its multimodal projector. The wording comes from the serving stack rather than
+// a provider error envelope, and no provider assigns it a structured `error.code`,
+// so one phrase list covers every backend variant.
+const CAPABILITY_REJECT_PHRASES: &[&str] = &[
+    "mmproj",
+    "image input",
+    "does not support image",
+    "does not support multimodal",
+    "no multimodal support",
+];
+
 /// Shared HTTP configuration for one upstream backend.
 #[derive(Clone)]
 pub struct HttpBackendConfig {
@@ -48,6 +61,8 @@ pub struct HttpBackendConfig {
     pub extra_body: BTreeMap<String, Value>,
     /// Additional attempts after the initial upstream request.
     pub max_retries: u32,
+    /// Per-attempt request timeout in seconds. `None` leaves the request unbounded.
+    pub timeout_secs: Option<f64>,
 }
 
 impl fmt::Debug for HttpBackendConfig {
@@ -58,6 +73,7 @@ impl fmt::Debug for HttpBackendConfig {
             .field("extra_headers", &self.extra_headers)
             .field("extra_body_keys", &self.extra_body.keys())
             .field("max_retries", &self.max_retries)
+            .field("timeout_secs", &self.timeout_secs)
             .finish()
     }
 }
@@ -145,6 +161,23 @@ impl Backend {
         self.config().max_retries
     }
 
+    /// Per-attempt request timeout, when a usable one is configured.
+    ///
+    /// Applies to each attempt rather than the call as a whole, so a call that
+    /// exhausts its retry budget can take up to `(max_retries + 1)` times this.
+    ///
+    /// Validated here rather than trusted from the field: [`HttpBackendConfig`] is
+    /// public and constructible directly, so the server's config check is not the only
+    /// way a value can arrive. `Duration::from_secs_f64` panics on negative, NaN, and
+    /// infinite input, and a malformed timeout must not be able to abort a request — an
+    /// unusable value is therefore treated as no timeout, matching an omitted field.
+    pub fn timeout(&self) -> Option<Duration> {
+        self.config()
+            .timeout_secs
+            .filter(|seconds| seconds.is_finite() && *seconds > 0.0)
+            .map(Duration::from_secs_f64)
+    }
+
     /// Whether this backend speaks the Anthropic Messages wire format — the only
     /// one with a `count_tokens` endpoint.
     pub fn is_anthropic(&self) -> bool {
@@ -176,6 +209,16 @@ impl Backend {
             Backend::Anthropic(_) => is_overflow_body(body, |_| false, ANTHROPIC_OVERFLOW_PHRASES),
         }
     }
+
+    /// Whether an upstream 400 `body` says the model cannot serve this request at
+    /// all — as opposed to it merely being too large.
+    ///
+    /// Provider-independent: the rejection is emitted by the serving stack, so the
+    /// same phrase list applies to every backend variant and there is no structured
+    /// check to short-circuit on.
+    pub(crate) fn is_capability_reject(&self, body: &str) -> bool {
+        is_overflow_body(body, |_| false, CAPABILITY_REJECT_PHRASES)
+    }
 }
 
 // Accept either a root `/v1` URL or an already-specific OpenAI endpoint URL.
@@ -209,6 +252,7 @@ mod tests {
             extra_headers: BTreeMap::new(),
             extra_body: BTreeMap::new(),
             max_retries: 0,
+            timeout_secs: None,
         }
     }
 
@@ -275,6 +319,28 @@ mod tests {
         );
     }
 
+    #[test]
+    fn an_unusable_timeout_is_treated_as_no_timeout() {
+        // `HttpBackendConfig` is public, so the server's validation is not the only way
+        // a value arrives. None of these may panic a request.
+        for seconds in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY, -1.0, 0.0] {
+            let mut inner = config("x");
+            inner.timeout_secs = Some(seconds);
+            assert_eq!(
+                Backend::OpenAiChat(inner).timeout(),
+                None,
+                "{seconds} must not configure a timeout"
+            );
+        }
+
+        let mut inner = config("x");
+        inner.timeout_secs = Some(1.5);
+        assert_eq!(
+            Backend::OpenAiChat(inner).timeout(),
+            Some(Duration::from_millis(1_500))
+        );
+    }
+
     #[test]
     fn only_anthropic_backend_is_anthropic() {
         assert!(Backend::Anthropic(config("x")).is_anthropic());
@@ -327,4 +393,36 @@ mod tests {
         );
         assert!(!backend.is_context_overflow(r#"{"error":{"message":"overloaded"}}"#));
     }
+
+    #[test]
+    fn detects_capability_reject_across_backends() {
+        // Provider-independent: the serving stack emits it, so every variant matches.
+        for backend in [
+            Backend::OpenAiChat(config("x")),
+            Backend::OpenAiResponses(config("x")),
+            Backend::Anthropic(config("x")),
+        ] {
+            assert!(backend.is_capability_reject(
+                r#"{"error":{"message":"image input is not supported by this model"}}"#
+            ));
+            assert!(backend.is_capability_reject(
+                r#"{"error":{"message":"server was started without an mmproj file"}}"#
+            ));
+            // Plain-text bodies from proxies still classify.
+            assert!(backend.is_capability_reject("this model does not support image content"));
+        }
+    }
+
+    #[test]
+    fn capability_reject_and_overflow_do_not_overlap() {
+        let backend = Backend::OpenAiChat(config("x"));
+        // An overflow is not a capability reject: a smaller request can still succeed.
+        let overflow = r#"{"error":{"code":"context_length_exceeded","message":"too long"}}"#;
+        assert!(backend.is_context_overflow(overflow));
+        assert!(!backend.is_capability_reject(overflow));
+        // And unrelated failures are neither.
+        let rate_limit = r#"{"error":{"message":"rate limit exceeded"}}"#;
+        assert!(!backend.is_context_overflow(rate_limit));
+        assert!(!backend.is_capability_reject(rate_limit));
+    }
 }