scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Merge pull request #729 from fabro-sh/fix/doctor-timeout-budget

Browse source 
Bound doctor diagnostics within client timeout
This commit is contained in:
Bryan Helmkamp 2026-08-20 21:58:22 -04:00 • committed by GitHub
commit 66dc30424a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 256 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
docs/public/api-reference/fabro-api.yaml
View file

@ -596,6 +596,15 @@ paths:
application/json:
schema:
$ref: "#/components/schemas/DiagnosticsReport"
"504":
description: Diagnostics operation timed out
headers:
x-request-id:
$ref: "#/components/headers/XRequestId"
content:
application/json:
schema:
$ref: "#/components/schemas/ErrorResponse"
/api/v1/openapi.json:
get:

84
lib/apps/fabro-server/src/diagnostics.rs
View file

@ -6,7 +6,7 @@ use base64::Engine as _;
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
use fabro_auth::auth_issue_message;
use fabro_llm::client::Client as LlmClient;
use fabro_llm::model_test::{ModelTestStatus, run_basic_model_probe};
use fabro_llm::model_test::{ModelTestStatus, run_basic_model_probe_with_timeout};
use fabro_model::{Catalog, ProviderId};
use fabro_redact::redact_string;
use fabro_sandbox::{DockerSandboxProvider, daytona};
@ -23,6 +23,9 @@ use tokio::time::timeout;
use crate::server::AppState;
const EXTERNAL_SERVICE_PROBE_TIMEOUT: Duration = Duration::from_secs(15);
const DOCKER_PROBE_TIMEOUT: Duration = Duration::from_secs(5);
fn http_client_or_check(
name: &str,
status: CheckStatus,
@ -252,13 +255,20 @@ async fn probe_single_provider(
None,
);
};
let model_id = model.id.clone();
let model_id = model.id.to_string();
let outcome = run_basic_model_probe_with_timeout(
&model_id,
&provider,
client,
EXTERNAL_SERVICE_PROBE_TIMEOUT,
)
.await;
let outcome = run_basic_model_probe(model_id.as_str(), &provider, client).await;
match outcome.status {
ModelTestStatus::Ok => ProviderProbeResult {
provider,
model_id: Some(model_id.to_string()),
model_id: Some(model_id),
status: ProviderProbeStatus::Ok,
error_message: None,
diagnostic_detail: None,
@ -267,12 +277,7 @@ async fn probe_single_provider(
let raw = outcome
.error_message
.unwrap_or_else(|| "provider probe failed".to_string());
provider_probe_error(
provider,
Some(model_id.to_string()),
redact_string(&raw),
None,
)
provider_probe_error(provider, Some(model_id), redact_string(&raw), None)
}
}
}
@ -388,7 +393,7 @@ async fn check_github_app(state: &AppState) -> CheckResult {
Err(result) => return result,
};
let probe = timeout(
Duration::from_secs(15),
EXTERNAL_SERVICE_PROBE_TIMEOUT,
http.get(format!("{}/user", fabro_github::github_api_base_url()))
.header("Authorization", format!("Bearer {token}"))
.header("Accept", "application/vnd.github+json")
@ -538,7 +543,7 @@ async fn check_github_app(state: &AppState) -> CheckResult {
Err(result) => return result,
};
let auth_result = timeout(
Duration::from_secs(15),
EXTERNAL_SERVICE_PROBE_TIMEOUT,
fabro_github::get_authenticated_app(&http, &jwt, &fabro_github::github_api_base_url()),
)
.await;
@ -581,7 +586,7 @@ async fn check_docker_sandbox(state: &AppState) -> CheckResult {
.await
.map_err(|err| err.display_with_causes())
},
Duration::from_secs(5),
DOCKER_PROBE_TIMEOUT,
)
.await
}
@ -656,7 +661,14 @@ async fn check_cloud_sandbox(state: &AppState) -> CheckResult {
};
};
match state.check_daytona_api_key(api_key).await {
let probe = state
.check_daytona_api_key_with_timeout(api_key, EXTERNAL_SERVICE_PROBE_TIMEOUT)
.await;
cloud_sandbox_probe_check(probe)
}
fn cloud_sandbox_probe_check(probe: anyhow::Result<daytona::DaytonaKeyCheck>) -> CheckResult {
match probe {
Ok(check) if check.ok() => CheckResult {
name: "Cloud Sandbox".to_string(),
status: CheckStatus::Pass,
@ -678,13 +690,29 @@ async fn check_cloud_sandbox(state: &AppState) -> CheckResult {
daytona::required_perms_display()
)),
},
Err(err) => CheckResult {
name: "Cloud Sandbox".to_string(),
status: CheckStatus::Error,
summary: "Daytona credential rejected".to_string(),
details: vec![CheckDetail::new(format!("{err:#}"))],
remediation: Some("Verify DAYTONA_API_KEY value and Daytona reachability".to_string()),
},
Err(err) => {
if let Some(timeout) = err.downcast_ref::<daytona::DaytonaCredentialProbeTimeout>() {
return CheckResult {
name: "Cloud Sandbox".to_string(),
status: CheckStatus::Error,
summary: format!("timeout ({:?})", timeout.timeout()),
details: vec![CheckDetail::new("Daytona probe timed out".to_string())],
remediation: Some(
"Verify DAYTONA_API_KEY value and Daytona reachability".to_string(),
),
};
}
CheckResult {
name: "Cloud Sandbox".to_string(),
status: CheckStatus::Error,
summary: "Daytona credential rejected".to_string(),
details: vec![CheckDetail::new(format!("{err:#}"))],
remediation: Some(
"Verify DAYTONA_API_KEY value and Daytona reachability".to_string(),
),
}
}
}
}
@ -750,7 +778,7 @@ async fn check_brave_search(state: &AppState) -> CheckResult {
Err(result) => return result,
};
let probe = timeout(Duration::from_secs(15), async move {
let probe = timeout(EXTERNAL_SERVICE_PROBE_TIMEOUT, async move {
http.get("https://api.search.brave.com/res/v1/web/search?q=test&count=1")
.header("X-Subscription-Token", api_key)
.send()
@ -1154,6 +1182,18 @@ enabled = false
);
}
#[test]
fn check_cloud_sandbox_reports_timeout() {
let result = cloud_sandbox_probe_check(Err(anyhow::Error::new(
daytona::DaytonaCredentialProbeTimeout::new(Duration::from_millis(1)),
)));
assert_eq!(result.name, "Cloud Sandbox");
assert_eq!(result.status, CheckStatus::Error);
assert_eq!(result.summary, "timeout (1ms)");
assert_eq!(result.details[0].text, "Daytona probe timed out");
}
#[tokio::test]
async fn check_brave_search_ignores_env_backed_api_key() {
let state = TestAppStateBuilder::new()

19
lib/apps/fabro-server/src/server.rs
View file

@ -1455,6 +1455,15 @@ impl AppState {
pub(crate) async fn check_daytona_api_key(
&self,
api_key: String,
) -> anyhow::Result<daytona::DaytonaKeyCheck> {
self.check_daytona_api_key_with_timeout(api_key, daytona::DAYTONA_CREDENTIAL_PROBE_TIMEOUT)
.await
}
pub(crate) async fn check_daytona_api_key_with_timeout(
&self,
api_key: String,
probe_timeout: Duration,
) -> anyhow::Result<daytona::DaytonaKeyCheck> {
let base_url = self
.config_env_lookup(EnvVars::DAYTONA_API_URL)
@ -1463,8 +1472,14 @@ impl AppState {
let org_id = self.config_env_lookup(EnvVars::DAYTONA_ORGANIZATION_ID);
let http_client = fabro_http::http_client().context("failed to build HTTP client")?;
daytona::check_daytona_api_key_with(&base_url, org_id.as_deref(), api_key, http_client)
.await
daytona::check_daytona_api_key_with_timeout(
&base_url,
org_id.as_deref(),
api_key,
http_client,
probe_timeout,
)
.await
}
/// Borrow the persistent store so sibling modules can open run readers

59
lib/apps/fabro-server/src/server/handler/system.rs
View file

@ -1,5 +1,7 @@
use std::collections::BTreeMap;
use std::future::Future;
use std::sync::Arc;
use std::time::Duration;
use chrono::Utc;
use fabro_slack::config::{
@ -9,6 +11,7 @@ use fabro_slack::config::{
use fabro_static::EnvVars;
use fabro_types::settings::server::GithubIntegrationSettings;
use fabro_vault::Vault;
use tokio::time::timeout;
use super::super::{
AggregateBilling, AggregateBillingTotals, ApiError, AppState, BilledTokenCounts,
@ -21,6 +24,8 @@ use super::super::{
resource_sampler, spawn_blocking, system_sandbox_provider, to_i64,
};
const SERVER_DIAGNOSTICS_TIMEOUT: Duration = Duration::from_secs(25);
pub(super) fn routes() -> Router<Arc<AppState>> {
Router::new()
.route("/repos/github/{owner}/{name}", get(get_github_repo))
@ -683,11 +688,34 @@ async fn get_github_repo(
}
async fn run_diagnostics(_auth: RequiredUser, State(state): State<Arc<AppState>>) -> Response {
(
StatusCode::OK,
Json(diagnostics::run_all(state.as_ref()).await),
diagnostics_response_with_timeout(
Box::pin(diagnostics::run_all(state.as_ref())),
SERVER_DIAGNOSTICS_TIMEOUT,
)
.into_response()
.await
}
async fn diagnostics_response_with_timeout<F>(
diagnostics: F,
operation_timeout: Duration,
) -> Response
where
F: Future<Output = diagnostics::DiagnosticsReport>,
{
let Ok(report) = timeout(operation_timeout, diagnostics).await else {
tracing::warn!(
timeout_secs = operation_timeout.as_secs(),
"server diagnostics timed out"
);
return ApiError::with_code(
StatusCode::GATEWAY_TIMEOUT,
"Server diagnostics timed out.",
"diagnostics_timeout",
)
.into_response();
};
(StatusCode::OK, Json(report)).into_response()
}
pub(in crate::server) async fn openapi_spec() -> Response {
@ -737,3 +765,26 @@ async fn get_aggregate_billing(
};
(StatusCode::OK, Json(response)).into_response()
}
#[cfg(test)]
mod tests {
use super::*;
#[tokio::test]
async fn diagnostics_response_returns_gateway_timeout_before_client_deadline() {
let response = diagnostics_response_with_timeout(
std::future::pending::<diagnostics::DiagnosticsReport>(),
Duration::from_millis(1),
)
.await;
let body = fabro_test::expect_axum_json(
response,
StatusCode::GATEWAY_TIMEOUT,
"GET /api/v1/system/diagnostics timeout",
)
.await;
assert_eq!(body["errors"][0]["code"], "diagnostics_timeout");
assert_eq!(body["errors"][0]["detail"], "Server diagnostics timed out.");
}
}

43
lib/components/fabro-llm/src/model_test.rs
View file

@ -1,3 +1,4 @@
use std::future::Future;
use std::sync::Arc;
use std::time::Duration;
@ -63,22 +64,38 @@ pub async fn run_basic_model_probe(
model_id: &str,
provider: impl ToString,
client: Arc<Client>,
) -> ModelTestOutcome {
run_basic_model_probe_with_timeout(
model_id,
provider,
client,
Duration::from_secs(ModelTestMode::Basic.timeout_secs()),
)
.await
}
pub async fn run_basic_model_probe_with_timeout(
model_id: &str,
provider: impl ToString,
client: Arc<Client>,
probe_timeout: Duration,
) -> ModelTestOutcome {
let params = GenerateParams::new(model_id, client)
.provider(provider.to_string())
.prompt("Say OK")
.max_tokens(16);
let result = time::timeout(
Duration::from_secs(ModelTestMode::Basic.timeout_secs()),
generate::generate(params),
)
.await;
basic_model_probe_outcome(generate::generate(params), probe_timeout).await
}
match result {
async fn basic_model_probe_outcome<F>(probe: F, probe_timeout: Duration) -> ModelTestOutcome
where
F: Future<Output = Result<GenerateResult, crate::Error>>,
{
match time::timeout(probe_timeout, probe).await {
Ok(Ok(_)) => ModelTestOutcome::ok(),
Ok(Err(err)) => ModelTestOutcome::error(err.to_string()),
Err(_) => ModelTestOutcome::error("timeout (30s)"),
Err(_) => ModelTestOutcome::error(format!("timeout ({probe_timeout:?})")),
}
}
@ -244,6 +261,18 @@ mod tests {
);
}
#[tokio::test]
async fn basic_model_probe_reports_configured_timeout() {
let outcome = basic_model_probe_outcome(
std::future::pending::<Result<GenerateResult, crate::Error>>(),
Duration::from_millis(1),
)
.await;
assert_eq!(outcome.status, ModelTestStatus::Error);
assert_eq!(outcome.error_message.as_deref(), Some("timeout (1ms)"));
}
#[test]
fn deep_test_omits_effort_for_reasoning_without_effort_controls() {
let info = test_model_with(ModelFeatures {

89
lib/components/fabro-sandbox/src/daytona/mod.rs
View file

@ -1,5 +1,6 @@
use std::collections::HashMap;
use std::fmt::Write;
use std::future::Future;
use std::path::Path;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
@ -64,7 +65,8 @@ pub const DEFAULT_DAYTONA_API_URL: &str = "https://app.daytona.io/api";
pub(crate) const DAYTONA_DASHBOARD_SANDBOXES_URL: &str =
"https://app.daytona.io/dashboard/sandboxes";
const FABRO_SANDBOX_USER_AGENT: &str = concat!("fabro-sandbox/", env!("CARGO_PKG_VERSION"));
const DAYTONA_PROBE_TIMEOUT: Duration = Duration::from_secs(20);
pub const DAYTONA_CREDENTIAL_PROBE_TIMEOUT: Duration = Duration::from_secs(20);
const DAYTONA_BASH_SESSION_PROBE_TIMEOUT: Duration = Duration::from_secs(20);
/// Upper bound on explicit and Drop-triggered Daytona cleanup calls (session
/// deletion, temporary stdin files) so a stalled REST call cannot block
/// cancellation/timeout paths indefinitely.
@ -169,6 +171,24 @@ pub struct DaytonaKeyCheck {
pub missing: Vec<Permissions>,
}
#[derive(Debug, thiserror::Error)]
#[error("Daytona credential probe timed out after {timeout:?}")]
pub struct DaytonaCredentialProbeTimeout {
timeout: Duration,
}
impl DaytonaCredentialProbeTimeout {
#[must_use]
pub const fn new(timeout: Duration) -> Self {
Self { timeout }
}
#[must_use]
pub const fn timeout(&self) -> Duration {
self.timeout
}
}
impl DaytonaKeyCheck {
pub fn ok(&self) -> bool {
self.missing.is_empty()
@ -266,6 +286,23 @@ pub async fn check_daytona_api_key_with(
org_id: Option<&str>,
api_key: String,
http_client: fabro_http::HttpClient,
) -> anyhow::Result<DaytonaKeyCheck> {
check_daytona_api_key_with_timeout(
base_url,
org_id,
api_key,
http_client,
DAYTONA_CREDENTIAL_PROBE_TIMEOUT,
)
.await
}
pub async fn check_daytona_api_key_with_timeout(
base_url: &str,
org_id: Option<&str>,
api_key: String,
http_client: fabro_http::HttpClient,
probe_timeout: Duration,
) -> anyhow::Result<DaytonaKeyCheck> {
let work = async {
let client = build_daytona_client_with(
@ -300,12 +337,21 @@ pub async fn check_daytona_api_key_with(
})
};
match time::timeout(DAYTONA_PROBE_TIMEOUT, work).await {
daytona_credential_probe_with_timeout(work, probe_timeout).await
}
async fn daytona_credential_probe_with_timeout<F>(
probe: F,
probe_timeout: Duration,
) -> anyhow::Result<DaytonaKeyCheck>
where
F: Future<Output = anyhow::Result<DaytonaKeyCheck>>,
{
match time::timeout(probe_timeout, probe).await {
Ok(result) => result,
Err(_) => Err(anyhow::anyhow!(
"Daytona credential probe timed out after {}s",
DAYTONA_PROBE_TIMEOUT.as_secs()
)),
Err(_) => Err(anyhow::Error::new(DaytonaCredentialProbeTimeout::new(
probe_timeout,
))),
}
}
@ -633,16 +679,16 @@ impl DaytonaSandbox {
/// non-POSIX, and completion assertions all hold.
///
/// Costs one session round trip plus a single status poll per sandbox
/// lifecycle transition. `DAYTONA_PROBE_TIMEOUT` is the outer backstop for
/// a stalled REST call; the inner [`BASH_PROBE_TIMEOUT_MS`] is the deadline
/// for the command itself. Session cleanup runs outside that deadline under
/// its own bounded timeout.
/// lifecycle transition. `DAYTONA_BASH_SESSION_PROBE_TIMEOUT` is the outer
/// backstop for a stalled REST call; the inner [`BASH_PROBE_TIMEOUT_MS`] is
/// the deadline for the command itself. Session cleanup runs outside that
/// deadline under its own bounded timeout.
async fn probe_bash_session(sandbox: &daytona_sdk::Sandbox) -> crate::Result<()> {
let deadline = time::Instant::now() + DAYTONA_PROBE_TIMEOUT;
let deadline = time::Instant::now() + DAYTONA_BASH_SESSION_PROBE_TIMEOUT;
let timeout_error = || {
crate::Error::message(format!(
"Daytona Bash session check timed out after {}s",
DAYTONA_PROBE_TIMEOUT.as_secs()
DAYTONA_BASH_SESSION_PROBE_TIMEOUT.as_secs()
))
};
let mut session = match time::timeout_at(deadline, DaytonaSession::create(sandbox)).await {
@ -3755,6 +3801,25 @@ mod tests {
auth.assert_async().await;
}
#[tokio::test]
async fn daytona_credential_probe_reports_configured_timeout() {
let err = daytona_credential_probe_with_timeout(
std::future::pending::<anyhow::Result<DaytonaKeyCheck>>(),
Duration::from_millis(1),
)
.await
.expect_err("probe should time out");
let timeout = err
.downcast_ref::<DaytonaCredentialProbeTimeout>()
.expect("timeout should preserve its type");
assert_eq!(timeout.timeout(), Duration::from_millis(1));
assert_eq!(
err.to_string(),
"Daytona credential probe timed out after 1ms"
);
}
#[tokio::test]
async fn daytona_stdin_file_uploads_exact_bytes_and_is_deleted() {
let server = MockServer::start_async().await;