mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(inference): classify list_models 404 at source + actionable hint (TAURI-RUST-8X3) (#3769)
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
This commit is contained in:
oxoxDev
co-authored by
Steven Enamakel
Steven Enamakel
parent
8fca02a22f
commit
4347e579c1
@@ -289,9 +289,15 @@ describe('<TaskSourcesPanel />', () => {
|
||||
it('clicking Refresh calls load() (line 586)', async () => {
|
||||
renderPanel();
|
||||
await screen.findByTestId('task-source-s-1');
|
||||
// The Refresh button early-returns inside load() while a load is in flight
|
||||
// (and is disabled in the UI for the same reason). Wait for the mount load
|
||||
// to finish — i.e. the button to become enabled — before clicking, mirroring
|
||||
// what a real user can do and removing the mount-vs-click race.
|
||||
const refreshButton = screen.getByRole('button', { name: /Refresh/i });
|
||||
await waitFor(() => expect(refreshButton).toBeEnabled());
|
||||
// The first listMock call happens on mount; clicking Refresh triggers another
|
||||
const beforeCount = listMock.mock.calls.length;
|
||||
fireEvent.click(screen.getByRole('button', { name: /Refresh/i }));
|
||||
fireEvent.click(refreshButton);
|
||||
await waitFor(() => {
|
||||
expect(listMock.mock.calls.length).toBeGreaterThan(beforeCount);
|
||||
});
|
||||
|
||||
@@ -1265,7 +1265,28 @@ fn is_provider_user_state_message(lower: &str) -> bool {
|
||||
// No `inference/provider/ops.rs::list_models` other than this site emits
|
||||
// the `provider returned NNN` prefix (verified via grep), so the prefix
|
||||
// alone is a sufficient anchor.
|
||||
if lower.starts_with("provider returned 404") {
|
||||
//
|
||||
// TAURI-RUST-8X3: anchor to the position where `provider returned 404` is
|
||||
// the formatted *error prefix* — never to any occurrence in the response
|
||||
// body. The primary fix classifies the *raw* error at the source
|
||||
// (`inference/ops.rs::inference_list_models`) before any log prefix is
|
||||
// applied, so the raw shape always starts with `provider returned 404:`.
|
||||
// The one historically-observed prefixed re-report path is the
|
||||
// `inference/ops.rs` `error!("[inference::ops] list_models:error: {err}")`
|
||||
// log line — handled below as an explicit prefixed shape.
|
||||
//
|
||||
// A bare `contains` would mis-fire: a genuine 400/500 list-models failure
|
||||
// formats as `provider returned 500: <body>`, and if `<body>` merely
|
||||
// relays an upstream phrase like `upstream provider returned 404 ...`, the
|
||||
// loose substring would demote that real 4xx/5xx defect out of Sentry —
|
||||
// exactly the failures the discrimination guard
|
||||
// (`does_not_classify_non_404_list_models_failures_as_user_state`) says
|
||||
// must still escalate. So we require the anchor to be the prefix, not buried
|
||||
// text. (Mirrors the parenthesised `(401` anchoring in
|
||||
// `is_session_expired_message`.)
|
||||
if lower.starts_with("provider returned 404")
|
||||
|| lower.contains("list_models:error: provider returned 404")
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -4390,6 +4411,55 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn couples_list_models_404_source_shape_to_classifier() {
|
||||
// TAURI-RUST-8X3 coupling guard. Ties the TYPED SOURCE error shape
|
||||
// emitted by `inference/provider/ops/models.rs` (the
|
||||
// `provider returned 404: <body>` format) to the classifier, so a
|
||||
// wording / prefix drift fails CI instead of silently leaking events.
|
||||
//
|
||||
// The wild Sentry message carried the `inference/ops.rs`
|
||||
// `error!("[inference::ops] list_models:error: {err}")` PREFIX. The
|
||||
// primary fix classifies the raw `err` at the source before that
|
||||
// prefix is applied, but the `contains` widening above must ALSO
|
||||
// catch the prefixed variant for any future prefixed re-report path.
|
||||
// Assert BOTH the raw source shape and the prefixed log-line shape.
|
||||
|
||||
// (a) Raw source shape — exactly what `models.rs` returns for a Go
|
||||
// default-handler 404 (`404 page not found`).
|
||||
let raw_source = "provider returned 404: 404 page not found";
|
||||
assert_eq!(
|
||||
expected_error_kind(raw_source),
|
||||
Some(ExpectedErrorKind::ProviderUserState),
|
||||
"raw list_models 404 source shape must classify as ProviderUserState"
|
||||
);
|
||||
|
||||
// (b) Raw source shape WITH the actionable hint appended by
|
||||
// `models.rs` for the 404 case — the prefix anchor must survive
|
||||
// the suffix.
|
||||
let raw_with_hint = "provider returned 404: 404 page not found — the configured base URL does not expose a `/models` endpoint; check the provider's base URL (it usually ends in `/v1`)";
|
||||
assert_eq!(
|
||||
expected_error_kind(raw_with_hint),
|
||||
Some(ExpectedErrorKind::ProviderUserState),
|
||||
"list_models 404 + actionable hint must still classify as ProviderUserState"
|
||||
);
|
||||
|
||||
// (c) Prefixed log-line shape — the exact pattern from
|
||||
// `inference/ops.rs::inference_list_models` `error!`. The explicit
|
||||
// `list_models:error: provider returned 404` anchor must catch this
|
||||
// even though it does not start with `provider returned 404`. The
|
||||
// anchor is the formatted prefix, not a bare `404` substring, so it
|
||||
// does NOT mis-fire on a 500 whose body merely relays an upstream
|
||||
// 404 (see `does_not_classify_non_404_list_models_failures_as_user_state`).
|
||||
let prefixed =
|
||||
"[inference::ops] list_models:error: provider returned 404: 404 page not found";
|
||||
assert_eq!(
|
||||
expected_error_kind(prefixed),
|
||||
Some(ExpectedErrorKind::ProviderUserState),
|
||||
"prefixed list_models 404 log line must still classify as ProviderUserState (anchored prefix)"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_classify_non_404_list_models_failures_as_user_state() {
|
||||
// Discrimination guard: only the 404 prefix demotes. Sibling 4xx /
|
||||
@@ -4411,6 +4481,16 @@ mod tests {
|
||||
r#"provider returned 503: upstream temporarily unavailable"#,
|
||||
// 500 — a real upstream bug; must reach Sentry.
|
||||
r#"provider returned 500: {"error":"internal_server_error"}"#,
|
||||
// TAURI-RUST-8X3 false-negative guard: a genuine 4xx/5xx whose
|
||||
// *body* merely relays an upstream 404 phrase. The actual status
|
||||
// is 500/400, so this is a real failure that MUST reach Sentry —
|
||||
// the classifier anchors on the `provider returned 404:` prefix,
|
||||
// not on any `404` occurrence in the body, so these must NOT demote.
|
||||
r#"provider returned 500: {"error":"upstream provider returned 404 page not found"}"#,
|
||||
r#"provider returned 400: gateway error — upstream provider returned 404"#,
|
||||
// Prefixed log-line variant of the same trap: the genuine status is
|
||||
// 500, the buried `... 404` is body text.
|
||||
"[inference::ops] list_models:error: provider returned 500: upstream provider returned 404 not found",
|
||||
] {
|
||||
assert_ne!(
|
||||
expected_error_kind(raw),
|
||||
|
||||
@@ -295,6 +295,32 @@ pub async fn inference_list_models(provider_id: &str) -> Result<RpcOutcome<Value
|
||||
error = %err,
|
||||
"{LOG_PREFIX} list_models:unknown-provider (user-config)"
|
||||
);
|
||||
} else if let Some(kind) = crate::core::observability::expected_error_kind(err) {
|
||||
// Classify at the TYPED SOURCE — run the raw provider error
|
||||
// through the central classifier BEFORE the
|
||||
// `[inference::ops] list_models:error: …` prefix is applied,
|
||||
// then `warn!` so the Sentry tracing layer records at most a
|
||||
// breadcrumb instead of a hard error event.
|
||||
//
|
||||
// TAURI-RUST-8X3: a user pointed a custom OpenAI-compatible
|
||||
// provider at a base URL with no `/models` route, so the
|
||||
// probe returns `provider returned 404: 404 page not found`.
|
||||
// That is a preventable user-state condition (wrong base URL;
|
||||
// the dropdown already surfaces an actionable hint inline) —
|
||||
// not a code bug. The 404 arm of
|
||||
// `is_provider_user_state_message` matched the *raw* error
|
||||
// string fine, but the previous `error!` path captured the
|
||||
// PREFIXED log line, so the demotion never reached Sentry's
|
||||
// classifier. Classifying the raw `err` here removes that
|
||||
// dependency on the log-string shape entirely; any
|
||||
// `ExpectedErrorKind` the central classifier recognizes is
|
||||
// demoted at the source.
|
||||
warn!(
|
||||
provider_id,
|
||||
error = %err,
|
||||
expected_kind = ?kind,
|
||||
"{LOG_PREFIX} list_models:expected (user-config): {err}"
|
||||
);
|
||||
} else {
|
||||
// Real error — embed `{err}` in the format string so
|
||||
// Sentry's event title carries the actionable cause
|
||||
|
||||
@@ -132,6 +132,21 @@ pub async fn list_configured_models_from_config(
|
||||
let body = response.text().await.unwrap_or_default();
|
||||
let sanitized = sanitize_api_error(&body);
|
||||
let truncated = crate::openhuman::util::truncate_with_ellipsis(&sanitized, 300);
|
||||
// TAURI-RUST-8X3: a 404 from the `<base>/models` probe means the
|
||||
// configured base URL does not host an OpenAI-compatible `/models`
|
||||
// listing (wrong base, a model-only proxy, or a missing `/v1`
|
||||
// suffix). Append an actionable hint so the model-dropdown probe
|
||||
// surfaces *recovery guidance* inline instead of the bare Go-style
|
||||
// `404 page not found`. The `provider returned 404` prefix is kept
|
||||
// verbatim so the `is_provider_user_state_message` classifier anchor
|
||||
// (which demotes this preventable user-state case out of Sentry)
|
||||
// still matches — see `src/core/observability.rs`.
|
||||
if status.as_u16() == 404 {
|
||||
return Err(format!(
|
||||
"provider returned 404: {} — the configured base URL does not expose a `/models` endpoint; check the provider's base URL (it usually ends in `/v1`)",
|
||||
truncated
|
||||
));
|
||||
}
|
||||
return Err(format!(
|
||||
"provider returned {}: {}",
|
||||
status.as_u16(),
|
||||
|
||||
Reference in New Issue
Block a user