Show errors on failure (#5643)

Co-authored-by: Douwe Osinga <douwe@squareup.com>
This commit is contained in:
Douwe Osinga
2025-11-10 13:39:23 -05:00
committed by GitHub
parent a971a64007
commit 8cd379ca33
17 changed files with 126 additions and 312 deletions
+1 -1
View File
@@ -9,7 +9,7 @@ use crate::commands::configure::handle_configure;
use crate::commands::info::handle_info;
use crate::commands::project::{handle_project_default, handle_projects_interactive};
use crate::commands::recipe::{handle_deeplink, handle_list, handle_open, handle_validate};
// Import the new handlers from commands::schedule
use crate::commands::schedule::{
handle_schedule_add, handle_schedule_cron_help, handle_schedule_list, handle_schedule_remove,
handle_schedule_run_now, handle_schedule_services_status, handle_schedule_services_stop,
+47 -17
View File
@@ -2,43 +2,73 @@ use anyhow::Result;
use console::style;
use goose::config::paths::Paths;
use goose::config::Config;
use goose::session::session_manager::{DB_NAME, SESSIONS_FOLDER};
use serde_yaml;
fn print_aligned(label: &str, value: &str, width: usize) {
println!(" {:<width$} {}", label, value, width = width);
}
use goose::config::base::CONFIG_YAML_NAME;
use std::fs;
use std::path::Path;
fn check_path_status(path: &Path) -> String {
if path.exists() {
"".to_string()
} else {
let mut current = path.parent();
while let Some(parent) = current {
if parent.exists() {
return match fs::metadata(parent).map(|m| !m.permissions().readonly()) {
Ok(true) => style("missing (can create)").dim().to_string(),
Ok(false) => style("missing (read-only parent)").red().to_string(),
Err(_) => style("missing (cannot check)").red().to_string(),
};
}
current = parent.parent();
}
style("missing (no writable parent)").red().to_string()
}
}
pub fn handle_info(verbose: bool) -> Result<()> {
let logs_dir = Paths::in_state_dir("logs");
let sessions_dir = Paths::in_data_dir("sessions");
let sessions_db = sessions_dir.join("sessions.db");
// Get paths using a stored reference to the global config
let sessions_dir = Paths::in_data_dir(SESSIONS_FOLDER);
let sessions_db = sessions_dir.join(DB_NAME);
let config = Config::global();
let config_dir = Paths::config_dir().display().to_string();
let config_dir = Paths::config_dir();
let config_yaml_file = config_dir.join(CONFIG_YAML_NAME);
// Define the labels and their corresponding path values once.
let paths = [
("Config dir:", config_dir),
("Sessions DB (sqlite):", sessions_db.display().to_string()),
("Logs dir:", logs_dir.display().to_string()),
("Config dir:", &config_dir),
("Config yaml:", &config_yaml_file),
("Sessions DB (sqlite):", &sessions_db),
("Logs dir:", &logs_dir),
];
// Calculate padding: use the max length of the label plus extra space.
let basic_padding = paths.iter().map(|(l, _)| l.len()).max().unwrap_or(0) + 4;
let label_padding = paths.iter().map(|(l, _)| l.len()).max().unwrap_or(0) + 4;
let path_padding = paths
.iter()
.map(|(_, p)| p.display().to_string().len())
.max()
.unwrap_or(0)
+ 4;
// Print version information
println!("{}", style("goose Version:").cyan().bold());
print_aligned("Version:", env!("CARGO_PKG_VERSION"), basic_padding);
print_aligned("Version:", env!("CARGO_PKG_VERSION"), label_padding);
println!();
// Print location information
println!("{}", style("goose Locations:").cyan().bold());
println!("{}", style("Paths:").cyan().bold());
for (label, path) in &paths {
print_aligned(label, path, basic_padding);
println!(
"{:<label_padding$}{:<path_padding$}{}",
label,
path.display(),
check_path_status(path)
);
}
// Print verbose info if requested
if verbose {
println!("\n{}", style("goose Configuration:").cyan().bold());
let values = config.all_values()?;
+1 -1
View File
@@ -4,7 +4,7 @@ use goose_cli::cli::cli;
#[tokio::main]
async fn main() -> Result<()> {
if let Err(e) = goose_cli::logging::setup_logging(None, None) {
eprintln!("Warning: Failed to initialize telemetry: {}", e);
eprintln!("Warning: Failed to initialize logging: {}", e);
}
let result = cli().await;
+1 -4
View File
@@ -28,13 +28,10 @@ async fn shutdown_signal() {
}
pub async fn run() -> Result<()> {
// Initialize logging and telemetry
crate::logging::setup_logging(Some("goosed"))?;
let settings = configuration::Settings::new()?;
// Initialize pricing cache on startup
tracing::info!("Initializing pricing cache...");
if let Err(e) = initialize_pricing_cache().await {
tracing::warn!(
"Failed to initialize pricing cache: {}. Pricing data may not be available.",
@@ -61,7 +58,7 @@ pub async fn run() -> Result<()> {
let listener = tokio::net::TcpListener::bind(settings.socket_addr()).await?;
info!("listening on {}", listener.local_addr()?);
// Ensure the listener/socket is properly closed on cancellation by using graceful shutdown
axum::serve(listener, app)
.with_graceful_shutdown(shutdown_signal())
.await?;
+16 -29
View File
@@ -1,4 +1,4 @@
use anyhow::{Context, Result};
use anyhow::Result;
use tracing_appender::rolling::Rotation;
use tracing_subscriber::{
filter::LevelFilter, fmt, layer::SubscriberExt, util::SubscriberInitExt, EnvFilter, Layer,
@@ -20,11 +20,8 @@ pub fn setup_logging(name: Option<&str>) -> Result<()> {
} else {
format!("{}.log", timestamp)
};
let file_appender = tracing_appender::rolling::RollingFileAppender::new(
Rotation::NEVER, // we do manual rotation via file naming and cleanup_old_logs
log_dir,
log_filename,
);
let file_appender =
tracing_appender::rolling::RollingFileAppender::new(Rotation::NEVER, log_dir, log_filename);
// Create JSON file logging layer
let file_layer = fmt::layer()
@@ -34,35 +31,27 @@ pub fn setup_logging(name: Option<&str>) -> Result<()> {
.with_ansi(false)
.with_file(true);
// Create console logging layer for development - INFO and above only
let base_env_filter = EnvFilter::try_from_default_env().unwrap_or_else(|_| {
EnvFilter::new("")
.add_directive("mcp_client=info".parse().unwrap())
.add_directive("goose=debug".parse().unwrap())
.add_directive("goose_server=info".parse().unwrap())
.add_directive("tower_http=info".parse().unwrap())
.add_directive(LevelFilter::WARN.into())
});
let console_layer = fmt::layer()
.with_writer(std::io::stderr)
.with_target(true)
.with_level(true)
.with_ansi(true)
.with_file(true)
.with_ansi(false)
.with_line_number(true)
.pretty();
// Base filter for all logging
let env_filter = EnvFilter::try_from_default_env().unwrap_or_else(|_| {
// Set default levels for different modules
EnvFilter::new("")
// Set mcp-client to DEBUG
.add_directive("mcp_client=debug".parse().unwrap())
// Set goose module to DEBUG
.add_directive("goose=debug".parse().unwrap())
// Set goose-server to INFO
.add_directive("goose_server=info".parse().unwrap())
// Set tower-http to INFO for request logging
.add_directive("tower_http=info".parse().unwrap())
// Set everything else to WARN
.add_directive(LevelFilter::WARN.into())
});
let mut layers = vec![
file_layer.with_filter(env_filter).boxed(),
console_layer.with_filter(LevelFilter::INFO).boxed(),
file_layer.with_filter(base_env_filter.clone()).boxed(),
console_layer.with_filter(base_env_filter).boxed(),
];
if let Ok((otlp_tracing_layer, otlp_metrics_layer, otlp_logs_layer)) = otlp_layer::init_otlp() {
@@ -89,9 +78,7 @@ pub fn setup_logging(name: Option<&str>) -> Result<()> {
let subscriber = Registry::default().with(layers);
subscriber
.try_init()
.context("Failed to set global subscriber")?;
subscriber.try_init()?;
Ok(())
}
+2 -4
View File
@@ -17,6 +17,7 @@ use thiserror::Error;
const KEYRING_SERVICE: &str = "goose";
const KEYRING_USERNAME: &str = "secrets";
pub const CONFIG_YAML_NAME: &str = "config.yaml";
#[cfg(test)]
const TEST_KEYRING_SERVICE: &str = "goose-test";
@@ -119,9 +120,7 @@ impl Default for Config {
fn default() -> Self {
let config_dir = Paths::config_dir();
std::fs::create_dir_all(&config_dir).expect("Failed to create config directory");
let config_path = config_dir.join("config.yaml");
let config_path = config_dir.join(CONFIG_YAML_NAME);
let secrets = match env::var("GOOSE_DISABLE_KEYRING") {
Ok(_) => SecretStorage::File {
@@ -433,7 +432,6 @@ impl Config {
// Convert to YAML for storage
let yaml_value = serde_yaml::to_string(&values)?;
// Ensure the directory exists
if let Some(parent) = self.config_path.parent() {
std::fs::create_dir_all(parent)
.map_err(|e| ConfigError::DirectoryError(e.to_string()))?;
+2 -1
View File
@@ -24,7 +24,8 @@ pub fn prepare_log_directory(component: &str, use_date_subdir: bool) -> Result<P
component_dir
};
fs::create_dir_all(&log_dir).context("Failed to create log directory")?;
fs::create_dir_all(&log_dir)
.with_context(|| format!("Failed to create log directory: {:?}", log_dir))?;
Ok(log_dir)
}
+4 -17
View File
@@ -1,4 +1,4 @@
use anyhow::Result;
use anyhow::{anyhow, Result};
use reqwest::Client;
use serde::{Deserialize, Serialize};
use std::collections::HashMap;
@@ -166,7 +166,6 @@ impl PricingCache {
self.save_to_disk(&cached_data).await?;
// Update memory cache
{
let mut cache = self.memory_cache.write().await;
*cache = Some(cached_data);
@@ -179,15 +178,6 @@ impl PricingCache {
pub async fn initialize(&self) -> Result<()> {
// Try loading from disk first
if let Ok(Some(cached)) = self.load_from_disk().await {
// Log how many models we have cached
let total_models: usize = cached.pricing.values().map(|models| models.len()).sum();
tracing::debug!(
"Loaded {} providers with {} total models from disk cache",
cached.pricing.len(),
total_models
);
// Update memory cache
{
let mut cache = self.memory_cache.write().await;
*cache = Some(cached);
@@ -196,8 +186,6 @@ impl PricingCache {
return Ok(());
}
// If no disk cache, fetch from OpenRouter
tracing::info!("Fetching pricing data from OpenRouter API");
self.refresh().await
}
}
@@ -213,14 +201,13 @@ lazy_static::lazy_static! {
static ref PRICING_CACHE: PricingCache = PricingCache::new();
}
/// Create a properly configured HTTP client for the current runtime
fn create_http_client() -> Client {
fn create_http_client() -> Result<Client> {
Client::builder()
.timeout(Duration::from_secs(30))
.pool_idle_timeout(Duration::from_secs(90))
.pool_max_idle_per_host(10)
.build()
.expect("Failed to create HTTP client")
.map_err(|e| anyhow!(e))
}
/// OpenRouter model pricing information
@@ -254,7 +241,7 @@ pub struct OpenRouterModelsResponse {
/// Internal function to fetch pricing data
async fn fetch_openrouter_pricing_internal() -> Result<HashMap<String, OpenRouterModel>> {
let client = create_http_client();
let client = create_http_client()?;
let response = client
.get("https://openrouter.ai/api/v1/models")
.send()
+2 -14
View File
@@ -190,13 +190,6 @@ impl Scheduler {
let local_tz = Local::now().timezone();
tracing::info!(
"Creating cron task for job '{}' cron: '{}' in timezone: {:?}",
job.id,
cron,
local_tz
);
Job::new_async_tz(&cron, local_tz, move |_uuid, _l| {
tracing::info!("Cron task triggered for job '{}'", job_for_task.id);
let task_job_id = job_for_task.id.clone();
@@ -215,7 +208,6 @@ impl Scheduler {
};
if !should_execute {
tracing::info!("Skipping paused job '{}'", task_job_id);
return;
}
@@ -731,15 +723,11 @@ async fn execute_job(
}
}
if let Err(e) = SessionManager::update_session(&session.id)
SessionManager::update_session(&session.id)
.schedule_id(Some(job.id.clone()))
.recipe(Some(recipe))
.apply()
.await
{
tracing::error!("Failed to update session: {}", e);
}
.await?;
Ok(session.id)
}
+4 -2
View File
@@ -19,6 +19,8 @@ use tracing::{info, warn};
use utoipa::ToSchema;
const CURRENT_SCHEMA_VERSION: i32 = 5;
pub const SESSIONS_FOLDER: &str = "sessions";
pub const DB_NAME: &str = "sessions.db";
#[derive(Debug, Clone, Copy, Serialize, Deserialize, ToSchema, PartialEq, Eq)]
#[serde(rename_all = "snake_case")]
@@ -335,7 +337,7 @@ pub struct SessionStorage {
}
pub fn ensure_session_dir() -> Result<PathBuf> {
let session_dir = Paths::data_dir().join("sessions");
let session_dir = Paths::data_dir().join(SESSIONS_FOLDER);
if !session_dir.exists() {
fs::create_dir_all(&session_dir)?;
@@ -439,7 +441,7 @@ impl sqlx::FromRow<'_, sqlx::sqlite::SqliteRow> for Session {
impl SessionStorage {
async fn new() -> Result<Self> {
let session_dir = ensure_session_dir()?;
let db_path = session_dir.join("sessions.db");
let db_path = session_dir.join(DB_NAME);
let storage = if db_path.exists() {
Self::open(&db_path).await?
-1
View File
@@ -34,7 +34,6 @@ impl Default for OtlpConfig {
impl OtlpConfig {
pub fn from_config() -> Option<Self> {
// Try to get from goose config system (which checks env vars first, then config file)
let config = crate::config::Config::global();
// Try to get the endpoint from config (checks OTEL_EXPORTER_OTLP_ENDPOINT env var first)