fix(observability): classify in-band SSE error frames instead of dropping them (#3588)

Co-authored-by: Cyrus Gray <cyrus@tinyhumans.ai>

src/core/observability.rs

@@ -302,6 +302,21 @@ pub fn expected_error_kind(message: &str) -> Option<ExpectedErrorKind> {
     if crate::openhuman::inference::provider::managed_error_skips_sentry(message) {
         return Some(ExpectedErrorKind::BackendErrorCodeOwned);
     }
+    // A managed-backend client-guard-leak code (`PAYLOAD_TOO_LARGE` /
+    // `CONTEXT_LENGTH_EXCEEDED`) must PAGE — the client enforces these limits
+    // before sending, so a backend rejection is our guard leaking. Force
+    // capture (return `None`) here, BEFORE the substring matchers below: a real
+    // managed `CONTEXT_LENGTH_EXCEEDED` body carries "context length
+    // exceeded"-style text that `is_context_window_exceeded_message` (further
+    // down) would otherwise re-demote into the suppressed
+    // `ContextWindowExceeded` bucket (CodeRabbit). Gated on the managed envelope
+    // so a BYO provider's own context-overflow — genuine user-state, not our
+    // guard — still flows to that matcher and stays demoted.
+    if crate::openhuman::inference::provider::is_managed_backend_envelope(message)
+        && crate::openhuman::inference::provider::is_backend_client_guard_leak(message)
+    {
+        return None;
+    }
     // Check the Codex-CLI import envelope first: it is highly specific
     // (literal `codex cli auth` / `.codex/auth.json`) and carries no overlap
     // with the generic matchers below, so ordering is for clarity, not
@@ -5330,8 +5345,6 @@ mod tests {
             ("402", "USER_INSUFFICIENT_CREDITS"),
             ("503", "UPSTREAM_UNAVAILABLE"),
             ("404", "MODEL_UNAVAILABLE"),
-            ("413", "PAYLOAD_TOO_LARGE"),
-            ("400", "CONTEXT_LENGTH_EXCEEDED"),
             ("400", "BAD_REQUEST"),
             ("500", "INTERNAL_ERROR"),
         ] {
@@ -5342,6 +5355,37 @@ mod tests {
                 "errorCode={code} must be backend-owned (no FE Sentry)"
             );
         }
+
+        // Client-guard-leak codes page (None = capture), even with realistic
+        // explanatory text that a later substring matcher would otherwise
+        // re-demote into a suppressed bucket.
+        let payload = "OpenHuman API error (413 Payload Too Large): \
+             {\"error\":{\"errorCode\":\"PAYLOAD_TOO_LARGE\",\"message\":\"request entity too large\"}}";
+        assert_eq!(
+            expected_error_kind(payload),
+            None,
+            "PAYLOAD_TOO_LARGE is a client guard leak and must page"
+        );
+
+        // This body's message matches `is_context_window_exceeded_message`, so
+        // without the early guard-leak bypass it would re-demote to the
+        // suppressed `ContextWindowExceeded` bucket (CodeRabbit).
+        let context = "OpenHuman API error (400 Bad Request): \
+             {\"error\":{\"errorCode\":\"CONTEXT_LENGTH_EXCEEDED\",\"message\":\"This model's maximum context length is 128000 tokens, however you requested more\"}}";
+        assert_eq!(
+            expected_error_kind(context),
+            None,
+            "CONTEXT_LENGTH_EXCEEDED must page even with context-window wording"
+        );
+
+        // Proof the bypass is load-bearing, not a no-op: this body's text DOES
+        // match the context-window matcher, so without the early guard-leak
+        // bypass `expected_error_kind` would have re-demoted it to the
+        // suppressed `ContextWindowExceeded` bucket.
+        assert!(
+            crate::openhuman::inference::provider::is_context_window_exceeded_message(context),
+            "test body must actually trigger the matcher the bypass guards against"
+        );
     }
 
     #[test]
@@ -5390,7 +5434,6 @@ mod tests {
             "RATE_LIMITED",
             "UPSTREAM_UNAVAILABLE",
             "MODEL_UNAVAILABLE",
-            "PAYLOAD_TOO_LARGE",
             "INTERNAL_ERROR",
             "BAD_REQUEST",
         ] {
@@ -5400,6 +5443,22 @@ mod tests {
                 "errorCode={code} event must be dropped by before_send"
             );
         }
+
+        // Client-guard-leak codes survive before_send and page (the client
+        // should have caught the limit before sending) — including a realistic
+        // CONTEXT_LENGTH_EXCEEDED body whose wording matches the context-window
+        // substring matcher (the filter keys on the errorCode, not the text).
+        let payload = "OpenHuman API error (413 Payload Too Large): \
+             {\"error\":{\"errorCode\":\"PAYLOAD_TOO_LARGE\",\"message\":\"request entity too large\"}}";
+        let context = "OpenHuman API error (400 Bad Request): \
+             {\"error\":{\"errorCode\":\"CONTEXT_LENGTH_EXCEEDED\",\"message\":\"This model's maximum context length is 128000 tokens\"}}";
+        for body in [payload, context] {
+            let event = event_with_message(body);
+            assert!(
+                !is_backend_error_code_event(&event),
+                "client guard leak must survive before_send: {body}"
+            );
+        }
     }
 
     #[test]

