mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(embeddings): prevent + handle custom endpoint with no embeddings API (Sentry TAURI-RUST-5JR) (#3625) (#3629)
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
This commit is contained in:
co-authored by
Steven Enamakel
parent
2944b83fc0
commit
27739ee59e
@@ -282,6 +282,18 @@ const EmbeddingsPanel = ({ embedded = false }: EmbeddingsPanelProps = {}) => {
|
||||
custom_endpoint: customEndpoint.trim(),
|
||||
confirm_wipe: false,
|
||||
});
|
||||
// The endpoint responded but exposes no embeddings API (e.g. a chat-only
|
||||
// base URL like DeepSeek). Keep the setup popup open and surface the
|
||||
// actionable message so the user can correct the endpoint — TAURI-RUST-5JR.
|
||||
if (result.error === 'EMBEDDINGS_ENDPOINT_NO_API') {
|
||||
setSetupError(
|
||||
typeof result.message === 'string'
|
||||
? result.message
|
||||
: 'This endpoint has no embeddings API. Choose an embeddings-capable provider or a different endpoint.'
|
||||
);
|
||||
setStatus({ kind: 'idle' });
|
||||
return;
|
||||
}
|
||||
if (result.error === 'EMBEDDINGS_DIMENSION_CHANGE_REQUIRES_WIPE') {
|
||||
setPendingWipe({
|
||||
provider: 'custom',
|
||||
|
||||
@@ -352,6 +352,38 @@ describe('EmbeddingsPanel', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('surfaces an actionable error and keeps the popup open when the custom endpoint has no embeddings API', async () => {
|
||||
// TAURI-RUST-5JR: the backend probes the endpoint and rejects a chat-only
|
||||
// URL (DeepSeek) with EMBEDDINGS_ENDPOINT_NO_API. The panel must show the
|
||||
// message and NOT close the setup popup, so the user can fix the endpoint.
|
||||
const settings = makeSettings({
|
||||
providers: [
|
||||
makeProvider('managed', { requires_api_key: false }),
|
||||
makeProvider('custom', { requires_api_key: false, requires_endpoint: true }),
|
||||
],
|
||||
});
|
||||
vi.mocked(loadEmbeddingsSettings).mockResolvedValue(settings);
|
||||
vi.mocked(updateEmbeddingsSettings).mockResolvedValue({
|
||||
error: 'EMBEDDINGS_ENDPOINT_NO_API',
|
||||
message: 'This endpoint has no embeddings API. Choose an embeddings-capable provider.',
|
||||
});
|
||||
|
||||
renderWithProviders(<EmbeddingsPanel />);
|
||||
await screen.findByText('Custom');
|
||||
|
||||
fireEvent.click(screen.getByRole('radio', { name: /custom/i }));
|
||||
await screen.findByPlaceholderText(/https:\/\/your-endpoint/i);
|
||||
fireEvent.change(screen.getByPlaceholderText(/https:\/\/your-endpoint/i), {
|
||||
target: { value: 'https://api.deepseek.com/v1' },
|
||||
});
|
||||
fireEvent.click(screen.getByRole('button', { name: /save.*switch/i }));
|
||||
|
||||
// Actionable message shown.
|
||||
await screen.findByText(/no embeddings API/i);
|
||||
// Popup stays open — endpoint input is still present so the user can fix it.
|
||||
expect(screen.getByPlaceholderText(/https:\/\/your-endpoint/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// ─── Confirm wipe dialog ──────────────────────────────────────────────────
|
||||
|
||||
it('shows confirm-wipe dialog when updateEmbeddingsSettings returns DIMENSION_CHANGE error', async () => {
|
||||
|
||||
@@ -417,6 +417,16 @@ pub fn expected_error_kind(message: &str) -> Option<ExpectedErrorKind> {
|
||||
if is_embedding_backend_auth_failure(&lower) {
|
||||
return Some(ExpectedErrorKind::SessionExpired);
|
||||
}
|
||||
// TAURI-RUST-5JR — a custom embeddings endpoint with no embeddings route
|
||||
// (the user pointed the Custom (OpenAI-compatible) provider at a chat-only
|
||||
// base URL, e.g. DeepSeek, which 404s every `/embeddings` POST).
|
||||
// Deterministic user-config state, re-emitted on every memory re-embed;
|
||||
// the embeddings settings UI surfaces an actionable "pick an
|
||||
// embeddings-capable provider" message. Demote to info. Scoped to 404/405
|
||||
// only so a real 500 from a valid embeddings endpoint stays in Sentry.
|
||||
if is_embedding_endpoint_absent(&lower) {
|
||||
return Some(ExpectedErrorKind::ProviderConfigRejection);
|
||||
}
|
||||
// Provider config-rejection (unknown model / abstract tier leaked to a
|
||||
// custom provider / model-specific temperature). Body-shape based and
|
||||
// intrinsically scoped to third-party providers — the OpenHuman
|
||||
@@ -557,6 +567,32 @@ fn is_embedding_backend_auth_failure(lower: &str) -> bool {
|
||||
&& lower.contains("invalid token")
|
||||
}
|
||||
|
||||
/// Detect a custom embeddings endpoint that exposes **no embeddings API** —
|
||||
/// the `OpenAiEmbedding` client POSTed `/embeddings` and the host answered
|
||||
/// `404 Not Found` (route absent) or `405 Method Not Allowed`. Canonical wire
|
||||
/// shape from `src/openhuman/embeddings/openai.rs`:
|
||||
///
|
||||
/// ```text
|
||||
/// Embedding API error (404 Not Found): <body>
|
||||
/// Embedding API error (405 Method Not Allowed): <body>
|
||||
/// ```
|
||||
///
|
||||
/// Deterministic user-config state: the user pointed the Custom
|
||||
/// (OpenAI-compatible) embeddings provider at a base URL whose host has no
|
||||
/// embeddings endpoint (e.g. a chat-only provider like DeepSeek). Every memory
|
||||
/// re-embed re-emits it (TAURI-RUST-5JR, ~2685 events / 9 users) and the
|
||||
/// settings UI surfaces an actionable remediation — Sentry has no fix to make.
|
||||
///
|
||||
/// Polarity (important): scoped to **404/405 only**. A `500` from a valid
|
||||
/// embeddings endpoint is a real server fault and must keep reaching Sentry; a
|
||||
/// `400` (e.g. oversized input) is prevented at source by the chunk cap
|
||||
/// (#3598) and likewise stays visible. Reused by
|
||||
/// `embeddings::rpc::update_settings` as the save-time hard-block signal so the
|
||||
/// two never drift.
|
||||
pub(crate) fn is_embedding_endpoint_absent(lower: &str) -> bool {
|
||||
lower.contains("embedding api error") && (lower.contains("(404") || lower.contains("(405"))
|
||||
}
|
||||
|
||||
/// Detect the memory-store chunk DB's circuit-breaker-open message that
|
||||
/// `memory_store::chunks::store::get_or_init_connection` emits via
|
||||
/// `anyhow::bail!` when the per-path breaker rejects new init attempts.
|
||||
@@ -3771,6 +3807,49 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classifies_embedding_endpoint_absent_as_config_rejection() {
|
||||
// TAURI-RUST-5JR — custom embeddings provider pointed at a chat-only
|
||||
// base URL (DeepSeek) that has no `/embeddings` route. Verbatim shape
|
||||
// produced by `src/openhuman/embeddings/openai.rs` (prefix preserved
|
||||
// even after the actionable-hint suffix is appended).
|
||||
assert_eq!(
|
||||
expected_error_kind(
|
||||
"Embedding API error (404 Not Found): <html>not found</html> \
|
||||
— this endpoint has no embeddings API; pick an embeddings-capable \
|
||||
provider in Settings → Memory"
|
||||
),
|
||||
Some(ExpectedErrorKind::ProviderConfigRejection)
|
||||
);
|
||||
// 405 Method Not Allowed — route exists for GET only / wrong verb.
|
||||
assert_eq!(
|
||||
expected_error_kind("Embedding API error (405 Method Not Allowed): {}"),
|
||||
Some(ExpectedErrorKind::ProviderConfigRejection)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_demote_real_embedding_server_faults() {
|
||||
// Polarity guard: a 500 from a VALID embeddings endpoint is a real
|
||||
// server fault and must keep reaching Sentry — not demoted.
|
||||
assert_eq!(
|
||||
expected_error_kind("Embedding API error (500 Internal Server Error): upstream boom"),
|
||||
None,
|
||||
"embedding 500 is a real fault and must stay in Sentry"
|
||||
);
|
||||
// A 400 (e.g. oversized input — TAURI-RUST-4SA) is prevented at source
|
||||
// by the chunk cap (#3598); a residual 400 must stay visible, NOT be
|
||||
// swallowed by the 404/405-scoped endpoint-absent arm.
|
||||
assert_eq!(
|
||||
expected_error_kind(
|
||||
"Embedding API error (400 Bad Request): {\"error\":{\"message\":\
|
||||
\"maximum input length is 8192 tokens.\"}}"
|
||||
),
|
||||
None,
|
||||
"embedding 400 must NOT be demoted by the endpoint-absent (404/405) arm"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_classify_unrelated_provider_failures_as_config_rejection() {
|
||||
// Inverted polarity / scope guard: a 5xx or a generic 4xx with no
|
||||
|
||||
@@ -207,7 +207,21 @@ impl EmbeddingProvider for OpenAiEmbedding {
|
||||
target: "openai::embed",
|
||||
"[openai] embed error: status={status}, body={text}"
|
||||
);
|
||||
let message = format!("Embedding API error ({status}): {text}");
|
||||
let mut message = format!("Embedding API error ({status}): {text}");
|
||||
// A 404/405 means the base URL responded but exposes no
|
||||
// embeddings route — the user pointed the Custom
|
||||
// (OpenAI-compatible) provider at a chat-only endpoint (e.g.
|
||||
// DeepSeek). Append an actionable remediation while PRESERVING
|
||||
// the `Embedding API error (404…)` prefix that
|
||||
// `observability::is_embedding_endpoint_absent` keys on, so the
|
||||
// event is still demoted from Sentry. Host-agnostic text (no
|
||||
// URL/credential echo). TAURI-RUST-5JR.
|
||||
if matches!(status.as_u16(), 404 | 405) {
|
||||
message.push_str(
|
||||
" — this endpoint has no embeddings API; pick an \
|
||||
embeddings-capable provider in Settings → Memory",
|
||||
);
|
||||
}
|
||||
// Use `report_error_or_expected` so transient upstream HTTP
|
||||
// failures (e.g. 429 Too Many Requests after retry cap) log a
|
||||
// warning breadcrumb instead of firing a Sentry error event.
|
||||
|
||||
@@ -263,6 +263,36 @@ async fn embed_server_error() {
|
||||
assert!(msg.contains("rate limited"), "body: {msg}");
|
||||
}
|
||||
|
||||
/// A 404 means the configured base URL has no embeddings route (the user
|
||||
/// pointed the Custom provider at a chat-only endpoint, e.g. DeepSeek —
|
||||
/// TAURI-RUST-5JR). The message must (a) carry an actionable remediation, and
|
||||
/// (b) PRESERVE the `Embedding API error (404…)` prefix the
|
||||
/// `observability::is_embedding_endpoint_absent` classifier keys on, so the
|
||||
/// flood is demoted from Sentry rather than firing on every re-embed.
|
||||
#[tokio::test]
|
||||
async fn embed_404_endpoint_absent_is_actionable_and_classifier_stable() {
|
||||
let app = Router::new().route(
|
||||
"/v1/embeddings",
|
||||
post(|| async { (StatusCode::NOT_FOUND, "Not Found") }),
|
||||
);
|
||||
let url = start_mock(app).await;
|
||||
let p = OpenAiEmbedding::new(&url, "k", "m", 1);
|
||||
|
||||
let err = p.embed(&["hi"]).await.unwrap_err();
|
||||
let msg = err.to_string();
|
||||
// Classifier contract: prefix preserved.
|
||||
assert!(
|
||||
msg.to_ascii_lowercase()
|
||||
.contains("embedding api error (404"),
|
||||
"must preserve the (404 classifier prefix: {msg}"
|
||||
);
|
||||
// Actionable remediation appended.
|
||||
assert!(
|
||||
msg.contains("no embeddings API") && msg.contains("Settings → Memory"),
|
||||
"must carry actionable remediation: {msg}"
|
||||
);
|
||||
}
|
||||
|
||||
/// 429 rate-limit responses must format their message in the canonical
|
||||
/// `"... API error (<status>): <body>"` shape so the shared
|
||||
/// `is_transient_upstream_http_message` classifier in `core::observability`
|
||||
|
||||
@@ -109,6 +109,70 @@ pub async fn update_settings(
|
||||
let dims_changed = new_dims != old_dims;
|
||||
let sig_changed = new_sig != old_sig;
|
||||
|
||||
// Prevention (TAURI-RUST-5JR): a Custom (OpenAI-compatible) endpoint whose
|
||||
// host has no `/embeddings` route (e.g. a chat-only provider like DeepSeek)
|
||||
// 404s every memory re-embed forever — 2685 Sentry events / 9 users before
|
||||
// this gate. Probe the endpoint ONCE here so a no-embeddings URL can never
|
||||
// be persisted (and we never wipe memory for a config that can't embed).
|
||||
// Only custom endpoints are probed: named catalog providers are
|
||||
// embedding-capable by construction, and probing `managed`/`cloud`
|
||||
// pre-login would false-fail. Resolve the provider string exactly as it
|
||||
// will be stored so the probe targets the real endpoint.
|
||||
let effective_provider = match &custom_endpoint {
|
||||
Some(ep) if new_provider == "custom" || new_provider.starts_with("custom:") => {
|
||||
format!("custom:{ep}")
|
||||
}
|
||||
_ => new_provider.clone(),
|
||||
};
|
||||
if effective_provider.starts_with("custom:") {
|
||||
match build_embedder(&config, &effective_provider, &new_model, new_dims) {
|
||||
Ok(embedder) => {
|
||||
// Time-box the probe so a black-hole host can't hang the RPC.
|
||||
let probe = tokio::time::timeout(
|
||||
std::time::Duration::from_secs(10),
|
||||
embedder.embed(&["connection test"]),
|
||||
)
|
||||
.await;
|
||||
if let Ok(Err(e)) = probe {
|
||||
let detail = e.to_string();
|
||||
// HARD-block only the deterministic endpoint-absent shape
|
||||
// (404/405). Transient failures (timeout/5xx/network) fall
|
||||
// through and save — never lock out a valid-but-down
|
||||
// endpoint; the embed-time classifier handles any residual.
|
||||
if crate::core::observability::is_embedding_endpoint_absent(
|
||||
&detail.to_ascii_lowercase(),
|
||||
) {
|
||||
tracing::warn!(
|
||||
provider = effective_provider.as_str(),
|
||||
"{LOG_PREFIX} update_settings rejected — endpoint has no embeddings API"
|
||||
);
|
||||
let payload = serde_json::json!({
|
||||
"error": "EMBEDDINGS_ENDPOINT_NO_API",
|
||||
"message": "This endpoint has no embeddings API. Choose an \
|
||||
embeddings-capable provider (Managed, Voyage, OpenAI, \
|
||||
Cohere, Ollama) or a different custom endpoint.",
|
||||
"detail": detail,
|
||||
});
|
||||
return Ok(RpcOutcome::new(
|
||||
payload,
|
||||
vec!["embeddings endpoint has no embeddings API — not saved".into()],
|
||||
));
|
||||
}
|
||||
tracing::warn!(
|
||||
provider = effective_provider.as_str(),
|
||||
error = detail.as_str(),
|
||||
"{LOG_PREFIX} update_settings probe inconclusive — saving anyway"
|
||||
);
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
// Construction failure (unknown slug / bad config) — surface it
|
||||
// rather than persisting a config that can never embed.
|
||||
return Err(format!("invalid embedding provider configuration: {e}"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Only require a wipe when dimensions actually change — switching
|
||||
// provider/model at the same dimensionality keeps vectors comparable.
|
||||
if dims_changed && !confirm_wipe {
|
||||
@@ -382,15 +446,31 @@ pub async fn test_connection(
|
||||
/// [`embed`] uses, exposed so other domains (e.g. `codegraph`) can obtain a
|
||||
/// provider for `signature()` + direct embedding without a JSON-RPC round-trip.
|
||||
pub fn provider_from_config(config: &Config) -> anyhow::Result<Box<dyn super::EmbeddingProvider>> {
|
||||
let provider_name = &config.memory.embedding_provider;
|
||||
let model = &config.memory.embedding_model;
|
||||
let dims = config.memory.embedding_dimensions;
|
||||
build_embedder(
|
||||
config,
|
||||
&config.memory.embedding_provider,
|
||||
&config.memory.embedding_model,
|
||||
config.memory.embedding_dimensions,
|
||||
)
|
||||
}
|
||||
|
||||
/// Construct an embedding provider for an explicit `(provider_name, model,
|
||||
/// dims)` triple, resolving the stored API key + inline `custom:<url>` endpoint
|
||||
/// the same way [`embed`] / [`test_connection`] do. Single construction seam so
|
||||
/// the save-time probe in [`update_settings`] and the live embed path can't
|
||||
/// drift on slug-normalization / credential-lookup rules.
|
||||
fn build_embedder(
|
||||
config: &Config,
|
||||
provider_name: &str,
|
||||
model: &str,
|
||||
dims: usize,
|
||||
) -> anyhow::Result<Box<dyn super::EmbeddingProvider>> {
|
||||
let api_key = resolve_api_key(config, provider_name);
|
||||
let custom_endpoint = provider_name.strip_prefix("custom:").map(|s| s.to_string());
|
||||
let provider_slug = if provider_name.starts_with("custom:") {
|
||||
"custom"
|
||||
} else {
|
||||
provider_name.as_str()
|
||||
provider_name
|
||||
};
|
||||
create_embedding_provider_with_credentials(
|
||||
provider_slug,
|
||||
|
||||
@@ -645,13 +645,24 @@ async fn embeddings_embed_with_custom_openai_endpoint_round_trips_vectors_and_ap
|
||||
);
|
||||
|
||||
let requests = mock_state.requests.lock().expect("mock requests lock");
|
||||
assert_eq!(requests.len(), 1, "custom endpoint should be called once");
|
||||
// Two hits: (0) the save-time connectivity probe update_settings now runs
|
||||
// against custom endpoints (TAURI-RUST-5JR prevention), (1) the real embed.
|
||||
assert_eq!(
|
||||
requests[0].get("model").and_then(Value::as_str),
|
||||
Some("mock-embedding-model")
|
||||
requests.len(),
|
||||
2,
|
||||
"expected one save-time probe + one embed call"
|
||||
);
|
||||
assert_eq!(
|
||||
requests[0].pointer("/input/0").and_then(Value::as_str),
|
||||
Some("connection test"),
|
||||
"first call must be the save-time validation probe"
|
||||
);
|
||||
assert_eq!(
|
||||
requests[1].get("model").and_then(Value::as_str),
|
||||
Some("mock-embedding-model")
|
||||
);
|
||||
assert_eq!(
|
||||
requests[1].pointer("/input/0").and_then(Value::as_str),
|
||||
Some("first custom text")
|
||||
);
|
||||
drop(requests);
|
||||
@@ -668,6 +679,88 @@ async fn embeddings_embed_with_custom_openai_endpoint_round_trips_vectors_and_ap
|
||||
mock_join.abort();
|
||||
}
|
||||
|
||||
/// A mock "OpenAI-compatible" host that has NO embeddings route — every POST to
|
||||
/// `/v1/embeddings` 404s, exactly like a chat-only provider (DeepSeek) does.
|
||||
async fn serve_mock_embeddings_no_api(
|
||||
) -> (String, tokio::task::JoinHandle<Result<(), std::io::Error>>) {
|
||||
async fn not_found() -> (axum::http::StatusCode, &'static str) {
|
||||
(axum::http::StatusCode::NOT_FOUND, "Not Found")
|
||||
}
|
||||
let router = Router::new().route("/v1/embeddings", post(not_found));
|
||||
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
|
||||
.await
|
||||
.expect("bind no-api mock server");
|
||||
let addr = listener.local_addr().expect("no-api mock local_addr");
|
||||
let join = tokio::spawn(async move { axum::serve(listener, router).await });
|
||||
(format!("http://{addr}"), join)
|
||||
}
|
||||
|
||||
/// TAURI-RUST-5JR prevention: `update_settings` must probe a custom endpoint
|
||||
/// and REFUSE to persist one that has no embeddings API (404), returning
|
||||
/// `EMBEDDINGS_ENDPOINT_NO_API` and leaving the stored provider unchanged — so
|
||||
/// the 404-on-every-re-embed Sentry flood can never be configured.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn embeddings_update_settings_rejects_endpoint_with_no_embeddings_api() {
|
||||
let _lock = embeddings_e2e_env_lock();
|
||||
let (rpc_base, _tmp, _guards, _join) = setup_embeddings_test().await;
|
||||
let (mock_base, mock_join) = serve_mock_embeddings_no_api().await;
|
||||
|
||||
// Snapshot the provider before the rejected save.
|
||||
let before = post_json_rpc(
|
||||
&rpc_base,
|
||||
80,
|
||||
"openhuman.embeddings_get_settings",
|
||||
json!({}),
|
||||
)
|
||||
.await;
|
||||
let before_result = assert_no_rpc_error(&before, "get_settings before");
|
||||
let before_inner = before_result.get("result").unwrap_or(before_result);
|
||||
let before_provider = before_inner
|
||||
.get("provider")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or_default()
|
||||
.to_string();
|
||||
|
||||
let update = post_json_rpc(
|
||||
&rpc_base,
|
||||
81,
|
||||
"openhuman.embeddings_update_settings",
|
||||
json!({
|
||||
"provider": "custom",
|
||||
"custom_endpoint": mock_base,
|
||||
"model": "mock-embedding-model",
|
||||
"dimensions": 3,
|
||||
"confirm_wipe": true
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
let update_result = assert_no_rpc_error(&update, "update_settings no-api endpoint");
|
||||
let update_inner = update_result.get("result").unwrap_or(update_result);
|
||||
assert_eq!(
|
||||
update_inner.get("error").and_then(Value::as_str),
|
||||
Some("EMBEDDINGS_ENDPOINT_NO_API"),
|
||||
"a no-embeddings endpoint must be rejected, not saved: {update_inner}"
|
||||
);
|
||||
|
||||
// The stored provider must be unchanged — nothing was persisted.
|
||||
let after = post_json_rpc(
|
||||
&rpc_base,
|
||||
82,
|
||||
"openhuman.embeddings_get_settings",
|
||||
json!({}),
|
||||
)
|
||||
.await;
|
||||
let after_result = assert_no_rpc_error(&after, "get_settings after");
|
||||
let after_inner = after_result.get("result").unwrap_or(after_result);
|
||||
assert_eq!(
|
||||
after_inner.get("provider").and_then(Value::as_str),
|
||||
Some(before_provider.as_str()),
|
||||
"provider must be unchanged after a rejected save: {after_inner}"
|
||||
);
|
||||
|
||||
mock_join.abort();
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn legacy_alias_inference_embed_resolves() {
|
||||
let _lock = embeddings_e2e_env_lock();
|
||||
|
||||
Reference in New Issue
Block a user