Skip to content

Commit 4b5ede2

Browse files
oxoxDevclaude
andcommitted
refactor(observability): share TRANSIENT_PROVIDER_HTTP_STATUSES const
Per CodeRabbit review on tinyhumansai#1529: the transient status set (408/429/502/ 503/504) was duplicated between providers/ops.rs (typed StatusCode match) and core/observability.rs (string match in before_send). Extract a single pub const TRANSIENT_PROVIDER_HTTP_STATUSES: &[u16] so the two suppression layers stay in lockstep — update here and both call sites pick it up. No behavior change: every status that was filtered before is still filtered; every status that was reported before is still reported. The before_send filter now parses the status tag as u16 (with the parse-failure path joining the existing "missing tag" branch — keep, not drop). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7c2fa76 commit 4b5ede2

2 files changed

Lines changed: 22 additions & 13 deletions

File tree

src/core/observability.rs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,21 @@ use std::fmt::Display;
2020
/// anything you'd want to facet on (`error_kind`, `tool_name`, `method`).
2121
pub type Tag<'a> = (&'a str, &'a str);
2222

23+
/// HTTP status codes that the reliable-provider layer already handles via
24+
/// retry + fallback, so per-attempt Sentry reports add noise without signal:
25+
///
26+
/// - **408** Request Timeout
27+
/// - **429** Too Many Requests
28+
/// - **502** Bad Gateway
29+
/// - **503** Service Unavailable
30+
/// - **504** Gateway Timeout
31+
///
32+
/// Single source of truth for both the call-site classifier
33+
/// (`openhuman::providers::ops::should_report_provider_http_failure`) and the
34+
/// `before_send` filter (`is_transient_provider_http_failure`). Update here
35+
/// and both sites pick it up — keeps the two layers from drifting.
36+
pub const TRANSIENT_PROVIDER_HTTP_STATUSES: &[u16] = &[408, 429, 502, 503, 504];
37+
2338
/// Capture an error to Sentry with structured tags.
2439
///
2540
/// `domain` and `operation` are required and become tags `domain:<…>` and
@@ -77,16 +92,17 @@ pub fn report_error<E: Display + ?Sized>(
7792
///
7893
/// Match criteria:
7994
/// - tag `failure == "non_2xx"` (the marker set by `ops::api_error`)
80-
/// - tag `status` matches a known transient status (429, 408, 502, 503, 504)
95+
/// - tag `status` matches a known transient status (see
96+
/// [`TRANSIENT_PROVIDER_HTTP_STATUSES`])
8197
pub fn is_transient_provider_http_failure(event: &sentry::protocol::Event<'_>) -> bool {
8298
let tags = &event.tags;
8399
if tags.get("failure").map(String::as_str) != Some("non_2xx") {
84100
return false;
85101
}
86-
matches!(
87-
tags.get("status").map(String::as_str),
88-
Some("429") | Some("408") | Some("502") | Some("503") | Some("504")
89-
)
102+
let Some(status_u16) = tags.get("status").and_then(|s| s.parse::<u16>().ok()) else {
103+
return false;
104+
};
105+
TRANSIENT_PROVIDER_HTTP_STATUSES.contains(&status_u16)
90106
}
91107

92108
#[cfg(test)]

src/openhuman/providers/ops.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -154,14 +154,7 @@ const OPENHUMAN_BACKEND_PROVIDER_LABEL: &str = "OpenHuman";
154154
/// propagate the error so retry and fallback logic runs unchanged; this
155155
/// only gates the per-attempt Sentry report.
156156
pub fn should_report_provider_http_failure(status: reqwest::StatusCode) -> bool {
157-
!matches!(
158-
status,
159-
reqwest::StatusCode::TOO_MANY_REQUESTS
160-
| reqwest::StatusCode::REQUEST_TIMEOUT
161-
| reqwest::StatusCode::BAD_GATEWAY
162-
| reqwest::StatusCode::SERVICE_UNAVAILABLE
163-
| reqwest::StatusCode::GATEWAY_TIMEOUT
164-
)
157+
!crate::core::observability::TRANSIENT_PROVIDER_HTTP_STATUSES.contains(&status.as_u16())
165158
}
166159

167160
/// Build a sanitized provider error from a failed HTTP response.

0 commit comments

Comments
 (0)