src/openhuman/inference/provider/compatible_stream_native.rs

@@ -171,6 +171,24 @@ impl OpenAiCompatibleProvider {
             while let Some(sep_idx) = buffer.find("\n\n") {
                 let event = buffer[..sep_idx].to_string();
                 buffer.drain(..sep_idx + 2);
+
+                // In-band SSE error frame. The response status flushed 200
+                // before the upstream call, so the managed backend delivers
+                // post-flush errors — including instant 4xx/429/413 whose typed
+                // `errorCode` (#870) is now stamped on the frame, see
+                // backend `routes/inference.ts` — as `event: error\ndata: {…}`.
+                // Surface it as a streaming-envelope error string so the
+                // `errorCode` classifier + Sentry golden rule run downstream,
+                // instead of skipping it as an unparseable chunk (which would
+                // aggregate to empty and surface as "empty response").
+                if let Some(message) = sse_error_frame_bail_message(
+                    self.name.as_str(),
+                    Some(native_request.model.as_str()),
+                    &event,
+                ) {
+                    anyhow::bail!(message);
+                }
+
                 for line in event.lines() {
                     let line = line.trim();
                     if line.is_empty() || line.starts_with(':') {
@@ -462,3 +480,124 @@ impl OpenAiCompatibleProvider {
         Self::parse_native_response(api_resp, &self.name)
     }
 }
+
+/// Extract the `data:` payload of an SSE `event: error` frame, or `None` when
+/// the event block is not an error frame.
+///
+/// The managed backend delivers post-flush stream errors as a two-line SSE
+/// block — `event: error` followed by `data: {"error":{…,"errorCode":…}}`
+/// (backend `routes/inference.ts`). The normal chunk loop can't parse that
+/// `data:` line as a `StreamChunkResponse`, so without this detection the frame
+/// is silently skipped and the turn aggregates to an empty response. We key on
+/// the `event: error` field rather than sniffing the data so a normal chunk
+/// that merely mentions "error" is never misclassified.
+fn sse_error_frame_payload(event: &str) -> Option<String> {
+    let mut is_error_frame = false;
+    let mut data: Option<String> = None;
+    for line in event.lines() {
+        let line = line.trim();
+        if let Some(event_type) = line.strip_prefix("event:") {
+            if event_type.trim() == "error" {
+                is_error_frame = true;
+            }
+        } else if let Some(payload) = line.strip_prefix("data:") {
+            data = Some(payload.trim().to_string());
+        }
+    }
+    is_error_frame.then(|| data.unwrap_or_default())
+}
+
+/// Build the bail message for an in-band SSE `event: error` frame, or `None`
+/// when the event block is a normal chunk / metadata event (the caller then
+/// continues parsing it as a chunk).
+///
+/// Mirrors the non-2xx HTTP path: produces a `<provider> streaming API error:
+/// <sanitized body>` envelope so the downstream `errorCode` classifier and the
+/// Sentry golden rule run on it. When the frame carries a backend-owned
+/// `errorCode`, it is logged (not re-reported) here, matching the HTTP path's
+/// [`is_backend_error_code_owned`] branch; a malformed `BAD_REQUEST` is excluded
+/// by that helper and still pages downstream. There is no HTTP status for an
+/// in-band frame, so the log records the `200` that was actually sent.
+fn sse_error_frame_bail_message(
+    provider: &str,
+    model: Option<&str>,
+    event: &str,
+) -> Option<String> {
+    let payload = sse_error_frame_payload(event)?;
+    let sanitized = super::super::sanitize_api_error(&payload);
+    let message = format!("{provider} streaming API error: {sanitized}");
+    if super::super::is_backend_error_code_owned(provider, &payload) {
+        super::super::log_backend_error_code_owned(
+            "streaming_chat",
+            provider,
+            model,
+            reqwest::StatusCode::OK,
+            &payload,
+        );
+    }
+    Some(message)
+}
+
+#[cfg(test)]
+mod sse_error_frame_tests {
+    use super::{sse_error_frame_bail_message, sse_error_frame_payload};
+    use crate::openhuman::inference::provider::openhuman_backend::PROVIDER_LABEL;
+
+    #[test]
+    fn extracts_payload_from_error_frame() {
+        let event = "event: error\ndata: {\"error\":{\"message\":\"boom\",\"type\":\"stream_error\",\"errorCode\":\"BAD_REQUEST\"}}";
+        let payload = sse_error_frame_payload(event).expect("error frame detected");
+        assert!(payload.contains("\"errorCode\":\"BAD_REQUEST\""));
+        assert!(payload.contains("\"type\":\"stream_error\""));
+    }
+
+    #[test]
+    fn ignores_normal_data_chunk() {
+        let event =
+            "data: {\"choices\":[{\"delta\":{\"content\":\"an error occurred in the story\"}}]}";
+        assert!(sse_error_frame_payload(event).is_none());
+    }
+
+    #[test]
+    fn ignores_metadata_event() {
+        let event = "event: openhuman-metadata\ndata: {\"openhuman\":{}}";
+        assert!(sse_error_frame_payload(event).is_none());
+    }
+
+    #[test]
+    fn bail_message_wraps_error_frame_in_streaming_envelope() {
+        // A managed-backend error frame yields a streaming-envelope string the
+        // downstream classifier recognises, preserving the `errorCode`.
+        let event = "event: error\ndata: {\"error\":{\"message\":\"slow down\",\"type\":\"stream_error\",\"errorCode\":\"RATE_LIMITED\"}}";
+        let message = sse_error_frame_bail_message(PROVIDER_LABEL, Some("reasoning-v1"), event)
+            .expect("bail message built");
+        assert!(message.contains("streaming API error"));
+        assert!(message.contains("\"errorCode\":\"RATE_LIMITED\""));
+    }
+
+    #[test]
+    fn bail_message_handles_frame_without_error_code() {
+        // An untyped mid-stream drop (no `errorCode`) still produces an
+        // envelope; it is simply not backend-owned, so downstream Sentry gating
+        // applies as before.
+        let event =
+            "event: error\ndata: {\"error\":{\"message\":\"socket hang up\",\"type\":\"stream_error\"}}";
+        let message =
+            sse_error_frame_bail_message(PROVIDER_LABEL, None, event).expect("bail message built");
+        assert!(message.contains("socket hang up"));
+    }
+
+    #[test]
+    fn bail_message_is_none_for_normal_chunk() {
+        let event = "data: {\"choices\":[{\"delta\":{\"content\":\"hi\"}}]}";
+        assert!(sse_error_frame_bail_message(PROVIDER_LABEL, None, event).is_none());
+    }
+
+    #[test]
+    fn error_frame_without_data_yields_empty_payload() {
+        // Defensive: an `event: error` with no `data:` line still resolves to a
+        // (empty) payload so the caller bails rather than skipping it.
+        let event = "event: error";
+        assert_eq!(sse_error_frame_payload(event).as_deref(), Some(""));
+    }
+}

src/openhuman/inference/provider/error_code.rs

@@ -175,16 +175,44 @@ pub fn is_backend_malformed_bad_request(err: &str) -> bool {
     ) && body_flags_malformed(err)
 }
 
+/// Whether the `errorCode` names a limit the **client enforces before sending**,
+/// so a backend rejection means our pre-send guard leaked — a client-side bug
+/// worth paging, not expected user-state.
+///
+/// - `PAYLOAD_TOO_LARGE`: the client gates attachment size up front
+///   (`app/src/lib/attachments.ts` — per-image / per-file byte caps + a
+///   `too_large` reject), so an over-limit request reaching the backend means
+///   the aggregate slipped past those gates.
+/// - `CONTEXT_LENGTH_EXCEEDED`: the client manages context before send (the
+///   context pipeline's `context_window`, `src/openhuman/context/pipeline.rs`),
+///   so a backend rejection means that fitting / trimming failed.
+///
+/// The backend does not ops-alert either (they are 4xx, not 500), so if the FE
+/// also suppressed them the guard leak would be invisible to everyone. Display
+/// classification is unchanged — the user still sees the actionable copy.
+pub fn is_backend_client_guard_leak(err: &str) -> bool {
+    matches!(
+        extract_backend_error_code(err),
+        Some(BackendErrorCode::PayloadTooLarge | BackendErrorCode::ContextLengthExceeded)
+    )
+}
+
 /// Sentry-ownership decision (F2 golden rule): a response carrying any backend
 /// `errorCode` must **not** page the FE — the backend owns it (it already
-/// paged) or it is expected user-state — *except* a backend-flagged malformed
-/// `BAD_REQUEST`, which the client caused and so still pages.
+/// paged) or it is expected user-state — *except* errors the **client** caused
+/// and so still page:
+/// - a backend-flagged malformed `BAD_REQUEST` (unparseable client payload), and
+/// - a client-guard-leak code (`PAYLOAD_TOO_LARGE` / `CONTEXT_LENGTH_EXCEEDED`)
+///   the client should have caught before sending — see
+///   [`is_backend_client_guard_leak`].
 ///
 /// Shared by the provider HTTP layer (`api_error`), the higher-layer re-report
 /// classifier (`observability::expected_error_kind`), and the Sentry
 /// `before_send` defense-in-depth filter so the three layers can't drift.
 pub fn backend_error_code_skips_sentry(err: &str) -> bool {
-    extract_backend_error_code_token(err).is_some() && !is_backend_malformed_bad_request(err)
+    extract_backend_error_code_token(err).is_some()
+        && !is_backend_malformed_bad_request(err)
+        && !is_backend_client_guard_leak(err)
 }
 
 #[cfg(test)]
@@ -249,6 +277,29 @@ mod tests {
         assert!(backend_error_code_skips_sentry(user_param));
     }
 
+    #[test]
+    fn client_guard_leak_codes_page_but_other_state_codes_do_not() {
+        // PAYLOAD_TOO_LARGE / CONTEXT_LENGTH_EXCEEDED are limits the client
+        // enforces before sending, so a backend rejection is a guard leak that
+        // must page the FE — unlike genuinely backend-owned / user-state codes.
+        let payload = r#"OpenHuman API error (413 Payload Too Large): {"error":{"errorCode":"PAYLOAD_TOO_LARGE","message":"too big"}}"#;
+        assert!(is_backend_client_guard_leak(payload));
+        assert!(!backend_error_code_skips_sentry(payload));
+        assert!(!managed_error_skips_sentry(payload));
+
+        let context = r#"OpenHuman API error (400 Bad Request): {"error":{"errorCode":"CONTEXT_LENGTH_EXCEEDED","message":"start a new chat"}}"#;
+        assert!(is_backend_client_guard_leak(context));
+        assert!(!backend_error_code_skips_sentry(context));
+
+        // Contrast: these remain backend-owned / expected user-state -> suppress.
+        let rate = r#"OpenHuman API error (429): {"error":{"errorCode":"RATE_LIMITED"}}"#;
+        let credits =
+            r#"OpenHuman API error (402): {"error":{"errorCode":"USER_INSUFFICIENT_CREDITS"}}"#;
+        assert!(!is_backend_client_guard_leak(rate));
+        assert!(backend_error_code_skips_sentry(rate));
+        assert!(backend_error_code_skips_sentry(credits));
+    }
+
     #[test]
     fn malformed_flag_without_bad_request_is_ignored() {
         // A stray `malformed` flag on a non-BAD_REQUEST code must not turn a

src/openhuman/inference/provider/mod.rs

@@ -37,8 +37,9 @@ pub use config_rejection::{
 };
 pub use error_code::{
     backend_error_code_skips_sentry, body_flags_malformed, extract_backend_error_code,
-    extract_backend_error_code_token, is_backend_malformed_bad_request,
-    is_managed_backend_envelope, managed_error_skips_sentry, BackendErrorCode,
+    extract_backend_error_code_token, is_backend_client_guard_leak,
+    is_backend_malformed_bad_request, is_managed_backend_envelope, managed_error_skips_sentry,
+    BackendErrorCode,
 };
 pub use factory::{create_chat_provider, provider_for_role, BYOK_INCOMPLETE_SENTINEL};
 pub use ops::*;

src/openhuman/inference/provider/ops_tests.rs

@@ -338,15 +338,13 @@ fn skips_sentry_report_for_transient_upstream_statuses() {
 fn backend_error_code_owned_gates_managed_errors_except_malformed_bad_request() {
     use crate::openhuman::inference::provider::openhuman_backend::PROVIDER_LABEL;
 
-    // F2/F4: any managed-backend body carrying an errorCode is backend-owned
-    // and must NOT page the provider HTTP layer.
+    // F2/F4: backend-owned / expected-user-state errorCodes must NOT page the
+    // provider HTTP layer.
     for code in [
         "RATE_LIMITED",
         "USER_INSUFFICIENT_CREDITS",
         "UPSTREAM_UNAVAILABLE",
         "MODEL_UNAVAILABLE",
-        "PAYLOAD_TOO_LARGE",
-        "CONTEXT_LENGTH_EXCEEDED",
         "INTERNAL_ERROR",
     ] {
         let body = format!("{{\"error\":{{\"errorCode\":\"{code}\",\"message\":\"x\"}}}}");
@@ -363,7 +361,18 @@ fn backend_error_code_owned_gates_managed_errors_except_malformed_bad_request()
         "{\"error\":{\"errorCode\":\"BAD_REQUEST\",\"message\":\"bad param\"}}"
     ));
 
-    // F8: a backend-flagged malformed BAD_REQUEST is the one case the FE still
+    // Client-guard-leak codes page: the client enforces these limits before
+    // sending (attachment size gates; context-window management), so a backend
+    // rejection means our guard leaked — the gate must NOT claim them.
+    for code in ["PAYLOAD_TOO_LARGE", "CONTEXT_LENGTH_EXCEEDED"] {
+        let body = format!("{{\"error\":{{\"errorCode\":\"{code}\",\"message\":\"x\"}}}}");
+        assert!(
+            !is_backend_error_code_owned(PROVIDER_LABEL, &body),
+            "errorCode={code} is a client guard leak and must page (not owned)"
+        );
+    }
+
+    // F8: a backend-flagged malformed BAD_REQUEST is also a case the FE still
     // pages — the gate must NOT claim it.
     assert!(!is_backend_error_code_owned(
         PROVIDER_LABEL,