From 14f86691f4d992281323578cd5941d47e2eb79b1 Mon Sep 17 00:00:00 2001 From: Vim Wickramasinghe Date: Wed, 31 Dec 2025 16:45:18 +0100 Subject: [PATCH 1/8] [TOW-1275] Emit proper error message for not logged in when running cmd --- crates/tower-cmd/src/run.rs | 4 ++-- crates/tower-cmd/src/secrets.rs | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/crates/tower-cmd/src/run.rs b/crates/tower-cmd/src/run.rs index ff9f5b62..b7df39c1 100644 --- a/crates/tower-cmd/src/run.rs +++ b/crates/tower-cmd/src/run.rs @@ -467,7 +467,7 @@ async fn get_secrets(config: &Config, env: &str) -> Result Result { debug!("Failed to create secrets: {}", err); spinner.failure(); + std::process::exit(1); } } } @@ -169,11 +170,15 @@ pub async fn do_delete(config: Config, args: &ArgMatches) { let mut spinner = output::spinner("Deleting secret..."); - if let Ok(_) = api::delete_secret(&config, &name, &environment).await { - spinner.success(); - } else { - spinner.failure(); - output::die("There was a problem with the Tower API! Please try again later."); + match api::delete_secret(&config, &name, &environment).await { + Ok(_) => { + spinner.success(); + } + Err(err) => { + spinner.failure(); + output::tower_error(err); + std::process::exit(1); + } } } @@ -209,8 +214,8 @@ async fn encrypt_and_create_secret( api::create_secret(&config, name, environment, &encrypted_value, &preview).await } Err(err) => { - debug!("failed to talk to tower api: {}", err); - output::die("There was a problem with the Tower API! Please try again later."); + output::tower_error(err); + std::process::exit(1); } } } From 45138a46aed79297c484944fd4fbd959fa5841e1 Mon Sep 17 00:00:00 2001 From: Vim Wickramasinghe Date: Fri, 2 Jan 2026 12:08:51 +0100 Subject: [PATCH 2/8] Consistent spinner, error and dying --- crates/tower-cmd/src/apps.rs | 13 +++--- crates/tower-cmd/src/deploy.rs | 39 ++++++++++++---- crates/tower-cmd/src/environments.rs | 7 ++- crates/tower-cmd/src/output.rs | 21 +++++++++ crates/tower-cmd/src/run.rs | 17 +++---- crates/tower-cmd/src/schedules.rs | 11 +++-- crates/tower-cmd/src/secrets.rs | 70 +++++++++++++++++----------- 7 files changed, 120 insertions(+), 58 deletions(-) diff --git a/crates/tower-cmd/src/apps.rs b/crates/tower-cmd/src/apps.rs index c08c12ca..c1764b2b 100644 --- a/crates/tower-cmd/src/apps.rs +++ b/crates/tower-cmd/src/apps.rs @@ -133,17 +133,17 @@ pub async fn do_show(config: Config, cmd: &ArgMatches) { output::table(headers, rows, Some(&app_response)); } - Err(err) => { - output::tower_error(err); - } + Err(err) => output::tower_error_and_die(err, "Fetching app details failed"), } } pub async fn do_list_apps(config: Config) { + let mut spinner = output::spinner("Listing apps..."); let resp = api::list_apps(&config).await; match resp { Ok(resp) => { + spinner.success(); let items = resp .apps .iter() @@ -160,7 +160,8 @@ pub async fn do_list_apps(config: Config) { output::list(items, Some(&resp.apps)); } Err(err) => { - output::tower_error(err); + spinner.failure(); + output::tower_error_and_die(err, "Listing apps failed"); } } } @@ -181,7 +182,7 @@ pub async fn do_create(config: Config, args: &ArgMatches) { } Err(err) => { spinner.failure(); - output::tower_error(err); + output::tower_error_and_die(err, "Creating app failed"); } } } @@ -192,7 +193,7 @@ pub async fn do_delete(config: Config, cmd: &ArgMatches) { if let Err(err) = api::delete_app(&config, &name).await { spinner.failure(); - output::tower_error(err); + output::tower_error_and_die(err, "Deleting app failed"); } else { spinner.success(); } diff --git a/crates/tower-cmd/src/deploy.rs b/crates/tower-cmd/src/deploy.rs index b5c9b896..3549d15c 100644 --- a/crates/tower-cmd/src/deploy.rs +++ b/crates/tower-cmd/src/deploy.rs @@ -40,11 +40,21 @@ pub async fn do_deploy(config: Config, args: &ArgMatches) { let create_app = args.get_flag("create"); if let Err(err) = deploy_from_dir(config, dir, create_app).await { match err { - crate::Error::ApiDeployError { source } => output::tower_error(source), - crate::Error::ApiDescribeAppError { source } => output::tower_error(source), - crate::Error::PackageError { source } => output::package_error(source), - crate::Error::TowerfileLoadFailed { source, .. } => output::config_error(source), - _ => output::error(&err.to_string()), + crate::Error::ApiDeployError { source } => { + output::tower_error_and_die(source, "Deploying app failed") + } + crate::Error::ApiDescribeAppError { source } => { + output::tower_error_and_die(source, "Fetching app details failed") + } + crate::Error::PackageError { source } => { + output::package_error(source); + std::process::exit(1); + } + crate::Error::TowerfileLoadFailed { source, .. } => { + output::config_error(source); + std::process::exit(1); + } + _ => output::die(&err.to_string()), } } } @@ -62,13 +72,21 @@ pub async fn deploy_from_dir( let api_config = config.into(); // Add app existence check before proceeding - util::apps::ensure_app_exists( + let mut spinner = output::spinner("Checking app..."); + match util::apps::ensure_app_exists( &api_config, &towerfile.app.name, &towerfile.app.description, create_app, ) - .await?; + .await + { + Ok(_) => spinner.success(), + Err(err) => { + spinner.failure(); + return Err(crate::Error::ApiDescribeAppError { source: err }); + } + } let spec = PackageSpec::from_towerfile(&towerfile); let mut spinner = output::spinner("Building package..."); @@ -91,15 +109,20 @@ async fn do_deploy_package( package: Package, towerfile: &Towerfile, ) -> Result<(), crate::Error> { + let mut spinner = output::spinner("Deploying to Tower..."); let res = util::deploy::deploy_app_package(&api_config, &towerfile.app.name, package).await; match res { Ok(resp) => { + spinner.success(); let version = resp.version; let line = format!("Version `{}` has been deployed to Tower!", version.version); output::success(&line); Ok(()) } - Err(err) => Err(crate::Error::ApiDeployError { source: err }), + Err(err) => { + spinner.failure(); + Err(crate::Error::ApiDeployError { source: err }) + } } } diff --git a/crates/tower-cmd/src/environments.rs b/crates/tower-cmd/src/environments.rs index 33b0cbe3..3a18511b 100644 --- a/crates/tower-cmd/src/environments.rs +++ b/crates/tower-cmd/src/environments.rs @@ -24,10 +24,12 @@ pub fn environments_cmd() -> Command { } pub async fn do_list(config: Config) { + let mut spinner = output::spinner("Listing environments..."); let resp = api::list_environments(&config).await; match resp { Ok(resp) => { + spinner.success(); let headers = vec!["Name"] .into_iter() .map(|h| h.yellow().to_string()) @@ -43,7 +45,8 @@ pub async fn do_list(config: Config) { output::table(headers, envs_data, Some(&resp.environments)); } Err(err) => { - output::tower_error(err); + spinner.failure(); + output::tower_error_and_die(err, "Listing environments failed"); } } } @@ -57,7 +60,7 @@ pub async fn do_create(config: Config, args: &ArgMatches) { if let Err(err) = api::create_environment(&config, name).await { spinner.failure(); - output::tower_error(err); + output::tower_error_and_die(err, "Creating environment failed"); } else { spinner.success(); output::success(&format!("Environment '{}' created", name)); diff --git a/crates/tower-cmd/src/output.rs b/crates/tower-cmd/src/output.rs index df0cf0b4..6f1bbe21 100644 --- a/crates/tower-cmd/src/output.rs +++ b/crates/tower-cmd/src/output.rs @@ -319,6 +319,27 @@ pub fn tower_error(err: ApiError) { } } +/// Handles Tower API errors with context-specific authentication messages. +/// If the error is a 401 Unauthorized, provides a helpful message mentioning +/// the operation that failed and suggests running 'tower login'. +/// Otherwise, delegates to tower_error() for standard error handling. +/// Always exits the process with error code 1. +pub fn tower_error_and_die(err: ApiError, operation: &str) -> ! { + // Check if this is an authentication error + if let ApiError::ResponseError(ref resp) = err { + if resp.status == StatusCode::UNAUTHORIZED { + die(&format!( + "{} because you are not logged into Tower. Please run 'tower login' first.", + operation + )); + } + } + + // For other errors, use standard error handling + tower_error(err); + std::process::exit(1); +} + pub fn table(headers: Vec, data: Vec>, json_data: Option<&T>) { if get_output_mode().is_json() { if let Some(data) = json_data { diff --git a/crates/tower-cmd/src/run.rs b/crates/tower-cmd/src/run.rs index b7df39c1..245323a1 100644 --- a/crates/tower-cmd/src/run.rs +++ b/crates/tower-cmd/src/run.rs @@ -240,8 +240,7 @@ pub async fn do_run_remote( Err(err) => { spinner.failure(); debug!("Failed to schedule run: {}", err); - output::tower_error(err); - Err(Error::RunFailed) + output::tower_error_and_die(err, "Scheduling run failed"); } Ok(res) => { spinner.success(); @@ -466,10 +465,9 @@ async fn get_secrets(config: &Config, env: &str) -> Result Result("app").map(|s| s.as_str()); let environment = args.get_one::("environment").map(|s| s.as_str()); + let mut spinner = output::spinner("Listing schedules..."); match api::list_schedules(&config, app, environment).await { Ok(response) => { + spinner.success(); if response.schedules.is_empty() { output::write("No schedules found.\n"); return; @@ -137,7 +139,8 @@ pub async fn do_list(config: Config, args: &ArgMatches) { output::table(headers, rows, Some(&response.schedules)); } Err(err) => { - output::tower_error(err); + spinner.failure(); + output::tower_error_and_die(err, "Listing schedules failed"); } } } @@ -160,7 +163,7 @@ pub async fn do_create(config: Config, args: &ArgMatches) { } Err(err) => { spinner.failure(); - output::tower_error(err); + output::tower_error_and_die(err, "Creating schedule failed"); } } } @@ -178,7 +181,7 @@ pub async fn do_update(config: Config, args: &ArgMatches) { } Err(err) => { spinner.failure(); - output::tower_error(err); + output::tower_error_and_die(err, "Updating schedule failed"); } } } @@ -194,7 +197,7 @@ pub async fn do_delete(config: Config, args: &ArgMatches) { } Err(err) => { spinner.failure(); - output::tower_error(err); + output::tower_error_and_die(err, "Deleting schedule failed"); } } } diff --git a/crates/tower-cmd/src/secrets.rs b/crates/tower-cmd/src/secrets.rs index 9865548d..e2ce5303 100644 --- a/crates/tower-cmd/src/secrets.rs +++ b/crates/tower-cmd/src/secrets.rs @@ -4,10 +4,7 @@ use config::Config; use crypto::encrypt; use rsa::pkcs1::DecodeRsaPublicKey; -use tower_api::{ - apis::{default_api::CreateSecretError, Error}, - models::CreateSecretResponse, -}; +use tower_api::models::CreateSecretResponse; use tower_telemetry::debug; use crate::{api, output, util::cmd}; @@ -88,8 +85,10 @@ pub async fn do_list(config: Config, args: &ArgMatches) { if show { let (private_key, public_key) = crypto::generate_key_pair(); + let mut spinner = output::spinner("Listing secrets..."); match api::export_secrets(&config, &env, all, public_key).await { Ok(list_response) => { + spinner.success(); let headers = vec![ "Secret".bold().yellow().to_string(), "Environment".bold().yellow().to_string(), @@ -113,11 +112,16 @@ pub async fn do_list(config: Config, args: &ArgMatches) { .collect(); output::table(headers, data, Some(&list_response.secrets)); } - Err(err) => output::tower_error(err), + Err(err) => { + spinner.failure(); + output::tower_error_and_die(err, "Listing secrets failed"); + } } } else { + let mut spinner = output::spinner("Listing secrets..."); match api::list_secrets(&config, &env, all).await { Ok(list_response) => { + spinner.success(); let headers = vec![ "Secret".bold().yellow().to_string(), "Environment".bold().yellow().to_string(), @@ -136,7 +140,10 @@ pub async fn do_list(config: Config, args: &ArgMatches) { .collect(); output::table(headers, data, Some(&list_response.secrets)); } - Err(err) => output::tower_error(err), + Err(err) => { + spinner.failure(); + output::tower_error_and_die(err, "Listing secrets failed"); + } } } } @@ -157,9 +164,15 @@ pub async fn do_create(config: Config, args: &ArgMatches) { output::success(&line); } Err(err) => { - debug!("Failed to create secrets: {}", err); spinner.failure(); - std::process::exit(1); + match err { + SecretCreationError::FetchKeyFailed(e) => { + output::tower_error_and_die(e, "Fetching secrets key failed"); + } + SecretCreationError::CreateFailed(e) => { + output::tower_error_and_die(e, "Creating secret failed"); + } + } } } } @@ -176,8 +189,7 @@ pub async fn do_delete(config: Config, args: &ArgMatches) { } Err(err) => { spinner.failure(); - output::tower_error(err); - std::process::exit(1); + output::tower_error_and_die(err, "Deleting secret failed"); } } } @@ -195,29 +207,31 @@ fn create_preview(value: &str) -> String { } } +enum SecretCreationError { + FetchKeyFailed(tower_api::apis::Error), + CreateFailed(tower_api::apis::Error), +} + async fn encrypt_and_create_secret( config: &Config, name: &str, value: &str, environment: &str, -) -> Result> { - match api::describe_secrets_key(config).await { - Ok(res) => { - let public_key = - rsa::RsaPublicKey::from_pkcs1_pem(&res.public_key).unwrap_or_else(|_| { - output::die("Failed to parse public key"); - }); - - let encrypted_value = encrypt(public_key, value.to_string()).unwrap(); - let preview = create_preview(value); - - api::create_secret(&config, name, environment, &encrypted_value, &preview).await - } - Err(err) => { - output::tower_error(err); - std::process::exit(1); - } - } +) -> Result { + let res = api::describe_secrets_key(config) + .await + .map_err(SecretCreationError::FetchKeyFailed)?; + + let public_key = rsa::RsaPublicKey::from_pkcs1_pem(&res.public_key).unwrap_or_else(|_| { + output::die("Failed to parse public key"); + }); + + let encrypted_value = encrypt(public_key, value.to_string()).unwrap(); + let preview = create_preview(value); + + api::create_secret(&config, name, environment, &encrypted_value, &preview) + .await + .map_err(SecretCreationError::CreateFailed) } fn extract_secret_environment_and_name( From f34db06647a54b637d5b853223dbb5c29781edae Mon Sep 17 00:00:00 2001 From: Vim Wickramasinghe Date: Fri, 2 Jan 2026 12:47:41 +0100 Subject: [PATCH 3/8] Refactor to use spinner and error handling easily/correctly --- crates/tower-cmd/src/apps.rs | 78 +++++++------- crates/tower-cmd/src/deploy.rs | 16 +-- crates/tower-cmd/src/environments.rs | 60 +++++------ crates/tower-cmd/src/output.rs | 33 ++++++ crates/tower-cmd/src/run.rs | 44 ++++---- crates/tower-cmd/src/schedules.rs | 145 ++++++++++++--------------- crates/tower-cmd/src/secrets.rs | 128 +++++++++++------------ crates/tower-cmd/src/teams.rs | 36 +++---- crates/tower-cmd/src/util/apps.rs | 15 ++- 9 files changed, 266 insertions(+), 289 deletions(-) diff --git a/crates/tower-cmd/src/apps.rs b/crates/tower-cmd/src/apps.rs index c1764b2b..40003ef9 100644 --- a/crates/tower-cmd/src/apps.rs +++ b/crates/tower-cmd/src/apps.rs @@ -138,32 +138,27 @@ pub async fn do_show(config: Config, cmd: &ArgMatches) { } pub async fn do_list_apps(config: Config) { - let mut spinner = output::spinner("Listing apps..."); - let resp = api::list_apps(&config).await; - - match resp { - Ok(resp) => { - spinner.success(); - let items = resp - .apps - .iter() - .map(|app_summary| { - let app = &app_summary.app; - let desc = if app.short_description.is_empty() { - output::placeholder("No description") - } else { - app.short_description.to_string() - }; - format!("{}\n{}", output::title(&app.name), desc) - }) - .collect(); - output::list(items, Some(&resp.apps)); - } - Err(err) => { - spinner.failure(); - output::tower_error_and_die(err, "Listing apps failed"); - } - } + let resp = output::with_spinner( + "Listing apps...", + "Listing apps failed", + api::list_apps(&config), + ) + .await; + + let items = resp + .apps + .iter() + .map(|app_summary| { + let app = &app_summary.app; + let desc = if app.short_description.is_empty() { + output::placeholder("No description") + } else { + app.short_description.to_string() + }; + format!("{}\n{}", output::title(&app.name), desc) + }) + .collect(); + output::list(items, Some(&resp.apps)); } pub async fn do_create(config: Config, args: &ArgMatches) { @@ -173,30 +168,25 @@ pub async fn do_create(config: Config, args: &ArgMatches) { let description = args.get_one::("description").unwrap(); - let mut spinner = output::spinner("Creating app"); + let app = output::with_spinner( + "Creating app", + "Creating app failed", + api::create_app(&config, name, description), + ) + .await; - match api::create_app(&config, name, description).await { - Ok(app) => { - spinner.success(); - output::success_with_data(&format!("App '{}' created", name), Some(app)); - } - Err(err) => { - spinner.failure(); - output::tower_error_and_die(err, "Creating app failed"); - } - } + output::success_with_data(&format!("App '{}' created", name), Some(app)); } pub async fn do_delete(config: Config, cmd: &ArgMatches) { let name = extract_app_name("delete", cmd.subcommand()); - let mut spinner = output::spinner("Deleting app"); - if let Err(err) = api::delete_app(&config, &name).await { - spinner.failure(); - output::tower_error_and_die(err, "Deleting app failed"); - } else { - spinner.success(); - } + output::with_spinner( + "Deleting app", + "Deleting app failed", + api::delete_app(&config, &name), + ) + .await; } /// Extract app name and run number from command diff --git a/crates/tower-cmd/src/deploy.rs b/crates/tower-cmd/src/deploy.rs index 3549d15c..cf3edcb7 100644 --- a/crates/tower-cmd/src/deploy.rs +++ b/crates/tower-cmd/src/deploy.rs @@ -72,8 +72,7 @@ pub async fn deploy_from_dir( let api_config = config.into(); // Add app existence check before proceeding - let mut spinner = output::spinner("Checking app..."); - match util::apps::ensure_app_exists( + if let Err(err) = util::apps::ensure_app_exists( &api_config, &towerfile.app.name, &towerfile.app.description, @@ -81,11 +80,7 @@ pub async fn deploy_from_dir( ) .await { - Ok(_) => spinner.success(), - Err(err) => { - spinner.failure(); - return Err(crate::Error::ApiDescribeAppError { source: err }); - } + return Err(crate::Error::ApiDescribeAppError { source: err }); } let spec = PackageSpec::from_towerfile(&towerfile); @@ -109,20 +104,15 @@ async fn do_deploy_package( package: Package, towerfile: &Towerfile, ) -> Result<(), crate::Error> { - let mut spinner = output::spinner("Deploying to Tower..."); let res = util::deploy::deploy_app_package(&api_config, &towerfile.app.name, package).await; match res { Ok(resp) => { - spinner.success(); let version = resp.version; let line = format!("Version `{}` has been deployed to Tower!", version.version); output::success(&line); Ok(()) } - Err(err) => { - spinner.failure(); - Err(crate::Error::ApiDeployError { source: err }) - } + Err(err) => Err(crate::Error::ApiDeployError { source: err }), } } diff --git a/crates/tower-cmd/src/environments.rs b/crates/tower-cmd/src/environments.rs index 3a18511b..a003dd35 100644 --- a/crates/tower-cmd/src/environments.rs +++ b/crates/tower-cmd/src/environments.rs @@ -24,31 +24,26 @@ pub fn environments_cmd() -> Command { } pub async fn do_list(config: Config) { - let mut spinner = output::spinner("Listing environments..."); - let resp = api::list_environments(&config).await; - - match resp { - Ok(resp) => { - spinner.success(); - let headers = vec!["Name"] - .into_iter() - .map(|h| h.yellow().to_string()) - .collect(); - - let envs_data: Vec> = resp - .environments - .iter() - .map(|env| vec![env.name.clone()]) - .collect(); - - // Display the table using the existing table function - output::table(headers, envs_data, Some(&resp.environments)); - } - Err(err) => { - spinner.failure(); - output::tower_error_and_die(err, "Listing environments failed"); - } - } + let resp = output::with_spinner( + "Listing environments...", + "Listing environments failed", + api::list_environments(&config), + ) + .await; + + let headers = vec!["Name"] + .into_iter() + .map(|h| h.yellow().to_string()) + .collect(); + + let envs_data: Vec> = resp + .environments + .iter() + .map(|env| vec![env.name.clone()]) + .collect(); + + // Display the table using the existing table function + output::table(headers, envs_data, Some(&resp.environments)); } pub async fn do_create(config: Config, args: &ArgMatches) { @@ -56,13 +51,12 @@ pub async fn do_create(config: Config, args: &ArgMatches) { output::die("Environment name (--name) is required"); }); - let mut spinner = output::spinner("Creating environment"); + output::with_spinner( + "Creating environment", + "Creating environment failed", + api::create_environment(&config, name), + ) + .await; - if let Err(err) = api::create_environment(&config, name).await { - spinner.failure(); - output::tower_error_and_die(err, "Creating environment failed"); - } else { - spinner.success(); - output::success(&format!("Environment '{}' created", name)); - } + output::success(&format!("Environment '{}' created", name)); } diff --git a/crates/tower-cmd/src/output.rs b/crates/tower-cmd/src/output.rs index 6f1bbe21..d651c8b6 100644 --- a/crates/tower-cmd/src/output.rs +++ b/crates/tower-cmd/src/output.rs @@ -340,6 +340,39 @@ pub fn tower_error_and_die(err: ApiError, operation: &str) -> ! { std::process::exit(1); } +/// Runs an async operation with a spinner and proper error handling. +/// +/// This helper provides consistent spinner behavior across all commands: +/// - Shows a spinner with the given message while the operation runs +/// - On success: stops the spinner with success indicator and returns the result +/// - On error: stops the spinner with failure indicator and shows auth-aware error message +/// +/// # Examples +/// +/// ```rust +/// let envs = output::with_spinner( +/// "Listing environments...", +/// "Listing environments failed", +/// api::list_environments(&config) +/// ).await; +/// ``` +pub async fn with_spinner(spinner_msg: &str, error_operation: &str, future: F) -> T +where + F: std::future::Future>>, +{ + let mut spinner = self::spinner(spinner_msg); + match future.await { + Ok(result) => { + spinner.success(); + result + } + Err(err) => { + spinner.failure(); + tower_error_and_die(err, error_operation); + } + } +} + pub fn table(headers: Vec, data: Vec>, json_data: Option<&T>) { if get_output_mode().is_json() { if let Some(data) = json_data { diff --git a/crates/tower-cmd/src/run.rs b/crates/tower-cmd/src/run.rs index 245323a1..15f5bb2a 100644 --- a/crates/tower-cmd/src/run.rs +++ b/crates/tower-cmd/src/run.rs @@ -234,35 +234,29 @@ pub async fn do_run_remote( towerfile.app.name }; - let mut spinner = output::spinner("Scheduling run..."); + let res = output::with_spinner( + "Scheduling run...", + "Scheduling run failed", + api::run_app(&config, &app_slug, env, params), + ) + .await; - match api::run_app(&config, &app_slug, env, params).await { - Err(err) => { - spinner.failure(); - debug!("Failed to schedule run: {}", err); - output::tower_error_and_die(err, "Scheduling run failed"); - } - Ok(res) => { - spinner.success(); + let run = res.run; - let run = res.run; + if should_follow_run { + do_follow_run(config, &run).await?; + } else { + let line = format!( + "Run #{} for app `{}` has been scheduled", + run.number, app_slug + ); + output::success(&line); - if should_follow_run { - do_follow_run(config, &run).await?; - } else { - let line = format!( - "Run #{} for app `{}` has been scheduled", - run.number, app_slug - ); - output::success(&line); - - let link_line = format!(" See more: {}", run.dollar_link); - output::write(&link_line); - output::newline(); - } - Ok(()) - } + let link_line = format!(" See more: {}", run.dollar_link); + output::write(&link_line); + output::newline(); } + Ok(()) } async fn stream_logs_until_complete( diff --git a/crates/tower-cmd/src/schedules.rs b/crates/tower-cmd/src/schedules.rs index b7c95770..d9a84afa 100644 --- a/crates/tower-cmd/src/schedules.rs +++ b/crates/tower-cmd/src/schedules.rs @@ -100,49 +100,46 @@ pub async fn do_list(config: Config, args: &ArgMatches) { let app = args.get_one::("app").map(|s| s.as_str()); let environment = args.get_one::("environment").map(|s| s.as_str()); - let mut spinner = output::spinner("Listing schedules..."); - match api::list_schedules(&config, app, environment).await { - Ok(response) => { - spinner.success(); - if response.schedules.is_empty() { - output::write("No schedules found.\n"); - return; - } - - let headers = vec![ - "ID".yellow().to_string(), - "App".yellow().to_string(), - "Environment".yellow().to_string(), - "Cron".yellow().to_string(), - "Status".yellow().to_string(), - ]; - - let rows: Vec> = response - .schedules - .iter() - .map(|schedule| { - let status = match schedule.status { - Status::Active => "active".green(), - Status::Disabled => "disabled".red(), - }; - - vec![ - schedule.id.clone(), - schedule.app_name.clone(), - schedule.environment.clone(), - schedule.cron.clone(), - status.to_string(), - ] - }) - .collect(); - - output::table(headers, rows, Some(&response.schedules)); - } - Err(err) => { - spinner.failure(); - output::tower_error_and_die(err, "Listing schedules failed"); - } + let response = output::with_spinner( + "Listing schedules...", + "Listing schedules failed", + api::list_schedules(&config, app, environment), + ) + .await; + + if response.schedules.is_empty() { + output::write("No schedules found.\n"); + return; } + + let headers = vec![ + "ID".yellow().to_string(), + "App".yellow().to_string(), + "Environment".yellow().to_string(), + "Cron".yellow().to_string(), + "Status".yellow().to_string(), + ]; + + let rows: Vec> = response + .schedules + .iter() + .map(|schedule| { + let status = match schedule.status { + Status::Active => "active".green(), + Status::Disabled => "disabled".red(), + }; + + vec![ + schedule.id.clone(), + schedule.app_name.clone(), + schedule.environment.clone(), + schedule.cron.clone(), + status.to_string(), + ] + }) + .collect(); + + output::table(headers, rows, Some(&response.schedules)); } pub async fn do_create(config: Config, args: &ArgMatches) { @@ -151,55 +148,45 @@ pub async fn do_create(config: Config, args: &ArgMatches) { let cron = args.get_one::("cron").unwrap(); let parameters = parse_parameters(args); - let mut spinner = output::spinner("Creating schedule"); - - match api::create_schedule(&config, app_name, environment, cron, parameters).await { - Ok(response) => { - spinner.success(); - output::success(&format!( - "Schedule created with ID: {}", - response.schedule.id - )); - } - Err(err) => { - spinner.failure(); - output::tower_error_and_die(err, "Creating schedule failed"); - } - } + let response = output::with_spinner( + "Creating schedule", + "Creating schedule failed", + api::create_schedule(&config, app_name, environment, cron, parameters), + ) + .await; + + output::success(&format!( + "Schedule created with ID: {}", + response.schedule.id + )); } pub async fn do_update(config: Config, args: &ArgMatches) { let schedule_id = extract_schedule_id("update", args.subcommand()); let cron = args.get_one::("cron"); let parameters = parse_parameters(args); - let mut spinner = output::spinner("Updating schedule"); - match api::update_schedule(&config, &schedule_id, cron, parameters).await { - Ok(_) => { - spinner.success(); - output::success(&format!("Schedule {} updated", schedule_id)); - } - Err(err) => { - spinner.failure(); - output::tower_error_and_die(err, "Updating schedule failed"); - } - } + output::with_spinner( + "Updating schedule", + "Updating schedule failed", + api::update_schedule(&config, &schedule_id, cron, parameters), + ) + .await; + + output::success(&format!("Schedule {} updated", schedule_id)); } pub async fn do_delete(config: Config, args: &ArgMatches) { let schedule_id = extract_schedule_id("delete", args.subcommand()); - let mut spinner = output::spinner("Deleting schedule"); - match api::delete_schedule(&config, &schedule_id).await { - Ok(_) => { - spinner.success(); - output::success(&format!("Schedule {} deleted", schedule_id)); - } - Err(err) => { - spinner.failure(); - output::tower_error_and_die(err, "Deleting schedule failed"); - } - } + output::with_spinner( + "Deleting schedule", + "Deleting schedule failed", + api::delete_schedule(&config, &schedule_id), + ) + .await; + + output::success(&format!("Schedule {} deleted", schedule_id)); } fn extract_schedule_id(subcmd: &str, cmd: Option<(&str, &ArgMatches)>) -> String { diff --git a/crates/tower-cmd/src/secrets.rs b/crates/tower-cmd/src/secrets.rs index e2ce5303..1bf34811 100644 --- a/crates/tower-cmd/src/secrets.rs +++ b/crates/tower-cmd/src/secrets.rs @@ -85,66 +85,59 @@ pub async fn do_list(config: Config, args: &ArgMatches) { if show { let (private_key, public_key) = crypto::generate_key_pair(); - let mut spinner = output::spinner("Listing secrets..."); - match api::export_secrets(&config, &env, all, public_key).await { - Ok(list_response) => { - spinner.success(); - let headers = vec![ - "Secret".bold().yellow().to_string(), - "Environment".bold().yellow().to_string(), - "Value".bold().yellow().to_string(), - ]; - let data = list_response - .secrets - .iter() - .map(|secret| { - // now we decrypt the value and show it. - let decrypted_value = - crypto::decrypt(private_key.clone(), secret.encrypted_value.clone()) - .unwrap(); - - vec![ - secret.name.clone(), - secret.environment.clone(), - decrypted_value, - ] - }) - .collect(); - output::table(headers, data, Some(&list_response.secrets)); - } - Err(err) => { - spinner.failure(); - output::tower_error_and_die(err, "Listing secrets failed"); - } - } + let list_response = output::with_spinner( + "Listing secrets...", + "Listing secrets failed", + api::export_secrets(&config, &env, all, public_key), + ) + .await; + + let headers = vec![ + "Secret".bold().yellow().to_string(), + "Environment".bold().yellow().to_string(), + "Value".bold().yellow().to_string(), + ]; + let data = list_response + .secrets + .iter() + .map(|secret| { + // now we decrypt the value and show it. + let decrypted_value = + crypto::decrypt(private_key.clone(), secret.encrypted_value.clone()).unwrap(); + + vec![ + secret.name.clone(), + secret.environment.clone(), + decrypted_value, + ] + }) + .collect(); + output::table(headers, data, Some(&list_response.secrets)); } else { - let mut spinner = output::spinner("Listing secrets..."); - match api::list_secrets(&config, &env, all).await { - Ok(list_response) => { - spinner.success(); - let headers = vec![ - "Secret".bold().yellow().to_string(), - "Environment".bold().yellow().to_string(), - "Preview".bold().yellow().to_string(), - ]; - let data = list_response - .secrets - .iter() - .map(|secret| { - vec![ - secret.name.clone(), - secret.environment.clone(), - secret.preview.dimmed().to_string(), - ] - }) - .collect(); - output::table(headers, data, Some(&list_response.secrets)); - } - Err(err) => { - spinner.failure(); - output::tower_error_and_die(err, "Listing secrets failed"); - } - } + let list_response = output::with_spinner( + "Listing secrets...", + "Listing secrets failed", + api::list_secrets(&config, &env, all), + ) + .await; + + let headers = vec![ + "Secret".bold().yellow().to_string(), + "Environment".bold().yellow().to_string(), + "Preview".bold().yellow().to_string(), + ]; + let data = list_response + .secrets + .iter() + .map(|secret| { + vec![ + secret.name.clone(), + secret.environment.clone(), + secret.preview.dimmed().to_string(), + ] + }) + .collect(); + output::table(headers, data, Some(&list_response.secrets)); } } @@ -181,17 +174,12 @@ pub async fn do_delete(config: Config, args: &ArgMatches) { let (environment, name) = extract_secret_environment_and_name("delete", args.subcommand()); debug!("deleting secret, environment={} name={}", environment, name); - let mut spinner = output::spinner("Deleting secret..."); - - match api::delete_secret(&config, &name, &environment).await { - Ok(_) => { - spinner.success(); - } - Err(err) => { - spinner.failure(); - output::tower_error_and_die(err, "Deleting secret failed"); - } - } + output::with_spinner( + "Deleting secret...", + "Deleting secret failed", + api::delete_secret(&config, &name, &environment), + ) + .await; } fn create_preview(value: &str) -> String { diff --git a/crates/tower-cmd/src/teams.rs b/crates/tower-cmd/src/teams.rs index cc7abfe4..84771968 100644 --- a/crates/tower-cmd/src/teams.rs +++ b/crates/tower-cmd/src/teams.rs @@ -1,7 +1,6 @@ use clap::{ArgMatches, Command}; use colored::*; use config::Config; -use tower_telemetry::debug; use crate::{api, output}; @@ -28,30 +27,23 @@ async fn refresh_session(config: &Config) -> config::Session { } }; - let mut spinner = output::spinner("Refreshing session..."); + let resp = output::with_spinner( + "Refreshing session...", + "Refreshing session failed", + api::refresh_session(&config), + ) + .await; - match api::refresh_session(&config).await { - Ok(resp) => { - spinner.success(); + // Create a mutable copy of the session to update + let mut session = current_session; - // Create a mutable copy of the session to update - let mut session = current_session; - - // Update it with the API response - if let Err(e) = session.update_from_api_response(&resp) { - output::config_error(e); - std::process::exit(1); - } - - session - } - Err(err) => { - debug!("Failed to refresh session: {}", err); - - spinner.failure(); - output::die("There was a problem talking to the Tower API. Try again later!"); - } + // Update it with the API response + if let Err(e) = session.update_from_api_response(&resp) { + output::config_error(e); + std::process::exit(1); } + + session } pub async fn do_list(config: Config) { diff --git a/crates/tower-cmd/src/util/apps.rs b/crates/tower-cmd/src/util/apps.rs index 69b57aed..949785d1 100644 --- a/crates/tower-cmd/src/util/apps.rs +++ b/crates/tower-cmd/src/util/apps.rs @@ -13,7 +13,8 @@ pub async fn ensure_app_exists( description: &str, create_app: bool, ) -> Result<(), tower_api::apis::Error> { - // Try to describe the app first + // Try to describe the app first (with spinner) + let mut spinner = output::spinner("Checking app..."); let describe_result = default_api::describe_app( api_config, DescribeAppParams { @@ -28,6 +29,7 @@ pub async fn ensure_app_exists( // If the app exists, return Ok if describe_result.is_ok() { + spinner.success(); return Ok(()); } @@ -44,11 +46,15 @@ pub async fn ensure_app_exists( _ => false, }; - // If it's not a 404 error, return the original error + // If it's not a 404 error, fail the spinner and return the error if !is_not_found { + spinner.failure(); return Err(err); } + // App not found - stop spinner before prompting user + drop(spinner); + // Decide whether to create the app let create_app = create_app || prompt_default( @@ -65,7 +71,8 @@ pub async fn ensure_app_exists( return Err(err); } - // Try to create the app + // Try to create the app (with a new spinner) + let mut spinner = output::spinner("Creating app..."); let create_result = default_api::create_app( api_config, CreateAppParams { @@ -83,10 +90,12 @@ pub async fn ensure_app_exists( match create_result { Ok(_) => { + spinner.success(); output::success(&format!("Created app '{}'", app_name)); Ok(()) } Err(create_err) => { + spinner.failure(); // Convert any creation error to a response error Err(tower_api::apis::Error::ResponseError( tower_api::apis::ResponseContent { From a9017fb0e60d0e8bed27f903324be54deb904ff2 Mon Sep 17 00:00:00 2001 From: Vim Wickramasinghe Date: Fri, 2 Jan 2026 12:58:23 +0100 Subject: [PATCH 4/8] Fix tests --- crates/tower-cmd/src/output.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tower-cmd/src/output.rs b/crates/tower-cmd/src/output.rs index d651c8b6..595b3e04 100644 --- a/crates/tower-cmd/src/output.rs +++ b/crates/tower-cmd/src/output.rs @@ -349,7 +349,7 @@ pub fn tower_error_and_die(err: ApiError, operation: &str) -> ! { /// /// # Examples /// -/// ```rust +/// ```ignore /// let envs = output::with_spinner( /// "Listing environments...", /// "Listing environments failed", From 78ff729ee598eb4e05166d4425bff9de03f2e2cd Mon Sep 17 00:00:00 2001 From: Vim Wickramasinghe Date: Fri, 2 Jan 2026 19:05:14 +0100 Subject: [PATCH 5/8] Fixes for integration tests 1 --- crates/tower-cmd/src/mcp.rs | 19 +++---- crates/tower-cmd/src/output.rs | 39 ++++++++++++-- crates/tower-cmd/src/run.rs | 95 ++++++++++++++++++++++++++-------- 3 files changed, 116 insertions(+), 37 deletions(-) diff --git a/crates/tower-cmd/src/mcp.rs b/crates/tower-cmd/src/mcp.rs index afa88a41..a7a5974d 100644 --- a/crates/tower-cmd/src/mcp.rs +++ b/crates/tower-cmd/src/mcp.rs @@ -687,18 +687,15 @@ impl TowerService { } } Err(e) => { - let error_text = if output.trim().is_empty() { - let api_error = Self::extract_api_error_message(&e); - if Self::is_deployment_error(&api_error) { - format!( - "App '{}' not deployed. Try running tower_deploy first.", - app_name - ) - } else { - api_error - } + // Always extract the detailed API error message + let api_error = Self::extract_api_error_message(&e); + let error_text = if Self::is_deployment_error(&api_error) { + format!( + "App '{}' not deployed. Try running tower_deploy first.", + app_name + ) } else { - output + api_error }; Self::error_result("Remote run failed", error_text) } diff --git a/crates/tower-cmd/src/output.rs b/crates/tower-cmd/src/output.rs index 595b3e04..bbe8be2e 100644 --- a/crates/tower-cmd/src/output.rs +++ b/crates/tower-cmd/src/output.rs @@ -211,6 +211,7 @@ pub fn write(msg: &str) { send_to_current_sender(clean_msg); } else { io::stdout().write_all(msg.as_bytes()).unwrap(); + io::stdout().flush().ok(); } } @@ -322,7 +323,6 @@ pub fn tower_error(err: ApiError) { /// Handles Tower API errors with context-specific authentication messages. /// If the error is a 401 Unauthorized, provides a helpful message mentioning /// the operation that failed and suggests running 'tower login'. -/// Otherwise, delegates to tower_error() for standard error handling. /// Always exits the process with error code 1. pub fn tower_error_and_die(err: ApiError, operation: &str) -> ! { // Check if this is an authentication error @@ -335,9 +335,9 @@ pub fn tower_error_and_die(err: ApiError, operation: &str) -> ! { } } - // For other errors, use standard error handling + // Show the detailed error first tower_error(err); - std::process::exit(1); + die(operation); } /// Runs an async operation with a spinner and proper error handling. @@ -373,6 +373,34 @@ where } } +/// Runs an async operation with a spinner, returning Result instead of exiting. +/// +/// This is the MCP-safe version of with_spinner that returns errors instead of exiting. +/// Use this for operations that may be called from MCP or other contexts where +/// process exit is not acceptable. Returns the error without displaying it, allowing +/// the caller to decide how to handle and display the error. +pub async fn try_with_spinner( + spinner_msg: &str, + _error_operation: &str, + future: F, +) -> Result> +where + F: std::future::Future>>, +{ + let mut spinner = self::spinner(spinner_msg); + match future.await { + Ok(result) => { + spinner.success(); + Ok(result) + } + Err(err) => { + spinner.failure(); + // Just return the error - let the caller decide how to handle it + Err(err) + } + } +} + pub fn table(headers: Vec, data: Vec>, json_data: Option<&T>) { if get_output_mode().is_json() { if let Some(data) = json_data { @@ -496,8 +524,13 @@ pub fn newline() { } pub fn die(msg: &str) -> ! { + io::stdout().flush().ok(); + io::stderr().flush().ok(); let line = format!("{} {}\n", "Error:".red(), msg); write(&line); + // Flush output before exit to ensure "Error:" message is displayed + io::stdout().flush().ok(); + io::stderr().flush().ok(); std::process::exit(1); } diff --git a/crates/tower-cmd/src/run.rs b/crates/tower-cmd/src/run.rs index 15f5bb2a..8908c4b1 100644 --- a/crates/tower-cmd/src/run.rs +++ b/crates/tower-cmd/src/run.rs @@ -60,7 +60,14 @@ pub fn run_cmd() -> Command { pub async fn do_run(config: Config, args: &ArgMatches, cmd: Option<(&str, &ArgMatches)>) { if let Err(e) = do_run_inner(config, args, cmd).await { - output::die(&e.to_string()); + match e { + Error::ApiRunError { source } => { + output::tower_error_and_die(source, "Scheduling run failed"); + } + _ => { + output::die(&e.to_string()); + } + } } } @@ -114,13 +121,30 @@ where Fut: std::future::Future + Send + 'static, T: Send + 'static, { + // Load all the secrets and catalogs from the server let mut spinner = output::spinner("Setting up runtime environment..."); - // Load all the secrets and catalogs from the server - let secrets = get_secrets(&config, &env).await?; - let catalogs = get_catalogs(&config, &env).await?; + let secrets = match get_secrets(&config, &env).await { + Ok(s) => { + spinner.success(); + s + } + Err(err) => { + spinner.failure(); + output::tower_error_and_die(err, "Fetching secrets failed"); + } + }; - spinner.success(); + let catalogs = match get_catalogs(&config, &env).await { + Ok(s) => { + spinner.success(); + s + } + Err(err) => { + spinner.failure(); + output::tower_error_and_die(err, "Fetching secrets failed"); + } + }; // We prepare all the other misc environment variables that we need to inject let mut env_vars = HashMap::new(); @@ -234,12 +258,13 @@ pub async fn do_run_remote( towerfile.app.name }; - let res = output::with_spinner( + let res = output::try_with_spinner( "Scheduling run...", "Scheduling run failed", api::run_app(&config, &app_slug, env, params), ) - .await; + .await + .map_err(|source| Error::ApiRunError { source })?; let run = res.run; @@ -452,45 +477,69 @@ fn get_app_name(cmd: Option<(&str, &ArgMatches)>) -> Option { } } -/// get_secrets manages the process of getting secrets from the Tower server in a way that can be -/// used by the local runtime during local app execution. -async fn get_secrets(config: &Config, env: &str) -> Result, Error> { +/// get_secrets_inner manages the process of getting secrets from the Tower server in a way that can be +/// used by the local runtime during local app execution. Returns API errors for spinner handling. +async fn get_secrets( + config: &Config, + env: &str, +) -> Result< + HashMap, + tower_api::apis::Error, +> { let (private_key, public_key) = crypto::generate_key_pair(); - let res = api::export_secrets(&config, env, false, public_key) - .await - .unwrap_or_else(|err| { - output::tower_error_and_die(err, "Fetching secrets failed"); - }); + let res = api::export_secrets(&config, env, false, public_key).await?; let mut secrets = HashMap::new(); for secret in res.secrets { // we will decrypt each property and inject it into the vals map. let decrypted_value = - crypto::decrypt(private_key.clone(), secret.encrypted_value.to_string())?; + crypto::decrypt(private_key.clone(), secret.encrypted_value.to_string()).map_err( + |_| { + tower_api::apis::Error::Io(std::io::Error::new( + std::io::ErrorKind::Other, + "Failed to decrypt secret", + )) + }, + )?; secrets.insert(secret.name, decrypted_value); } Ok(secrets) } -/// get_catalogs manages the process of exporting catalogs, decrypting their properties, and -/// preparting them for injection into the environment during app execution -async fn get_catalogs(config: &Config, env: &str) -> Result, Error> { +/// get_catalogs_inner manages the process of exporting catalogs, decrypting their properties, and +/// preparting them for injection into the environment during app execution. Returns plain Error for direct use. +async fn get_catalogs( + config: &Config, + env: &str, +) -> Result< + HashMap, + tower_api::apis::Error, +> { let (private_key, public_key) = crypto::generate_key_pair(); let res = api::export_catalogs(&config, env, false, public_key) .await - .unwrap_or_else(|err| { - output::tower_error_and_die(err, "Fetching catalogs failed"); - }); + .map_err(|_| { + tower_api::apis::Error::Io(std::io::Error::new( + std::io::ErrorKind::Other, + "Failed to export catalogs", + )) + })?; let mut vals = HashMap::new(); for catalog in res.catalogs { // we will decrypt each property and inject it into the vals map. for property in catalog.properties { let decrypted_value = - crypto::decrypt(private_key.clone(), property.encrypted_value.to_string())?; + crypto::decrypt(private_key.clone(), property.encrypted_value.to_string()) + .map_err(|_| { + tower_api::apis::Error::Io(std::io::Error::new( + std::io::ErrorKind::Other, + "Failed to decrypt catalog property", + )) + })?; let name = create_pyiceberg_catalog_property_name(&catalog.name, &property.name); vals.insert(name, decrypted_value); } From d687999098b71dc486c9dd323d6abaf5ee65df1b Mon Sep 17 00:00:00 2001 From: Vim Wickramasinghe Date: Fri, 2 Jan 2026 19:10:13 +0100 Subject: [PATCH 6/8] minor --- crates/tower-cmd/src/run.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/crates/tower-cmd/src/run.rs b/crates/tower-cmd/src/run.rs index 8908c4b1..369c2551 100644 --- a/crates/tower-cmd/src/run.rs +++ b/crates/tower-cmd/src/run.rs @@ -125,10 +125,7 @@ where let mut spinner = output::spinner("Setting up runtime environment..."); let secrets = match get_secrets(&config, &env).await { - Ok(s) => { - spinner.success(); - s - } + Ok(s) => s, Err(err) => { spinner.failure(); output::tower_error_and_die(err, "Fetching secrets failed"); @@ -136,16 +133,15 @@ where }; let catalogs = match get_catalogs(&config, &env).await { - Ok(s) => { - spinner.success(); - s - } + Ok(c) => c, Err(err) => { spinner.failure(); - output::tower_error_and_die(err, "Fetching secrets failed"); + output::tower_error_and_die(err, "Fetching catalogs failed"); } }; + spinner.success(); + // We prepare all the other misc environment variables that we need to inject let mut env_vars = HashMap::new(); env_vars.extend(catalogs); From 3b35ff724d8b9eb2ac21eaa828bbb5f1cd2979f6 Mon Sep 17 00:00:00 2001 From: Vim Wickramasinghe Date: Fri, 2 Jan 2026 19:52:58 +0100 Subject: [PATCH 7/8] Fix to use current year in integration test --- tests/integration/features/steps/cli_steps.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/features/steps/cli_steps.py b/tests/integration/features/steps/cli_steps.py index eee67012..da8ef117 100644 --- a/tests/integration/features/steps/cli_steps.py +++ b/tests/integration/features/steps/cli_steps.py @@ -6,6 +6,7 @@ import shutil import json import shlex +from datetime import datetime from pathlib import Path from behave import given, when, then from dirty_equals import IsStr, IsPartialDict @@ -84,10 +85,11 @@ def step_log_lines_should_be_separate(context): assert len(lines) > 3, f"Expected multiple lines of output, got: {len(lines)} lines" # Lines with timestamps should not be concatenated + current_year = str(datetime.now().year) timestamp_lines = [ line for line in lines - if "2025-" in line and ("Hello" in line or "Creating" in line) + if current_year in line and ("Hello" in line or "Creating" in line) ] assert ( len(timestamp_lines) > 1 From ff9e545b105007959d93121ee35cf6041f0dd46a Mon Sep 17 00:00:00 2001 From: Vim Wickramasinghe Date: Fri, 2 Jan 2026 20:10:24 +0100 Subject: [PATCH 8/8] review --- crates/tower-cmd/src/apps.rs | 22 ++++------------------ crates/tower-cmd/src/environments.rs | 8 +------- crates/tower-cmd/src/output.rs | 24 ++++++++++++------------ crates/tower-cmd/src/run.rs | 3 +-- crates/tower-cmd/src/schedules.rs | 6 +----- crates/tower-cmd/src/secrets.rs | 14 ++++---------- crates/tower-cmd/src/teams.rs | 7 +------ 7 files changed, 24 insertions(+), 60 deletions(-) diff --git a/crates/tower-cmd/src/apps.rs b/crates/tower-cmd/src/apps.rs index 40003ef9..668d5f79 100644 --- a/crates/tower-cmd/src/apps.rs +++ b/crates/tower-cmd/src/apps.rs @@ -138,12 +138,7 @@ pub async fn do_show(config: Config, cmd: &ArgMatches) { } pub async fn do_list_apps(config: Config) { - let resp = output::with_spinner( - "Listing apps...", - "Listing apps failed", - api::list_apps(&config), - ) - .await; + let resp = output::with_spinner("Listing apps", api::list_apps(&config)).await; let items = resp .apps @@ -168,12 +163,8 @@ pub async fn do_create(config: Config, args: &ArgMatches) { let description = args.get_one::("description").unwrap(); - let app = output::with_spinner( - "Creating app", - "Creating app failed", - api::create_app(&config, name, description), - ) - .await; + let app = + output::with_spinner("Creating app", api::create_app(&config, name, description)).await; output::success_with_data(&format!("App '{}' created", name), Some(app)); } @@ -181,12 +172,7 @@ pub async fn do_create(config: Config, args: &ArgMatches) { pub async fn do_delete(config: Config, cmd: &ArgMatches) { let name = extract_app_name("delete", cmd.subcommand()); - output::with_spinner( - "Deleting app", - "Deleting app failed", - api::delete_app(&config, &name), - ) - .await; + output::with_spinner("Deleting app", api::delete_app(&config, &name)).await; } /// Extract app name and run number from command diff --git a/crates/tower-cmd/src/environments.rs b/crates/tower-cmd/src/environments.rs index a003dd35..57609e5f 100644 --- a/crates/tower-cmd/src/environments.rs +++ b/crates/tower-cmd/src/environments.rs @@ -24,12 +24,7 @@ pub fn environments_cmd() -> Command { } pub async fn do_list(config: Config) { - let resp = output::with_spinner( - "Listing environments...", - "Listing environments failed", - api::list_environments(&config), - ) - .await; + let resp = output::with_spinner("Listing environments", api::list_environments(&config)).await; let headers = vec!["Name"] .into_iter() @@ -53,7 +48,6 @@ pub async fn do_create(config: Config, args: &ArgMatches) { output::with_spinner( "Creating environment", - "Creating environment failed", api::create_environment(&config, name), ) .await; diff --git a/crates/tower-cmd/src/output.rs b/crates/tower-cmd/src/output.rs index bbe8be2e..fc3b7bb2 100644 --- a/crates/tower-cmd/src/output.rs +++ b/crates/tower-cmd/src/output.rs @@ -343,7 +343,7 @@ pub fn tower_error_and_die(err: ApiError, operation: &str) -> ! { /// Runs an async operation with a spinner and proper error handling. /// /// This helper provides consistent spinner behavior across all commands: -/// - Shows a spinner with the given message while the operation runs +/// - Shows a spinner with "{operation}..." while the operation runs /// - On success: stops the spinner with success indicator and returns the result /// - On error: stops the spinner with failure indicator and shows auth-aware error message /// @@ -351,16 +351,16 @@ pub fn tower_error_and_die(err: ApiError, operation: &str) -> ! { /// /// ```ignore /// let envs = output::with_spinner( -/// "Listing environments...", -/// "Listing environments failed", +/// "Listing environments", /// api::list_environments(&config) /// ).await; /// ``` -pub async fn with_spinner(spinner_msg: &str, error_operation: &str, future: F) -> T +pub async fn with_spinner(operation: &str, future: F) -> T where F: std::future::Future>>, { - let mut spinner = self::spinner(spinner_msg); + let spinner_msg = format!("{}...", operation); + let mut spinner = self::spinner(&spinner_msg); match future.await { Ok(result) => { spinner.success(); @@ -368,7 +368,8 @@ where } Err(err) => { spinner.failure(); - tower_error_and_die(err, error_operation); + let error_msg = format!("{} failed", operation); + tower_error_and_die(err, &error_msg); } } } @@ -379,15 +380,14 @@ where /// Use this for operations that may be called from MCP or other contexts where /// process exit is not acceptable. Returns the error without displaying it, allowing /// the caller to decide how to handle and display the error. -pub async fn try_with_spinner( - spinner_msg: &str, - _error_operation: &str, - future: F, -) -> Result> +/// +/// Shows "{operation}..." during execution and stops the spinner on completion. +pub async fn try_with_spinner(operation: &str, future: F) -> Result> where F: std::future::Future>>, { - let mut spinner = self::spinner(spinner_msg); + let spinner_msg = format!("{}...", operation); + let mut spinner = self::spinner(&spinner_msg); match future.await { Ok(result) => { spinner.success(); diff --git a/crates/tower-cmd/src/run.rs b/crates/tower-cmd/src/run.rs index 369c2551..dc7eb4e0 100644 --- a/crates/tower-cmd/src/run.rs +++ b/crates/tower-cmd/src/run.rs @@ -255,8 +255,7 @@ pub async fn do_run_remote( }; let res = output::try_with_spinner( - "Scheduling run...", - "Scheduling run failed", + "Scheduling run", api::run_app(&config, &app_slug, env, params), ) .await diff --git a/crates/tower-cmd/src/schedules.rs b/crates/tower-cmd/src/schedules.rs index d9a84afa..68c8cb8e 100644 --- a/crates/tower-cmd/src/schedules.rs +++ b/crates/tower-cmd/src/schedules.rs @@ -101,8 +101,7 @@ pub async fn do_list(config: Config, args: &ArgMatches) { let environment = args.get_one::("environment").map(|s| s.as_str()); let response = output::with_spinner( - "Listing schedules...", - "Listing schedules failed", + "Listing schedules", api::list_schedules(&config, app, environment), ) .await; @@ -150,7 +149,6 @@ pub async fn do_create(config: Config, args: &ArgMatches) { let response = output::with_spinner( "Creating schedule", - "Creating schedule failed", api::create_schedule(&config, app_name, environment, cron, parameters), ) .await; @@ -168,7 +166,6 @@ pub async fn do_update(config: Config, args: &ArgMatches) { output::with_spinner( "Updating schedule", - "Updating schedule failed", api::update_schedule(&config, &schedule_id, cron, parameters), ) .await; @@ -181,7 +178,6 @@ pub async fn do_delete(config: Config, args: &ArgMatches) { output::with_spinner( "Deleting schedule", - "Deleting schedule failed", api::delete_schedule(&config, &schedule_id), ) .await; diff --git a/crates/tower-cmd/src/secrets.rs b/crates/tower-cmd/src/secrets.rs index 1bf34811..5e878c13 100644 --- a/crates/tower-cmd/src/secrets.rs +++ b/crates/tower-cmd/src/secrets.rs @@ -86,8 +86,7 @@ pub async fn do_list(config: Config, args: &ArgMatches) { let (private_key, public_key) = crypto::generate_key_pair(); let list_response = output::with_spinner( - "Listing secrets...", - "Listing secrets failed", + "Listing secrets", api::export_secrets(&config, &env, all, public_key), ) .await; @@ -114,12 +113,8 @@ pub async fn do_list(config: Config, args: &ArgMatches) { .collect(); output::table(headers, data, Some(&list_response.secrets)); } else { - let list_response = output::with_spinner( - "Listing secrets...", - "Listing secrets failed", - api::list_secrets(&config, &env, all), - ) - .await; + let list_response = + output::with_spinner("Listing secrets", api::list_secrets(&config, &env, all)).await; let headers = vec![ "Secret".bold().yellow().to_string(), @@ -175,8 +170,7 @@ pub async fn do_delete(config: Config, args: &ArgMatches) { debug!("deleting secret, environment={} name={}", environment, name); output::with_spinner( - "Deleting secret...", - "Deleting secret failed", + "Deleting secret", api::delete_secret(&config, &name, &environment), ) .await; diff --git a/crates/tower-cmd/src/teams.rs b/crates/tower-cmd/src/teams.rs index 84771968..d631e532 100644 --- a/crates/tower-cmd/src/teams.rs +++ b/crates/tower-cmd/src/teams.rs @@ -27,12 +27,7 @@ async fn refresh_session(config: &Config) -> config::Session { } }; - let resp = output::with_spinner( - "Refreshing session...", - "Refreshing session failed", - api::refresh_session(&config), - ) - .await; + let resp = output::with_spinner("Refreshing session", api::refresh_session(&config)).await; // Create a mutable copy of the session to update let mut session = current_session;