diff --git a/crates/goose-server/src/routes/action_required.rs b/crates/goose-server/src/routes/action_required.rs index 9f7bfdfc12c9..9fe4166e1881 100644 --- a/crates/goose-server/src/routes/action_required.rs +++ b/crates/goose-server/src/routes/action_required.rs @@ -1,5 +1,6 @@ +use crate::routes::errors::ErrorResponse; use crate::state::AppState; -use axum::{extract::State, http::StatusCode, routing::post, Json, Router}; +use axum::{extract::State, routing::post, Json, Router}; use goose::permission::permission_confirmation::PrincipalType; use goose::permission::{Permission, PermissionConfirmation}; use serde::{Deserialize, Serialize}; @@ -34,7 +35,7 @@ fn default_principal_type() -> PrincipalType { pub async fn confirm_tool_action( State(state): State>, Json(request): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let agent = state.get_agent_for_route(request.session_id).await?; let permission = match request.action.as_str() { "always_allow" => Permission::AlwaysAllow, @@ -72,6 +73,7 @@ mod tests { mod integration_tests { use super::*; use axum::{body::Body, http::Request}; + use http::StatusCode; use tower::ServiceExt; #[tokio::test(flavor = "multi_thread")] diff --git a/crates/goose-server/src/routes/audio.rs b/crates/goose-server/src/routes/audio.rs index cfd61d44205b..c0364ef3d768 100644 --- a/crates/goose-server/src/routes/audio.rs +++ b/crates/goose-server/src/routes/audio.rs @@ -2,6 +2,7 @@ /// /// This module provides endpoints for audio transcription using OpenAI's Whisper API. /// The OpenAI API key must be configured in the backend for this to work. +use crate::routes::errors::ErrorResponse; use crate::state::AppState; use axum::{ http::StatusCode, @@ -44,18 +45,22 @@ struct WhisperResponse { fn validate_audio_input( audio: &str, mime_type: &str, -) -> Result<(Vec, &'static str), StatusCode> { +) -> Result<(Vec, &'static str), ErrorResponse> { // Decode the base64 audio data - let audio_bytes = BASE64.decode(audio).map_err(|_| StatusCode::BAD_REQUEST)?; + let audio_bytes = BASE64 + .decode(audio) + .map_err(|_| ErrorResponse::bad_request("Invalid base64 audio data"))?; // Check file size if audio_bytes.len() > MAX_AUDIO_SIZE_BYTES { - tracing::warn!( - "Audio file too large: {} bytes (max: {} bytes)", - audio_bytes.len(), - MAX_AUDIO_SIZE_BYTES - ); - return Err(StatusCode::PAYLOAD_TOO_LARGE); + return Err(ErrorResponse { + message: format!( + "Audio file too large: {} bytes (max: {} bytes)", + audio_bytes.len(), + MAX_AUDIO_SIZE_BYTES + ), + status: StatusCode::PAYLOAD_TOO_LARGE, + }); } // Determine file extension based on MIME type @@ -68,19 +73,28 @@ fn validate_audio_input( "audio/m4a" => "m4a", "audio/wav" => "wav", "audio/x-wav" => "wav", - _ => return Err(StatusCode::UNSUPPORTED_MEDIA_TYPE), + _ => { + return Err(ErrorResponse { + message: format!("Unsupported audio format: {}", mime_type), + status: StatusCode::UNSUPPORTED_MEDIA_TYPE, + }) + } }; Ok((audio_bytes, file_extension)) } /// Get OpenAI configuration (API key and host) -fn get_openai_config() -> Result<(String, String), StatusCode> { +fn get_openai_config() -> Result<(String, String), ErrorResponse> { let config = goose::config::Config::global(); - let api_key: String = config.get_secret("OPENAI_API_KEY").map_err(|e| { - tracing::error!("Failed to get OpenAI API key: {:?}", e); - StatusCode::PRECONDITION_FAILED + let api_key: String = config.get_secret("OPENAI_API_KEY").map_err(|e| match e { + goose::config::ConfigError::NotFound(_) => ErrorResponse { + message: "OpenAI API key not configured. Please set OPENAI_API_KEY in settings." + .to_string(), + status: StatusCode::PRECONDITION_FAILED, + }, + _ => ErrorResponse::internal(format!("Failed to get OpenAI API key: {:?}", e)), })?; let openai_host = match config.get("OPENAI_HOST", false) { @@ -101,7 +115,7 @@ async fn send_openai_request( mime_type: &str, api_key: &str, openai_host: &str, -) -> Result { +) -> Result { tracing::info!("Using OpenAI host: {}", openai_host); tracing::info!( "Audio file size: {} bytes, extension: {}, mime_type: {}", @@ -115,8 +129,7 @@ async fn send_openai_request( .file_name(format!("audio.{}", file_extension)) .mime_str(mime_type) .map_err(|e| { - tracing::error!("Failed to create multipart part: {:?}", e); - StatusCode::INTERNAL_SERVER_ERROR + ErrorResponse::internal(format!("Failed to create multipart part: {:?}", e)) })?; let form = reqwest::multipart::Form::new() @@ -130,10 +143,7 @@ async fn send_openai_request( let client = Client::builder() .timeout(Duration::from_secs(OPENAI_TIMEOUT_SECONDS)) .build() - .map_err(|e| { - tracing::error!("Failed to create HTTP client: {}", e); - StatusCode::INTERNAL_SERVER_ERROR - })?; + .map_err(|e| ErrorResponse::internal(format!("Failed to create HTTP client: {}", e)))?; tracing::info!( "Sending request to OpenAI: {}/v1/audio/transcriptions", @@ -148,14 +158,18 @@ async fn send_openai_request( .await .map_err(|e| { if e.is_timeout() { - tracing::error!( - "OpenAI API request timed out after {}s", - OPENAI_TIMEOUT_SECONDS - ); - StatusCode::GATEWAY_TIMEOUT + ErrorResponse { + message: format!( + "OpenAI API request timed out after {}s", + OPENAI_TIMEOUT_SECONDS + ), + status: StatusCode::GATEWAY_TIMEOUT, + } } else { - tracing::error!("Failed to send request to OpenAI: {}", e); - StatusCode::SERVICE_UNAVAILABLE + ErrorResponse { + message: format!("Failed to send request to OpenAI: {}", e), + status: StatusCode::SERVICE_UNAVAILABLE, + } } })?; @@ -171,20 +185,27 @@ async fn send_openai_request( // Check for specific error codes if status == 401 { - tracing::error!("OpenAI API key appears to be invalid or unauthorized"); - return Err(StatusCode::UNAUTHORIZED); + return Err(ErrorResponse { + message: "OpenAI API key appears to be invalid or unauthorized".to_string(), + status: StatusCode::UNAUTHORIZED, + }); } else if status == 429 { - tracing::error!("OpenAI API quota or rate limit exceeded"); - return Err(StatusCode::TOO_MANY_REQUESTS); + return Err(ErrorResponse { + message: "OpenAI API quota or rate limit exceeded".to_string(), + status: StatusCode::TOO_MANY_REQUESTS, + }); } - return Err(StatusCode::BAD_GATEWAY); + return Err(ErrorResponse { + message: format!("OpenAI API error: {}", error_text), + status: StatusCode::BAD_GATEWAY, + }); } - let whisper_response: WhisperResponse = response.json().await.map_err(|e| { - tracing::error!("Failed to parse OpenAI response: {}", e); - StatusCode::INTERNAL_SERVER_ERROR - })?; + let whisper_response: WhisperResponse = response + .json() + .await + .map_err(|e| ErrorResponse::internal(format!("Failed to parse OpenAI response: {}", e)))?; Ok(whisper_response) } @@ -208,7 +229,7 @@ async fn send_openai_request( /// - 503: Service Unavailable (network error) async fn transcribe_handler( Json(request): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let (audio_bytes, file_extension) = validate_audio_input(&request.audio, &request.mime_type)?; let (api_key, openai_host) = get_openai_config()?; @@ -232,7 +253,7 @@ async fn transcribe_handler( /// Requires an ElevenLabs API key with speech-to-text access. async fn transcribe_elevenlabs_handler( Json(request): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let (audio_bytes, file_extension) = validate_audio_input(&request.audio, &request.mime_type)?; // Get the ElevenLabs API key from config (after input validation) @@ -266,17 +287,17 @@ async fn transcribe_elevenlabs_handler( key } None => { - tracing::error!( + return Err(ErrorResponse::bad_request(format!( "ElevenLabs API key is not a string, found: {:?}", value - ); - return Err(StatusCode::PRECONDITION_FAILED); + ))); } } } Err(_) => { - tracing::error!("No ElevenLabs API key found in configuration"); - return Err(StatusCode::PRECONDITION_FAILED); + return Err(ErrorResponse::bad_request( + "No ElevenLabs API key found in configuration", + )); } } } @@ -286,7 +307,7 @@ async fn transcribe_elevenlabs_handler( let part = reqwest::multipart::Part::bytes(audio_bytes) .file_name(format!("audio.{}", file_extension)) .mime_str(&request.mime_type) - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + .map_err(|_| ErrorResponse::internal("Failed to create multipart part"))?; let form = reqwest::multipart::Form::new() .part("file", part) // Changed from "audio" to "file" @@ -298,10 +319,7 @@ async fn transcribe_elevenlabs_handler( let client = Client::builder() .timeout(Duration::from_secs(OPENAI_TIMEOUT_SECONDS)) .build() - .map_err(|e| { - tracing::error!("Failed to create HTTP client: {}", e); - StatusCode::INTERNAL_SERVER_ERROR - })?; + .map_err(|e| ErrorResponse::internal(format!("Failed to create HTTP client: {}", e)))?; let response = client .post("https://api.elevenlabs.io/v1/speech-to-text") @@ -311,14 +329,18 @@ async fn transcribe_elevenlabs_handler( .await .map_err(|e| { if e.is_timeout() { - tracing::error!( - "ElevenLabs API request timed out after {}s", - OPENAI_TIMEOUT_SECONDS - ); - StatusCode::GATEWAY_TIMEOUT + ErrorResponse { + message: format!( + "ElevenLabs API request timed out after {}s", + OPENAI_TIMEOUT_SECONDS + ), + status: StatusCode::GATEWAY_TIMEOUT, + } } else { - tracing::error!("Failed to send request to ElevenLabs: {}", e); - StatusCode::SERVICE_UNAVAILABLE + ErrorResponse { + message: format!("Failed to send request to ElevenLabs: {}", e), + status: StatusCode::SERVICE_UNAVAILABLE, + } } })?; @@ -329,12 +351,21 @@ async fn transcribe_elevenlabs_handler( // Check for specific error codes if error_text.contains("Unauthorized") || error_text.contains("Invalid API key") { - return Err(StatusCode::UNAUTHORIZED); + return Err(ErrorResponse { + message: "ElevenLabs API key is invalid or unauthorized".to_string(), + status: StatusCode::UNAUTHORIZED, + }); } else if error_text.contains("quota") || error_text.contains("limit") { - return Err(StatusCode::PAYMENT_REQUIRED); + return Err(ErrorResponse { + message: "ElevenLabs API quota or rate limit exceeded".to_string(), + status: StatusCode::PAYMENT_REQUIRED, + }); } - return Err(StatusCode::BAD_GATEWAY); + return Err(ErrorResponse { + message: format!("ElevenLabs API error: {}", error_text), + status: StatusCode::BAD_GATEWAY, + }); } // Parse ElevenLabs response @@ -347,8 +378,7 @@ async fn transcribe_elevenlabs_handler( } let elevenlabs_response: ElevenLabsResponse = response.json().await.map_err(|e| { - tracing::error!("Failed to parse ElevenLabs response: {}", e); - StatusCode::INTERNAL_SERVER_ERROR + ErrorResponse::internal(format!("Failed to parse ElevenLabs response: {}", e)) })?; Ok(Json(TranscribeResponse { @@ -359,7 +389,7 @@ async fn transcribe_elevenlabs_handler( /// Check if dictation providers are configured /// /// Returns configuration status for dictation providers -async fn check_dictation_config() -> Result, StatusCode> { +async fn check_dictation_config() -> Result, ErrorResponse> { let config = goose::config::Config::global(); // Check if ElevenLabs API key is configured diff --git a/crates/goose-server/src/routes/config_management.rs b/crates/goose-server/src/routes/config_management.rs index 98fa44c61d34..67aef90fdb20 100644 --- a/crates/goose-server/src/routes/config_management.rs +++ b/crates/goose-server/src/routes/config_management.rs @@ -1,3 +1,4 @@ +use crate::routes::errors::ErrorResponse; use crate::routes::utils::check_provider_configured; use crate::state::AppState; use axum::routing::put; @@ -15,13 +16,11 @@ use goose::providers::auto_detect::detect_provider_from_api_key; use goose::providers::base::{ProviderMetadata, ProviderType}; use goose::providers::canonical::maybe_get_canonical_model; use goose::providers::create_with_default_model; -use goose::providers::errors::ProviderError; use goose::providers::providers as get_providers; use goose::{ agents::execute_commands, agents::ExtensionConfig, config::permission::PermissionLevel, slash_commands, }; -use http::StatusCode; use serde::{Deserialize, Serialize}; use serde_json::Value; use serde_yaml; @@ -163,14 +162,10 @@ pub struct DetectProviderResponse { )] pub async fn upsert_config( Json(query): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let config = Config::global(); - let result = config.set(&query.key, &query.value, query.is_secret); - - match result { - Ok(_) => Ok(Json(Value::String(format!("Upserted key {}", query.key)))), - Err(_) => Err(StatusCode::INTERNAL_SERVER_ERROR), - } + config.set(&query.key, &query.value, query.is_secret)?; + Ok(Json(Value::String(format!("Upserted key {}", query.key)))) } #[utoipa::path( @@ -183,19 +178,18 @@ pub async fn upsert_config( (status = 500, description = "Internal server error") ) )] -pub async fn remove_config(Json(query): Json) -> Result, StatusCode> { +pub async fn remove_config( + Json(query): Json, +) -> Result, ErrorResponse> { let config = Config::global(); - let result = if query.is_secret { - config.delete_secret(&query.key) + if query.is_secret { + config.delete_secret(&query.key)?; } else { - config.delete(&query.key) - }; - - match result { - Ok(_) => Ok(Json(format!("Removed key {}", query.key))), - Err(_) => Err(StatusCode::NOT_FOUND), + config.delete(&query.key)?; } + + Ok(Json(format!("Removed key {}", query.key))) } const SECRET_MASK_SHOW_LEN: usize = 8; @@ -232,12 +226,12 @@ fn is_valid_provider_name(provider_name: &str) -> bool { )] pub async fn read_config( Json(query): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { if query.key == "model-limits" { let limits = ModelConfig::get_all_model_limits(); - return Ok(Json(ConfigValueResponse::Value( - serde_json::to_value(limits).map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?, - ))); + return Ok(Json(ConfigValueResponse::Value(serde_json::to_value( + limits, + )?))); } let config = Config::global(); @@ -253,9 +247,7 @@ pub async fn read_config( } } Err(ConfigError::NotFound(_)) => ConfigValueResponse::Value(Value::Null), - Err(_) => { - return Err(StatusCode::INTERNAL_SERVER_ERROR); - } + Err(e) => return Err(e.into()), }; Ok(Json(response_value)) } @@ -268,7 +260,7 @@ pub async fn read_config( (status = 500, description = "Internal server error") ) )] -pub async fn get_extensions() -> Result, StatusCode> { +pub async fn get_extensions() -> Result, ErrorResponse> { let extensions = goose::config::get_all_extensions(); let warnings = goose::config::get_warnings(); Ok(Json(ExtensionResponse { @@ -290,7 +282,7 @@ pub async fn get_extensions() -> Result, StatusCode> { )] pub async fn add_extension( Json(extension_query): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let extensions = goose::config::get_all_extensions(); let key = goose::config::extensions::name_to_key(&extension_query.name); @@ -317,7 +309,7 @@ pub async fn add_extension( (status = 500, description = "Internal server error") ) )] -pub async fn remove_extension(Path(name): Path) -> Result, StatusCode> { +pub async fn remove_extension(Path(name): Path) -> Result, ErrorResponse> { let key = goose::config::extensions::name_to_key(&name); goose::config::remove_extension(&key); Ok(Json(format!("Removed extension {}", name))) @@ -330,13 +322,11 @@ pub async fn remove_extension(Path(name): Path) -> Result, (status = 200, description = "All configuration values retrieved successfully", body = ConfigResponse) ) )] -pub async fn read_all_config() -> Result, StatusCode> { +pub async fn read_all_config() -> Result, ErrorResponse> { let config = Config::global(); - let values = config .all_values() - .map_err(|_| StatusCode::UNPROCESSABLE_ENTITY)?; - + .map_err(|e| ErrorResponse::unprocessable(e.to_string()))?; Ok(Json(ConfigResponse { config: values })) } @@ -347,7 +337,7 @@ pub async fn read_all_config() -> Result, StatusCode> { (status = 200, description = "All configuration values retrieved successfully", body = [ProviderDetails]) ) )] -pub async fn providers() -> Result>, StatusCode> { +pub async fn providers() -> Result>, ErrorResponse> { let providers = get_providers().await; let providers_response: Vec = providers .into_iter() @@ -381,7 +371,7 @@ pub async fn providers() -> Result>, StatusCode> { )] pub async fn get_provider_models( Path(name): Path, -) -> Result>, StatusCode> { +) -> Result>, ErrorResponse> { let loaded_provider = goose::config::declarative_providers::load_provider(name.as_str()).ok(); // TODO(Douwe): support a get models url for custom providers if let Some(loaded_provider) = loaded_provider { @@ -395,49 +385,29 @@ pub async fn get_provider_models( )); } - let all = get_providers() - .await - .into_iter() - //.map(|(m, p)| m) - .collect::>(); + let all = get_providers().await.into_iter().collect::>(); let Some((metadata, provider_type)) = all.into_iter().find(|(m, _)| m.name == name) else { - return Err(StatusCode::BAD_REQUEST); + return Err(ErrorResponse::bad_request(format!( + "Unknown provider: {}", + name + ))); }; if !check_provider_configured(&metadata, provider_type) { - return Err(StatusCode::BAD_REQUEST); + return Err(ErrorResponse::bad_request(format!( + "Provider '{}' is not configured", + name + ))); } - let model_config = - ModelConfig::new(&metadata.default_model).map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; - let provider = goose::providers::create(&name, model_config) - .await - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + let model_config = ModelConfig::new(&metadata.default_model)?; + let provider = goose::providers::create(&name, model_config).await?; let models_result = provider.fetch_recommended_models().await; match models_result { Ok(Some(models)) => Ok(Json(models)), Ok(None) => Ok(Json(Vec::new())), - Err(provider_error) => { - let status_code = match provider_error { - // Permanent misconfigurations - client should fix configuration - ProviderError::Authentication(_) => StatusCode::BAD_REQUEST, - ProviderError::UsageError(_) => StatusCode::BAD_REQUEST, - - // Transient errors - client should retry later - ProviderError::RateLimitExceeded { .. } => StatusCode::TOO_MANY_REQUESTS, - - // All other errors - internal server error - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - - tracing::warn!( - "Provider {} failed to fetch models: {}", - name, - provider_error - ); - Err(status_code) - } + Err(provider_error) => Err(provider_error.into()), } } @@ -448,7 +418,7 @@ pub async fn get_provider_models( (status = 200, description = "Slash commands retrieved successfully", body = SlashCommandsResponse) ) )] -pub async fn get_slash_commands() -> Result, StatusCode> { +pub async fn get_slash_commands() -> Result, ErrorResponse> { let mut commands: Vec<_> = slash_commands::list_commands() .iter() .map(|command| SlashCommand { @@ -501,9 +471,14 @@ pub struct PricingQuery { )] pub async fn get_pricing( Json(query): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let canonical_model = - maybe_get_canonical_model(&query.provider, &query.model).ok_or(StatusCode::NOT_FOUND)?; + maybe_get_canonical_model(&query.provider, &query.model).ok_or_else(|| { + ErrorResponse::not_found(format!( + "Model '{}/{}' not found", + query.provider, query.model + )) + })?; let mut pricing_data = Vec::new(); @@ -535,7 +510,7 @@ pub async fn get_pricing( (status = 500, description = "Internal server error") ) )] -pub async fn init_config() -> Result, StatusCode> { +pub async fn init_config() -> Result, ErrorResponse> { let config = Config::global(); if config.exists() { @@ -544,10 +519,10 @@ pub async fn init_config() -> Result, StatusCode> { // Use the shared function to load init-config.yaml match goose::config::base::load_init_config_from_workspace() { - Ok(init_values) => match config.initialize_if_empty(init_values) { - Ok(_) => Ok(Json("Config initialized successfully".to_string())), - Err(_) => Err(StatusCode::INTERNAL_SERVER_ERROR), - }, + Ok(init_values) => { + config.initialize_if_empty(init_values)?; + Ok(Json("Config initialized successfully".to_string())) + } Err(_) => Ok(Json( "No init-config.yaml found, using default configuration".to_string(), )), @@ -565,7 +540,7 @@ pub async fn init_config() -> Result, StatusCode> { )] pub async fn upsert_permissions( Json(query): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let permission_manager = goose::config::PermissionManager::instance(); for tool_permission in &query.tool_permissions { @@ -589,7 +564,7 @@ pub async fn upsert_permissions( )] pub async fn detect_provider( Json(detect_request): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let api_key = detect_request.api_key.trim(); match detect_provider_from_api_key(api_key).await { @@ -597,7 +572,9 @@ pub async fn detect_provider( provider_name, models, })), - None => Err(StatusCode::NOT_FOUND), + None => Err(ErrorResponse::not_found( + "Could not detect provider from the provided API key", + )), } } @@ -609,25 +586,23 @@ pub async fn detect_provider( (status = 500, description = "Internal server error") ) )] -pub async fn backup_config() -> Result, StatusCode> { +pub async fn backup_config() -> Result, ErrorResponse> { let config_path = Paths::config_dir().join("config.yaml"); - if config_path.exists() { - let file_name = config_path - .file_name() - .ok_or(StatusCode::INTERNAL_SERVER_ERROR)?; + if !config_path.exists() { + return Err(ErrorResponse::not_found("Config file does not exist")); + } - let mut backup_name = file_name.to_os_string(); - backup_name.push(".bak"); + let file_name = config_path + .file_name() + .ok_or_else(|| ErrorResponse::internal("Invalid config file path"))?; - let backup = config_path.with_file_name(backup_name); - match std::fs::copy(&config_path, &backup) { - Ok(_) => Ok(Json(format!("Copied {:?} to {:?}", config_path, backup))), - Err(_) => Err(StatusCode::INTERNAL_SERVER_ERROR), - } - } else { - Err(StatusCode::INTERNAL_SERVER_ERROR) - } + let mut backup_name = file_name.to_os_string(); + backup_name.push(".bak"); + + let backup = config_path.with_file_name(backup_name); + std::fs::copy(&config_path, &backup)?; + Ok(Json(format!("Copied {:?} to {:?}", config_path, backup))) } #[utoipa::path( @@ -638,27 +613,21 @@ pub async fn backup_config() -> Result, StatusCode> { (status = 500, description = "Internal server error") ) )] -pub async fn recover_config() -> Result, StatusCode> { +pub async fn recover_config() -> Result, ErrorResponse> { let config = Config::global(); // Force a reload which will trigger recovery if needed - match config.all_values() { - Ok(values) => { - let recovered_keys: Vec = values.keys().cloned().collect(); - if recovered_keys.is_empty() { - Ok(Json("Config recovery completed, but no data was recoverable. Starting with empty configuration.".to_string())) - } else { - Ok(Json(format!( - "Config recovery completed. Recovered {} keys: {}", - recovered_keys.len(), - recovered_keys.join(", ") - ))) - } - } - Err(e) => { - tracing::error!("Config recovery failed: {}", e); - Err(StatusCode::INTERNAL_SERVER_ERROR) - } + let values = config.all_values()?; + let recovered_keys: Vec = values.keys().cloned().collect(); + + if recovered_keys.is_empty() { + Ok(Json("Config recovery completed, but no data was recoverable. Starting with empty configuration.".to_string())) + } else { + Ok(Json(format!( + "Config recovery completed. Recovered {} keys: {}", + recovered_keys.len(), + recovered_keys.join(", ") + ))) } } @@ -670,26 +639,18 @@ pub async fn recover_config() -> Result, StatusCode> { (status = 422, description = "Config file is corrupted") ) )] -pub async fn validate_config() -> Result, StatusCode> { +pub async fn validate_config() -> Result, ErrorResponse> { let config_path = Paths::config_dir().join("config.yaml"); if !config_path.exists() { return Ok(Json("Config file does not exist".to_string())); } - match std::fs::read_to_string(&config_path) { - Ok(content) => match serde_yaml::from_str::(&content) { - Ok(_) => Ok(Json("Config file is valid".to_string())), - Err(e) => { - tracing::warn!("Config validation failed: {}", e); - Err(StatusCode::UNPROCESSABLE_ENTITY) - } - }, - Err(e) => { - tracing::error!("Failed to read config file: {}", e); - Err(StatusCode::INTERNAL_SERVER_ERROR) - } - } + let content = std::fs::read_to_string(&config_path)?; + serde_yaml::from_str::(&content) + .map_err(|e| ErrorResponse::unprocessable(format!("Config file is corrupted: {}", e)))?; + + Ok(Json("Config file is valid".to_string())) } #[utoipa::path( post, @@ -703,7 +664,7 @@ pub async fn validate_config() -> Result, StatusCode> { )] pub async fn create_custom_provider( Json(request): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let config = goose::config::declarative_providers::create_custom_provider( goose::config::declarative_providers::CreateCustomProviderParams { engine: request.engine, @@ -715,12 +676,9 @@ pub async fn create_custom_provider( headers: request.headers, requires_auth: request.requires_auth, }, - ) - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + )?; - if let Err(e) = goose::providers::refresh_custom_providers().await { - tracing::warn!("Failed to refresh custom providers after creation: {}", e); - } + goose::providers::refresh_custom_providers().await?; Ok(Json(format!("Custom provider added - ID: {}", config.id()))) } @@ -736,9 +694,11 @@ pub async fn create_custom_provider( )] pub async fn get_custom_provider( Path(id): Path, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let loaded_provider = goose::config::declarative_providers::load_provider(id.as_str()) - .map_err(|_| StatusCode::NOT_FOUND)?; + .map_err(|e| { + ErrorResponse::not_found(format!("Custom provider '{}' not found: {}", id, e)) + })?; Ok(Json(loaded_provider)) } @@ -752,13 +712,10 @@ pub async fn get_custom_provider( (status = 500, description = "Internal server error") ) )] -pub async fn remove_custom_provider(Path(id): Path) -> Result, StatusCode> { - goose::config::declarative_providers::remove_custom_provider(&id) - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; +pub async fn remove_custom_provider(Path(id): Path) -> Result, ErrorResponse> { + goose::config::declarative_providers::remove_custom_provider(&id)?; - if let Err(e) = goose::providers::refresh_custom_providers().await { - tracing::warn!("Failed to refresh custom providers after deletion: {}", e); - } + goose::providers::refresh_custom_providers().await?; Ok(Json(format!("Removed custom provider: {}", id))) } @@ -776,7 +733,7 @@ pub async fn remove_custom_provider(Path(id): Path) -> Result, Json(request): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { goose::config::declarative_providers::update_custom_provider( goose::config::declarative_providers::UpdateCustomProviderParams { id: id.clone(), @@ -789,12 +746,9 @@ pub async fn update_custom_provider( headers: request.headers, requires_auth: request.requires_auth, }, - ) - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + )?; - if let Err(e) = goose::providers::refresh_custom_providers().await { - tracing::warn!("Failed to refresh custom providers after update: {}", e); - } + goose::providers::refresh_custom_providers().await?; Ok(Json(format!("Updated custom provider: {}", id))) } @@ -806,10 +760,10 @@ pub async fn update_custom_provider( )] pub async fn check_provider( Json(CheckProviderRequest { provider }): Json, -) -> Result<(), (StatusCode, String)> { - create_with_default_model(&provider) - .await - .map_err(|err| (StatusCode::BAD_REQUEST, err.to_string()))?; +) -> Result<(), ErrorResponse> { + create_with_default_model(&provider).await.map_err(|err| { + ErrorResponse::bad_request(format!("Provider '{}' check failed: {}", provider, err)) + })?; Ok(()) } @@ -820,17 +774,22 @@ pub async fn check_provider( )] pub async fn set_config_provider( Json(SetProviderRequest { provider, model }): Json, -) -> Result<(), (StatusCode, String)> { +) -> Result<(), ErrorResponse> { create_with_default_model(&provider) .await .and_then(|_| { let config = Config::global(); config - .set_goose_provider(provider) - .and_then(|_| config.set_goose_model(model)) + .set_goose_provider(provider.clone()) + .and_then(|_| config.set_goose_model(model.clone())) .map_err(|e| anyhow::anyhow!(e)) }) - .map_err(|err| (StatusCode::BAD_REQUEST, err.to_string()))?; + .map_err(|err| { + ErrorResponse::bad_request(format!( + "Failed to set provider to '{}' with model '{}': {}", + provider, model, err + )) + })?; Ok(()) } @@ -847,37 +806,39 @@ pub async fn set_config_provider( )] pub async fn configure_provider_oauth( Path(provider_name): Path, -) -> Result, (StatusCode, String)> { +) -> Result, ErrorResponse> { use goose::model::ModelConfig; use goose::providers::create; if !is_valid_provider_name(&provider_name) { - return Err((StatusCode::BAD_REQUEST, "Invalid provider name".to_string())); + return Err(ErrorResponse::bad_request(format!( + "Invalid provider name: '{}'", + provider_name + ))); } - let temp_model = - ModelConfig::new("temp").map_err(|e| (StatusCode::BAD_REQUEST, e.to_string()))?; + let temp_model = ModelConfig::new("temp").map_err(|e| { + ErrorResponse::bad_request(format!("Failed to create temporary model config: {}", e)) + })?; let provider = create(&provider_name, temp_model).await.map_err(|e| { - ( - StatusCode::BAD_REQUEST, - format!("Failed to create provider: {}", e), - ) + ErrorResponse::bad_request(format!( + "Failed to create provider '{}': {}", + provider_name, e + )) })?; provider.configure_oauth().await.map_err(|e| { - ( - StatusCode::BAD_REQUEST, - format!("OAuth configuration failed: {}", e), - ) + ErrorResponse::bad_request(format!( + "OAuth configuration failed for provider '{}': {}", + provider_name, e + )) })?; // Mark the provider as configured after successful OAuth let configured_marker = format!("{}_configured", provider_name); let config = goose::config::Config::global(); - config - .set_param(&configured_marker, true) - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + config.set_param(&configured_marker, true)?; Ok(Json("OAuth configuration completed".to_string())) } diff --git a/crates/goose-server/src/routes/errors.rs b/crates/goose-server/src/routes/errors.rs index fb6ab9b8be21..43835e3d97bf 100644 --- a/crates/goose-server/src/routes/errors.rs +++ b/crates/goose-server/src/routes/errors.rs @@ -4,6 +4,8 @@ use axum::{ Json, }; use goose::config::ConfigError; +use goose::model::ConfigError as ModelConfigError; +use goose::providers::errors::ProviderError; use serde::Serialize; use utoipa::ToSchema; @@ -21,6 +23,27 @@ impl ErrorResponse { status: StatusCode::INTERNAL_SERVER_ERROR, } } + + pub(crate) fn bad_request(message: impl Into) -> Self { + Self { + message: message.into(), + status: StatusCode::BAD_REQUEST, + } + } + + pub(crate) fn not_found(message: impl Into) -> Self { + Self { + message: message.into(), + status: StatusCode::NOT_FOUND, + } + } + + pub(crate) fn unprocessable(message: impl Into) -> Self { + Self { + message: message.into(), + status: StatusCode::UNPROCESSABLE_ENTITY, + } + } } impl IntoResponse for ErrorResponse { @@ -41,6 +64,67 @@ impl From for ErrorResponse { impl From for ErrorResponse { fn from(err: ConfigError) -> Self { - Self::internal(err.to_string()) + match err { + ConfigError::NotFound(key) => Self::not_found(format!("Config key not found: {}", key)), + _ => Self::internal(err.to_string()), + } + } +} + +impl From for ErrorResponse { + fn from(err: ModelConfigError) -> Self { + Self::internal(format!("Model configuration error: {}", err)) + } +} + +impl From for ErrorResponse { + fn from(status: StatusCode) -> Self { + let message = status.canonical_reason().unwrap_or("Unknown error"); + Self { + message: message.to_string(), + status, + } + } +} + +impl From for ErrorResponse { + fn from(err: std::io::Error) -> Self { + Self::internal(format!("IO error: {}", err)) + } +} + +impl From for ErrorResponse { + fn from(err: serde_json::Error) -> Self { + Self::internal(format!("JSON serialization error: {}", err)) + } +} + +impl From for ErrorResponse { + fn from(err: serde_yaml::Error) -> Self { + Self::unprocessable(format!("YAML parsing error: {}", err)) + } +} + +impl From for ErrorResponse { + fn from(err: ProviderError) -> Self { + let (status, message) = match err { + ProviderError::Authentication(_) => ( + StatusCode::BAD_REQUEST, + format!("Authentication failed: {}", err), + ), + ProviderError::UsageError(_) => { + (StatusCode::BAD_REQUEST, format!("Usage error: {}", err)) + } + ProviderError::RateLimitExceeded { .. } => ( + StatusCode::TOO_MANY_REQUESTS, + format!("Rate limit exceeded: {}", err), + ), + _ => ( + StatusCode::INTERNAL_SERVER_ERROR, + format!("Provider error: {}", err), + ), + }; + + Self { message, status } } } diff --git a/crates/goose-server/src/routes/prompts.rs b/crates/goose-server/src/routes/prompts.rs index ed62c1e42707..b807dc591031 100644 --- a/crates/goose-server/src/routes/prompts.rs +++ b/crates/goose-server/src/routes/prompts.rs @@ -1,3 +1,4 @@ +use crate::routes::errors::ErrorResponse; use axum::{ extract::Path, routing::{delete, get, put}, @@ -6,7 +7,6 @@ use axum::{ use goose::prompt_template::{ get_template, list_templates, reset_template, save_template, Template, }; -use http::StatusCode; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; @@ -54,8 +54,9 @@ pub async fn get_prompts() -> Json { )] pub async fn get_prompt( Path(name): Path, -) -> Result, StatusCode> { - let template = get_template(&name).ok_or(StatusCode::NOT_FOUND)?; +) -> Result, ErrorResponse> { + let template = get_template(&name) + .ok_or_else(|| ErrorResponse::not_found(format!("Prompt template '{}' not found", name)))?; let content = template .user_content @@ -86,13 +87,12 @@ pub async fn get_prompt( pub async fn save_prompt( Path(name): Path, Json(request): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { save_template(&name, &request.content).map_err(|e| { if e.kind() == std::io::ErrorKind::NotFound { - StatusCode::NOT_FOUND + ErrorResponse::not_found(format!("Prompt template '{}' not found", name)) } else { - tracing::error!("Failed to save prompt {}: {}", name, e); - StatusCode::INTERNAL_SERVER_ERROR + ErrorResponse::internal(format!("Failed to save prompt '{}': {}", name, e)) } })?; @@ -111,13 +111,12 @@ pub async fn save_prompt( (status = 500, description = "Failed to reset prompt") ) )] -pub async fn reset_prompt(Path(name): Path) -> Result, StatusCode> { +pub async fn reset_prompt(Path(name): Path) -> Result, ErrorResponse> { reset_template(&name).map_err(|e| { if e.kind() == std::io::ErrorKind::NotFound { - StatusCode::NOT_FOUND + ErrorResponse::not_found(format!("Prompt template '{}' not found", name)) } else { - tracing::error!("Failed to reset prompt {}: {}", name, e); - StatusCode::INTERNAL_SERVER_ERROR + ErrorResponse::internal(format!("Failed to reset prompt '{}': {}", name, e)) } })?; diff --git a/crates/goose-server/src/routes/reply.rs b/crates/goose-server/src/routes/reply.rs index 39a6e16bb1e2..897702a719a3 100644 --- a/crates/goose-server/src/routes/reply.rs +++ b/crates/goose-server/src/routes/reply.rs @@ -1,7 +1,10 @@ +use crate::routes::errors::ErrorResponse; use crate::state::AppState; +#[cfg(test)] +use axum::http::StatusCode; use axum::{ extract::{DefaultBodyLimit, State}, - http::{self, StatusCode}, + http::{self}, response::IntoResponse, routing::post, Json, Router, @@ -202,7 +205,7 @@ async fn stream_event( pub async fn reply( State(state): State>, Json(request): Json, -) -> Result { +) -> Result { let session_start = std::time::Instant::now(); tracing::info!( diff --git a/crates/goose-server/src/routes/schedule.rs b/crates/goose-server/src/routes/schedule.rs index 8c7b9bc5f398..5d6d651240f0 100644 --- a/crates/goose-server/src/routes/schedule.rs +++ b/crates/goose-server/src/routes/schedule.rs @@ -8,6 +8,7 @@ use axum::{ }; use serde::{Deserialize, Serialize}; +use crate::routes::errors::ErrorResponse; use crate::state::AppState; use goose::scheduler::ScheduledJob; @@ -87,13 +88,9 @@ pub struct SessionDisplayInfo { async fn create_schedule( State(state): State>, Json(req): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let scheduler = state.scheduler(); - tracing::info!( - "Server: Calling scheduler.add_scheduled_job() for job '{}'", - req.id - ); let job = ScheduledJob { id: req.id, source: req.recipe_source, @@ -107,15 +104,18 @@ async fn create_schedule( scheduler .add_scheduled_job(job.clone(), true) .await - .map_err(|e| { - eprintln!("Error creating schedule: {:?}", e); // Log error - match e { - goose::scheduler::SchedulerError::JobNotFound(_) => StatusCode::NOT_FOUND, - goose::scheduler::SchedulerError::CronParseError(_) => StatusCode::BAD_REQUEST, - goose::scheduler::SchedulerError::RecipeLoadError(_) => StatusCode::BAD_REQUEST, - goose::scheduler::SchedulerError::JobIdExists(_) => StatusCode::CONFLICT, - _ => StatusCode::INTERNAL_SERVER_ERROR, + .map_err(|e| match e { + goose::scheduler::SchedulerError::CronParseError(msg) => { + ErrorResponse::bad_request(format!("Invalid cron expression: {}", msg)) + } + goose::scheduler::SchedulerError::RecipeLoadError(msg) => { + ErrorResponse::bad_request(format!("Recipe load error: {}", msg)) } + goose::scheduler::SchedulerError::JobIdExists(msg) => ErrorResponse { + message: format!("Job ID already exists: {}", msg), + status: StatusCode::CONFLICT, + }, + _ => ErrorResponse::internal(format!("Error creating schedule: {}", e)), })?; Ok(Json(job)) } @@ -132,10 +132,9 @@ async fn create_schedule( #[axum::debug_handler] async fn list_schedules( State(state): State>, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let scheduler = state.scheduler(); - tracing::info!("Server: Calling scheduler.list_scheduled_jobs()"); let jobs = scheduler.list_scheduled_jobs().await; Ok(Json(ListSchedulesResponse { jobs })) } @@ -157,17 +156,16 @@ async fn list_schedules( async fn delete_schedule( State(state): State>, Path(id): Path, -) -> Result { +) -> Result { let scheduler = state.scheduler(); scheduler .remove_scheduled_job(&id, true) .await - .map_err(|e| { - eprintln!("Error deleting schedule '{}': {:?}", id, e); - match e { - goose::scheduler::SchedulerError::JobNotFound(_) => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, + .map_err(|e| match e { + goose::scheduler::SchedulerError::JobNotFound(msg) => { + ErrorResponse::not_found(format!("Schedule not found: {}", msg)) } + _ => ErrorResponse::internal(format!("Error deleting schedule: {}", e)), })?; Ok(StatusCode::NO_CONTENT) } @@ -189,7 +187,7 @@ async fn delete_schedule( async fn run_now_handler( State(state): State>, Path(id): Path, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let scheduler = state.scheduler(); let (recipe_display_name, recipe_version_opt) = if let Some(job) = scheduler @@ -238,28 +236,31 @@ async fn run_now_handler( "Recipe execution started" ); - tracing::info!("Server: Calling scheduler.run_now() for job '{}'", id); - match scheduler.run_now(&id).await { Ok(session_id) => Ok(Json(RunNowResponse { session_id })), - Err(e) => { - eprintln!("Error running schedule '{}' now: {:?}", id, e); - match e { - goose::scheduler::SchedulerError::JobNotFound(_) => Err(StatusCode::NOT_FOUND), - goose::scheduler::SchedulerError::AnyhowError(ref err) => { - // Check if this is a cancellation error - if err.to_string().contains("was successfully cancelled") { - // Return a special session_id to indicate cancellation - Ok(Json(RunNowResponse { - session_id: "CANCELLED".to_string(), - })) - } else { - Err(StatusCode::INTERNAL_SERVER_ERROR) - } + Err(e) => match e { + goose::scheduler::SchedulerError::JobNotFound(msg) => Err(ErrorResponse::not_found( + format!("Schedule not found: {}", msg), + )), + goose::scheduler::SchedulerError::AnyhowError(ref err) => { + // Check if this is a cancellation error + if err.to_string().contains("was successfully cancelled") { + // Return a special session_id to indicate cancellation + Ok(Json(RunNowResponse { + session_id: "CANCELLED".to_string(), + })) + } else { + Err(ErrorResponse::internal(format!( + "Error running schedule: {}", + err + ))) } - _ => Err(StatusCode::INTERNAL_SERVER_ERROR), } - } + _ => Err(ErrorResponse::internal(format!( + "Error running schedule: {}", + e + ))), + }, } } @@ -281,41 +282,32 @@ async fn sessions_handler( State(state): State>, Path(schedule_id_param): Path, // Renamed to avoid confusion with session_id Query(query_params): Query, -) -> Result>, StatusCode> { +) -> Result>, ErrorResponse> { let scheduler = state.scheduler(); - match scheduler + let session_tuples = scheduler .sessions(&schedule_id_param, query_params.limit) .await - { - Ok(session_tuples) => { - let mut display_infos = Vec::new(); - for (session_name, session) in session_tuples { - display_infos.push(SessionDisplayInfo { - id: session_name.clone(), - name: session.name, - created_at: session.created_at.to_rfc3339(), - working_dir: session.working_dir.to_string_lossy().into_owned(), - schedule_id: session.schedule_id, - message_count: session.message_count, - total_tokens: session.total_tokens, - input_tokens: session.input_tokens, - output_tokens: session.output_tokens, - accumulated_total_tokens: session.accumulated_total_tokens, - accumulated_input_tokens: session.accumulated_input_tokens, - accumulated_output_tokens: session.accumulated_output_tokens, - }); - } - Ok(Json(display_infos)) - } - Err(e) => { - eprintln!( - "Error fetching sessions for schedule '{}': {:?}", - schedule_id_param, e - ); - Err(StatusCode::INTERNAL_SERVER_ERROR) - } + .map_err(|e| ErrorResponse::internal(format!("Error fetching sessions: {}", e)))?; + + let mut display_infos = Vec::new(); + for (session_name, session) in session_tuples { + display_infos.push(SessionDisplayInfo { + id: session_name.clone(), + name: session.name, + created_at: session.created_at.to_rfc3339(), + working_dir: session.working_dir.to_string_lossy().into_owned(), + schedule_id: session.schedule_id, + message_count: session.message_count, + total_tokens: session.total_tokens, + input_tokens: session.input_tokens, + output_tokens: session.output_tokens, + accumulated_total_tokens: session.accumulated_total_tokens, + accumulated_input_tokens: session.accumulated_input_tokens, + accumulated_output_tokens: session.accumulated_output_tokens, + }); } + Ok(Json(display_infos)) } #[utoipa::path( @@ -336,16 +328,17 @@ async fn sessions_handler( async fn pause_schedule( State(state): State>, Path(id): Path, -) -> Result { +) -> Result { let scheduler = state.scheduler(); - scheduler.pause_schedule(&id).await.map_err(|e| { - eprintln!("Error pausing schedule '{}': {:?}", id, e); - match e { - goose::scheduler::SchedulerError::JobNotFound(_) => StatusCode::NOT_FOUND, - goose::scheduler::SchedulerError::AnyhowError(_) => StatusCode::BAD_REQUEST, - _ => StatusCode::INTERNAL_SERVER_ERROR, + scheduler.pause_schedule(&id).await.map_err(|e| match e { + goose::scheduler::SchedulerError::JobNotFound(msg) => { + ErrorResponse::not_found(format!("Schedule not found: {}", msg)) + } + goose::scheduler::SchedulerError::AnyhowError(err) => { + ErrorResponse::bad_request(format!("Cannot pause schedule: {}", err)) } + _ => ErrorResponse::internal(format!("Error pausing schedule: {}", e)), })?; Ok(StatusCode::NO_CONTENT) } @@ -367,15 +360,14 @@ async fn pause_schedule( async fn unpause_schedule( State(state): State>, Path(id): Path, -) -> Result { +) -> Result { let scheduler = state.scheduler(); - scheduler.unpause_schedule(&id).await.map_err(|e| { - eprintln!("Error unpausing schedule '{}': {:?}", id, e); - match e { - goose::scheduler::SchedulerError::JobNotFound(_) => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, + scheduler.unpause_schedule(&id).await.map_err(|e| match e { + goose::scheduler::SchedulerError::JobNotFound(msg) => { + ErrorResponse::not_found(format!("Schedule not found: {}", msg)) } + _ => ErrorResponse::internal(format!("Error unpausing schedule: {}", e)), })?; Ok(StatusCode::NO_CONTENT) } @@ -400,27 +392,30 @@ async fn update_schedule( State(state): State>, Path(id): Path, Json(req): Json, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let scheduler = state.scheduler(); scheduler .update_schedule(&id, req.cron) .await - .map_err(|e| { - eprintln!("Error updating schedule '{}': {:?}", id, e); - match e { - goose::scheduler::SchedulerError::JobNotFound(_) => StatusCode::NOT_FOUND, - goose::scheduler::SchedulerError::AnyhowError(_) => StatusCode::BAD_REQUEST, - goose::scheduler::SchedulerError::CronParseError(_) => StatusCode::BAD_REQUEST, - _ => StatusCode::INTERNAL_SERVER_ERROR, + .map_err(|e| match e { + goose::scheduler::SchedulerError::JobNotFound(msg) => { + ErrorResponse::not_found(format!("Schedule not found: {}", msg)) } + goose::scheduler::SchedulerError::AnyhowError(err) => { + ErrorResponse::bad_request(format!("Cannot update schedule: {}", err)) + } + goose::scheduler::SchedulerError::CronParseError(msg) => { + ErrorResponse::bad_request(format!("Invalid cron expression: {}", msg)) + } + _ => ErrorResponse::internal(format!("Error updating schedule: {}", e)), })?; let jobs = scheduler.list_scheduled_jobs().await; let updated_job = jobs .into_iter() .find(|job| job.id == id) - .ok_or(StatusCode::INTERNAL_SERVER_ERROR)?; + .ok_or_else(|| ErrorResponse::internal("Schedule not found after update"))?; Ok(Json(updated_job)) } @@ -437,16 +432,17 @@ async fn update_schedule( pub async fn kill_running_job( State(state): State>, Path(id): Path, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let scheduler = state.scheduler(); - scheduler.kill_running_job(&id).await.map_err(|e| { - eprintln!("Error killing running job '{}': {:?}", id, e); - match e { - goose::scheduler::SchedulerError::JobNotFound(_) => StatusCode::NOT_FOUND, - goose::scheduler::SchedulerError::AnyhowError(_) => StatusCode::BAD_REQUEST, - _ => StatusCode::INTERNAL_SERVER_ERROR, + scheduler.kill_running_job(&id).await.map_err(|e| match e { + goose::scheduler::SchedulerError::JobNotFound(msg) => { + ErrorResponse::not_found(format!("Job not found: {}", msg)) + } + goose::scheduler::SchedulerError::AnyhowError(err) => { + ErrorResponse::bad_request(format!("Cannot kill job: {}", err)) } + _ => ErrorResponse::internal(format!("Error killing job: {}", e)), })?; Ok(Json(KillJobResponse { @@ -471,33 +467,32 @@ pub async fn kill_running_job( pub async fn inspect_running_job( State(state): State>, Path(id): Path, -) -> Result, StatusCode> { +) -> Result, ErrorResponse> { let scheduler = state.scheduler(); - match scheduler.get_running_job_info(&id).await { - Ok(info) => { - if let Some((session_id, start_time)) = info { - let duration = chrono::Utc::now().signed_duration_since(start_time); - Ok(Json(InspectJobResponse { - session_id: Some(session_id), - process_start_time: Some(start_time.to_rfc3339()), - running_duration_seconds: Some(duration.num_seconds()), - })) - } else { - Ok(Json(InspectJobResponse { - session_id: None, - process_start_time: None, - running_duration_seconds: None, - })) - } - } - Err(e) => { - eprintln!("Error inspecting running job '{}': {:?}", id, e); - match e { - goose::scheduler::SchedulerError::JobNotFound(_) => Err(StatusCode::NOT_FOUND), - _ => Err(StatusCode::INTERNAL_SERVER_ERROR), + let info = scheduler + .get_running_job_info(&id) + .await + .map_err(|e| match e { + goose::scheduler::SchedulerError::JobNotFound(msg) => { + ErrorResponse::not_found(format!("Job not found: {}", msg)) } - } + _ => ErrorResponse::internal(format!("Error inspecting job: {}", e)), + })?; + + if let Some((session_id, start_time)) = info { + let duration = chrono::Utc::now().signed_duration_since(start_time); + Ok(Json(InspectJobResponse { + session_id: Some(session_id), + process_start_time: Some(start_time.to_rfc3339()), + running_duration_seconds: Some(duration.num_seconds()), + })) + } else { + Ok(Json(InspectJobResponse { + session_id: None, + process_start_time: None, + running_duration_seconds: None, + })) } } diff --git a/crates/goose-server/src/routes/setup.rs b/crates/goose-server/src/routes/setup.rs index 46406a16dd89..fbf9ef01e79e 100644 --- a/crates/goose-server/src/routes/setup.rs +++ b/crates/goose-server/src/routes/setup.rs @@ -1,5 +1,6 @@ +use crate::routes::errors::ErrorResponse; use crate::state::AppState; -use axum::{http::StatusCode, routing::post, Json, Router}; +use axum::{routing::post, Json, Router}; use goose::config::signup_openrouter::OpenRouterAuth; use goose::config::signup_tetrate::{configure_tetrate, TetrateAuth}; use goose::config::{configure_openrouter, Config}; @@ -27,43 +28,30 @@ pub fn routes(state: Arc) -> Router { (status = 200, body=SetupResponse) ), )] -async fn start_openrouter_setup() -> Result, StatusCode> { - tracing::info!("Starting OpenRouter setup flow"); - - let mut auth_flow = OpenRouterAuth::new().map_err(|e| { - tracing::error!("Failed to initialize auth flow: {}", e); - StatusCode::INTERNAL_SERVER_ERROR - })?; - - tracing::info!("Auth flow initialized, starting complete_flow"); +async fn start_openrouter_setup() -> Result, ErrorResponse> { + let mut auth_flow = OpenRouterAuth::new() + .map_err(|e| ErrorResponse::internal(format!("Failed to initialize auth flow: {}", e)))?; match auth_flow.complete_flow().await { Ok(api_key) => { - tracing::info!("Got API key, configuring OpenRouter..."); - let config = Config::global(); if let Err(e) = configure_openrouter(config, api_key) { - tracing::error!("Failed to configure OpenRouter: {}", e); return Ok(Json(SetupResponse { success: false, message: format!("Failed to configure OpenRouter: {}", e), })); } - tracing::info!("OpenRouter setup completed successfully"); Ok(Json(SetupResponse { success: true, message: "OpenRouter setup completed successfully".to_string(), })) } - Err(e) => { - tracing::error!("OpenRouter setup failed: {}", e); - Ok(Json(SetupResponse { - success: false, - message: format!("Setup failed: {}", e), - })) - } + Err(e) => Ok(Json(SetupResponse { + success: false, + message: format!("Setup failed: {}", e), + })), } } @@ -74,42 +62,29 @@ async fn start_openrouter_setup() -> Result, StatusCode> { (status = 200, body=SetupResponse) ), )] -async fn start_tetrate_setup() -> Result, StatusCode> { - tracing::info!("Starting Tetrate Agent Router Service setup flow"); - - let mut auth_flow = TetrateAuth::new().map_err(|e| { - tracing::error!("Failed to initialize auth flow: {}", e); - StatusCode::INTERNAL_SERVER_ERROR - })?; - - tracing::info!("Auth flow initialized, starting complete_flow"); +async fn start_tetrate_setup() -> Result, ErrorResponse> { + let mut auth_flow = TetrateAuth::new() + .map_err(|e| ErrorResponse::internal(format!("Failed to initialize auth flow: {}", e)))?; match auth_flow.complete_flow().await { Ok(api_key) => { - tracing::info!("Got API key, configuring Tetrate Agent Router Service..."); - let config = Config::global(); if let Err(e) = configure_tetrate(config, api_key) { - tracing::error!("Failed to configure Tetrate Agent Router Service: {}", e); return Ok(Json(SetupResponse { success: false, message: format!("Failed to configure Tetrate Agent Router Service: {}", e), })); } - tracing::info!("Tetrate Agent Router Service setup completed successfully"); Ok(Json(SetupResponse { success: true, message: "Tetrate Agent Router Service setup completed successfully".to_string(), })) } - Err(e) => { - tracing::error!("Tetrate Agent Router Service setup failed: {}", e); - Ok(Json(SetupResponse { - success: false, - message: format!("Setup failed: {}", e), - })) - } + Err(e) => Ok(Json(SetupResponse { + success: false, + message: format!("Setup failed: {}", e), + })), } }