From 9c5fa5b23f1fa7387d06de0ae5fd4fdc0b6f31e3 Mon Sep 17 00:00:00 2001 From: nicholaslyang Date: Fri, 20 Oct 2023 13:07:25 -0400 Subject: [PATCH] Starting on thiserror refactor --- Cargo.lock | 1 + crates/turborepo-auth/Cargo.toml | 1 + crates/turborepo-auth/src/auth/login.rs | 14 +-- crates/turborepo-auth/src/auth/sso.rs | 28 +++--- crates/turborepo-auth/src/error.rs | 12 +++ .../turborepo-auth/src/server/login_server.rs | 16 ++-- .../turborepo-auth/src/server/sso_server.rs | 19 ++-- crates/turborepo-lib/src/cli/error.rs | 47 ++++++++++ .../turborepo-lib/src/{cli.rs => cli/mod.rs} | 35 ++++---- crates/turborepo-lib/src/commands/bin.rs | 14 ++- crates/turborepo-lib/src/commands/login.rs | 62 ++++++++----- crates/turborepo-lib/src/commands/logout.rs | 21 +++-- crates/turborepo-lib/src/commands/unlink.rs | 87 ++++++++++++------- crates/turborepo-lib/src/shim.rs | 10 +-- crates/turborepo-lib/src/tracing.rs | 3 +- .../src/absolute_system_path_buf.rs | 1 + crates/turborepo-scm/src/git.rs | 28 +++--- crates/turborepo-scm/src/lib.rs | 17 ++-- crates/turborepo-scm/src/package_deps.rs | 4 +- 19 files changed, 263 insertions(+), 157 deletions(-) create mode 100644 crates/turborepo-lib/src/cli/error.rs rename crates/turborepo-lib/src/{cli.rs => cli/mod.rs} (98%) diff --git a/Cargo.lock b/Cargo.lock index 48b4eea9b732c..777afe3da1a6e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10302,6 +10302,7 @@ dependencies = [ "turborepo-ui", "turborepo-vercel-api", "turborepo-vercel-api-mock", + "url", "webbrowser", ] diff --git a/crates/turborepo-auth/Cargo.toml b/crates/turborepo-auth/Cargo.toml index a5da16698e6e5..fd3ce8c9d5f81 100644 --- a/crates/turborepo-auth/Cargo.toml +++ b/crates/turborepo-auth/Cargo.toml @@ -21,6 +21,7 @@ turborepo-api-client = { workspace = true } turborepo-ui.workspace = true turborepo-vercel-api = { workspace = true } turborepo-vercel-api-mock = { workspace = true } +url = { workspace = true } webbrowser = { workspace = true } [dev-dependencies] diff --git a/crates/turborepo-auth/src/auth/login.rs b/crates/turborepo-auth/src/auth/login.rs index acf215f63f6f4..18e53f1d67533 100644 --- a/crates/turborepo-auth/src/auth/login.rs +++ b/crates/turborepo-auth/src/auth/login.rs @@ -1,6 +1,5 @@ use std::{borrow::Cow, sync::Arc}; -use anyhow::{anyhow, Result}; pub use error::Error; use reqwest::Url; use tokio::sync::OnceCell; @@ -20,8 +19,8 @@ pub async fn login<'a>( ui: &UI, existing_token: Option<&'a str>, login_url_configuration: &str, - login_server: &impl LoginServer, -) -> Result> { + login_server: impl LoginServer, +) -> Result, Error> { // Check if token exists first. if let Some(token) = existing_token { if let Ok(response) = api_client.get_user(token).await { @@ -65,12 +64,13 @@ pub async fn login<'a>( spinner.finish_and_clear(); - let token = token_cell - .get() - .ok_or_else(|| anyhow!("Failed to get token"))?; + let token = token_cell.get().ok_or(Error::FailedToGetToken)?; // TODO: make this a request to /teams endpoint instead? - let user_response = api_client.get_user(token.as_str()).await?; + let user_response = api_client + .get_user(token.as_str()) + .await + .map_err(Error::FailedToFetchUser)?; ui::print_cli_authorized(&user_response.user.email, ui); diff --git a/crates/turborepo-auth/src/auth/sso.rs b/crates/turborepo-auth/src/auth/sso.rs index 510abaa31a98e..534e078810cb2 100644 --- a/crates/turborepo-auth/src/auth/sso.rs +++ b/crates/turborepo-auth/src/auth/sso.rs @@ -1,20 +1,19 @@ use std::{borrow::Cow, sync::Arc}; -use anyhow::{anyhow, Context, Result}; use reqwest::Url; use tokio::sync::OnceCell; use tracing::warn; use turborepo_api_client::Client; use turborepo_ui::{start_spinner, BOLD, UI}; -use crate::{error, server, ui}; +use crate::{error, server, ui, Error, Error::FailedToMakeSSOTokenName}; const DEFAULT_HOST_NAME: &str = "127.0.0.1"; const DEFAULT_PORT: u16 = 9789; const DEFAULT_SSO_PROVIDER: &str = "SAML/OIDC Single Sign-On"; -fn make_token_name() -> Result { - let host = hostname::get()?; +fn make_token_name() -> Result { + let host = hostname::get().map_err(FailedToMakeSSOTokenName)?; Ok(format!( "Turbo CLI on {} via {DEFAULT_SSO_PROVIDER}", @@ -30,8 +29,8 @@ pub async fn sso_login<'a>( existing_token: Option<&'a str>, login_url_configuration: &str, sso_team: &str, - login_server: &impl server::SSOLoginServer, -) -> Result> { + login_server: impl server::SSOLoginServer, +) -> Result, Error> { // Check if token exists first. Must be there for the user and contain the // sso_team passed into this function. if let Some(token) = existing_token { @@ -80,14 +79,19 @@ pub async fn sso_login<'a>( login_server.run(DEFAULT_PORT, token_cell.clone()).await?; spinner.finish_and_clear(); - let token = token_cell - .get() - .ok_or_else(|| anyhow!("no token auth token found"))?; + let token = token_cell.get().ok_or(Error::FailedToGetToken)?; - let token_name = make_token_name().context("failed to make sso token name")?; + let token_name = make_token_name()?; - let verified_user = api_client.verify_sso_token(token, &token_name).await?; - let user_response = api_client.get_user(&verified_user.token).await?; + let verified_user = api_client + .verify_sso_token(token, &token_name) + .await + .map_err(Error::FailedToValidateSSOToken)?; + + let user_response = api_client + .get_user(&verified_user.token) + .await + .map_err(Error::FailedToFetchUser)?; ui::print_cli_authorized(&user_response.user.email, ui); diff --git a/crates/turborepo-auth/src/error.rs b/crates/turborepo-auth/src/error.rs index 1355c63aab2b5..01ad3cb3f4d4b 100644 --- a/crates/turborepo-auth/src/error.rs +++ b/crates/turborepo-auth/src/error.rs @@ -1,3 +1,5 @@ +use std::io; + use thiserror::Error; #[derive(Debug, Error)] @@ -7,4 +9,14 @@ pub enum Error { situations like using a `data:` URL." )] LoginUrlCannotBeABase { value: String }, + #[error("failed to get token")] + FailedToGetToken, + #[error("failed to fetch user: {0}")] + FailedToFetchUser(turborepo_api_client::Error), + #[error("url is invalid: {0}")] + InvalidUrl(#[from] url::ParseError), + #[error("failed to validate sso token")] + FailedToValidateSSOToken(turborepo_api_client::Error), + #[error("failed to make sso token name")] + FailedToMakeSSOTokenName(io::Error), } diff --git a/crates/turborepo-auth/src/server/login_server.rs b/crates/turborepo-auth/src/server/login_server.rs index 27cb4908f4906..440c5118054d0 100644 --- a/crates/turborepo-auth/src/server/login_server.rs +++ b/crates/turborepo-auth/src/server/login_server.rs @@ -6,6 +6,8 @@ use axum::{extract::Query, response::Redirect, routing::get, Router}; use serde::Deserialize; use tokio::sync::OnceCell; +use crate::Error; + #[derive(Debug, Clone, Deserialize)] struct LoginPayload { token: String, @@ -18,19 +20,12 @@ pub trait LoginServer { port: u16, login_url_base: String, login_token: Arc>, - ) -> Result<()>; + ) -> Result<(), Error>; } /// TODO: Document this. -#[derive(Default)] pub struct DefaultLoginServer; -impl DefaultLoginServer { - pub fn new() -> Self { - DefaultLoginServer {} - } -} - #[async_trait] impl LoginServer for DefaultLoginServer { async fn run( @@ -38,7 +33,7 @@ impl LoginServer for DefaultLoginServer { port: u16, login_url_base: String, login_token: Arc>, - ) -> Result<()> { + ) -> Result<(), Error> { let handle = axum_server::Handle::new(); let route_handle = handle.clone(); let app = Router::new() @@ -56,6 +51,7 @@ impl LoginServer for DefaultLoginServer { Ok(axum_server::bind(addr) .handle(handle) .serve(app.into_make_service()) - .await?) + .await + .expect("failed to start one-shot server")) } } diff --git a/crates/turborepo-auth/src/server/sso_server.rs b/crates/turborepo-auth/src/server/sso_server.rs index 3e8a6818ffd9b..51a6391d1d4a9 100644 --- a/crates/turborepo-auth/src/server/sso_server.rs +++ b/crates/turborepo-auth/src/server/sso_server.rs @@ -1,12 +1,13 @@ use std::{net::SocketAddr, sync::Arc}; -use anyhow::Result; use async_trait::async_trait; use axum::{extract::Query, response::Redirect, routing::get, Router}; use reqwest::Url; use serde::Deserialize; use tokio::sync::OnceCell; +use crate::Error; + #[derive(Debug, Default, Clone, Deserialize)] #[allow(dead_code)] pub struct SsoPayload { @@ -20,22 +21,15 @@ pub struct SsoPayload { #[async_trait] pub trait SSOLoginServer { - async fn run(&self, port: u16, verification_token: Arc>) -> Result<()>; + async fn run(&self, port: u16, verification_token: Arc>) -> Result<(), Error>; } /// TODO: Document this. -#[derive(Default)] pub struct DefaultSSOLoginServer; -impl DefaultSSOLoginServer { - pub fn new() -> Self { - DefaultSSOLoginServer {} - } -} - #[async_trait] impl SSOLoginServer for DefaultSSOLoginServer { - async fn run(&self, port: u16, verification_token: Arc>) -> Result<()> { + async fn run(&self, port: u16, verification_token: Arc>) -> Result<(), Error> { let handle = axum_server::Handle::new(); let route_handle = handle.clone(); let app = Router::new() @@ -57,11 +51,12 @@ impl SSOLoginServer for DefaultSSOLoginServer { Ok(axum_server::bind(addr) .handle(handle) .serve(app.into_make_service()) - .await?) + .await + .expect("failed to start one-shot server")) } } -fn get_token_and_redirect(payload: SsoPayload) -> Result<(Option, Url)> { +fn get_token_and_redirect(payload: SsoPayload) -> Result<(Option, Url), Error> { let location_stub = "https://vercel.com/notifications/cli-login/turbo/"; if let Some(login_error) = payload.login_error { let mut url = Url::parse(&format!("{}failed", location_stub))?; diff --git a/crates/turborepo-lib/src/cli/error.rs b/crates/turborepo-lib/src/cli/error.rs new file mode 100644 index 0000000000000..8ef862ff126ba --- /dev/null +++ b/crates/turborepo-lib/src/cli/error.rs @@ -0,0 +1,47 @@ +use std::{backtrace, io}; + +use thiserror::Error; +use turbopath::AbsoluteSystemPathBuf; + +use crate::{ + commands::{bin, prune}, + daemon::DaemonError, + rewrite_json::RewriteError, +}; + +#[derive(Debug, Error)] +pub enum Error { + #[error("No command specified")] + NoCommand(#[backtrace] backtrace::Backtrace), + #[error("{0}")] + Bin(#[from] bin::Error, #[backtrace] backtrace::Backtrace), + #[error(transparent)] + Path(#[from] turbopath::PathError), + #[error("at least one task must be specified")] + NoTasks(#[backtrace] backtrace::Backtrace), + #[error(transparent)] + Config(#[from] crate::config::Error), + #[error(transparent)] + ChromeTracing(#[from] crate::tracing::Error), + #[error("Encountered an IO error while attempting to read {config_path}: {error}")] + FailedToReadConfig { + config_path: AbsoluteSystemPathBuf, + error: io::Error, + }, + #[error("Encountered an IO error while attempting to set {config_path}: {error}")] + FailedToSetConfig { + config_path: AbsoluteSystemPathBuf, + error: io::Error, + }, + #[error(transparent)] + Rewrite(#[from] RewriteError), + #[error(transparent)] + Auth(#[from] turborepo_auth::Error), + #[error(transparent)] + Daemon(#[from] DaemonError), + #[error(transparent)] + Prune(#[from] prune::Error), + // Temporary to prevent having to move all of the errors from anyhow to thiserror at once. + #[error(transparent)] + Anyhow(#[from] anyhow::Error), +} diff --git a/crates/turborepo-lib/src/cli.rs b/crates/turborepo-lib/src/cli/mod.rs similarity index 98% rename from crates/turborepo-lib/src/cli.rs rename to crates/turborepo-lib/src/cli/mod.rs index ab711d31eb00d..92e1ae40c4081 100644 --- a/crates/turborepo-lib/src/cli.rs +++ b/crates/turborepo-lib/src/cli/mod.rs @@ -1,9 +1,9 @@ -use std::{env, io, mem, path::Path, process}; +use std::{backtrace, backtrace::Backtrace, env, io, mem, process}; -use anyhow::{anyhow, Result}; -use camino::Utf8PathBuf; +use camino::{Utf8Path, Utf8PathBuf}; use clap::{ArgAction, CommandFactory, Parser, Subcommand, ValueEnum}; use clap_complete::{generate, Shell}; +pub use error::Error; use serde::{Deserialize, Serialize}; use tracing::{debug, error}; use turbopath::AbsoluteSystemPathBuf; @@ -17,6 +17,8 @@ use crate::{ Payload, }; +mod error; + // Global turbo sets this environment variable to its cwd so that local // turbo can use it for package inference. pub const INVOCATION_DIR_ENV_VAR: &str = "TURBO_INVOCATION_DIR"; @@ -201,14 +203,13 @@ pub enum LinkTarget { } impl Args { - pub fn new() -> Result { + pub fn new() -> Self { // We always pass --single-package in from the shim. // We need to omit it, and then add it in for run. - let arg_separator_position = - std::env::args_os().position(|input_token| input_token == "--"); + let arg_separator_position = env::args_os().position(|input_token| input_token == "--"); let single_package_position = - std::env::args_os().position(|input_token| input_token == "--single-package"); + env::args_os().position(|input_token| input_token == "--single-package"); let is_single_package = match (arg_separator_position, single_package_position) { (_, None) => false, @@ -282,7 +283,7 @@ impl Args { clap_args.test_run = true; } - Ok(clap_args) + clap_args } pub fn get_tasks(&self) -> &[String] { @@ -631,14 +632,15 @@ pub async fn run( repo_state: Option, #[allow(unused_variables)] logger: &TurboSubscriber, ui: UI, -) -> Result { - let mut cli_args = Args::new()?; +) -> Result { + let mut cli_args = Args::new(); // If there is no command, we set the command to `Command::Run` with // `self.parsed_args.run_args` as arguments. let mut command = if let Some(command) = mem::take(&mut cli_args.command) { command } else { - let run_args = mem::take(&mut cli_args.run_args).ok_or(anyhow!("No command specified"))?; + let run_args = mem::take(&mut cli_args.run_args) + .ok_or_else(|| Error::NoCommand(Backtrace::capture()))?; if run_args.tasks.is_empty() { let mut cmd = ::command(); let _ = cmd.print_help(); @@ -660,7 +662,7 @@ pub async fn run( // inference root, as long as the user hasn't overridden the cwd if cli_args.cwd.is_none() { if let Ok(invocation_dir) = env::var(INVOCATION_DIR_ENV_VAR) { - let invocation_path = Path::new(&invocation_dir); + let invocation_path = Utf8Path::new(&invocation_dir); // If repo state doesn't exist, we're either local turbo running at the root // (cwd), or inference failed. @@ -669,11 +671,8 @@ pub async fn run( let this_dir = AbsoluteSystemPathBuf::cwd()?; let repo_root = repo_state.as_ref().map_or(&this_dir, |r| &r.root); if let Ok(relative_path) = invocation_path.strip_prefix(repo_root) { - debug!("pkg_inference_root set to \"{}\"", relative_path.display()); - let utf8_path = relative_path - .to_str() - .ok_or_else(|| anyhow!("invalid utf8 path: {:?}", relative_path))?; - run_args.pkg_inference_root = Some(utf8_path.to_owned()); + debug!("pkg_inference_root set to \"{}\"", relative_path); + run_args.pkg_inference_root = Some(relative_path.to_string()); } } else { debug!("{} not set", INVOCATION_DIR_ENV_VAR); @@ -808,7 +807,7 @@ pub async fn run( // in the case of enabling the run stub, we want to be able to opt-in // to the rust codepath for running turbo if args.tasks.is_empty() { - return Err(anyhow!("at least one task must be specified")); + return Err(Error::NoTasks(backtrace::Backtrace::capture())); } if let Some(file_path) = &args.profile { // TODO: Do we want to handle the result / error? diff --git a/crates/turborepo-lib/src/commands/bin.rs b/crates/turborepo-lib/src/commands/bin.rs index 21cc11a27fb95..297d0bed2bddb 100644 --- a/crates/turborepo-lib/src/commands/bin.rs +++ b/crates/turborepo-lib/src/commands/bin.rs @@ -1,9 +1,15 @@ -use std::env::current_exe; +use std::{env::current_exe, io}; -use anyhow::{anyhow, Result}; +use thiserror::Error; -pub fn run() -> Result<()> { - let path = current_exe().map_err(|e| anyhow!("could not get path to turbo binary: {}", e))?; +#[derive(Debug, Error)] +pub enum Error { + #[error("could not get path to turbo binary: {0}")] + NoCurrentExe(#[from] io::Error), +} + +pub fn run() -> Result<(), Error> { + let path = current_exe()?; // NOTE: The Go version uses `base.UI.Output`, we should use the Rust equivalent // eventually. println!("{}", path.to_string_lossy()); diff --git a/crates/turborepo-lib/src/commands/login.rs b/crates/turborepo-lib/src/commands/login.rs index 1d0b56a9b2d36..f2df02f31086c 100644 --- a/crates/turborepo-lib/src/commands/login.rs +++ b/crates/turborepo-lib/src/commands/login.rs @@ -1,12 +1,11 @@ -use anyhow::{anyhow, Result}; use turborepo_api_client::APIClient; use turborepo_auth::{ login as auth_login, sso_login as auth_sso_login, DefaultLoginServer, DefaultSSOLoginServer, }; -use crate::{commands::CommandBase, rewrite_json::set_path}; +use crate::{cli::Error, commands::CommandBase, rewrite_json::set_path}; -pub async fn sso_login(base: &mut CommandBase, sso_team: &str) -> Result<()> { +pub async fn sso_login(base: &mut CommandBase, sso_team: &str) -> Result<(), Error> { let api_client: APIClient = base.api_client()?; let ui = base.ui; let login_url_config = base.config()?.login_url().to_string(); @@ -17,28 +16,38 @@ pub async fn sso_login(base: &mut CommandBase, sso_team: &str) -> Result<()> { base.config()?.token(), &login_url_config, sso_team, - &DefaultSSOLoginServer::new(), + DefaultSSOLoginServer, ) .await?; let global_config_path = base.global_config_path()?; let before = global_config_path .read_existing_to_string_or(Ok("{}")) - .map_err(|e| { - anyhow!( - "Encountered an IO error while attempting to read {}: {}", - global_config_path, - e - ) + .map_err(|e| Error::FailedToReadConfig { + config_path: global_config_path.clone(), + error: e, })?; + let after = set_path(&before, &["token"], &format!("\"{}\"", token))?; - global_config_path.ensure_dir()?; - global_config_path.create_with_contents(after)?; + + global_config_path + .ensure_dir() + .map_err(|e| Error::FailedToSetConfig { + config_path: global_config_path.clone(), + error: e, + })?; + + global_config_path + .create_with_contents(after) + .map_err(|e| Error::FailedToSetConfig { + config_path: global_config_path.clone(), + error: e, + })?; Ok(()) } -pub async fn login(base: &mut CommandBase) -> Result<()> { +pub async fn login(base: &mut CommandBase) -> Result<(), Error> { let api_client: APIClient = base.api_client()?; let ui = base.ui; let login_url_config = base.config()?.login_url().to_string(); @@ -48,23 +57,32 @@ pub async fn login(base: &mut CommandBase) -> Result<()> { &ui, base.config()?.token(), &login_url_config, - &DefaultLoginServer::new(), + DefaultLoginServer, ) .await?; let global_config_path = base.global_config_path()?; let before = global_config_path .read_existing_to_string_or(Ok("{}")) - .map_err(|e| { - anyhow!( - "Encountered an IO error while attempting to read {}: {}", - global_config_path, - e - ) + .map_err(|e| Error::FailedToReadConfig { + config_path: global_config_path.clone(), + error: e, })?; let after = set_path(&before, &["token"], &format!("\"{}\"", token))?; - global_config_path.ensure_dir()?; - global_config_path.create_with_contents(after)?; + + global_config_path + .ensure_dir() + .map_err(|e| Error::FailedToSetConfig { + config_path: global_config_path.clone(), + error: e, + })?; + + global_config_path + .create_with_contents(after) + .map_err(|e| Error::FailedToSetConfig { + config_path: global_config_path.clone(), + error: e, + })?; Ok(()) } diff --git a/crates/turborepo-lib/src/commands/logout.rs b/crates/turborepo-lib/src/commands/logout.rs index cb4eed6fa2b9c..45e5785ec3131 100644 --- a/crates/turborepo-lib/src/commands/logout.rs +++ b/crates/turborepo-lib/src/commands/logout.rs @@ -1,10 +1,9 @@ -use anyhow::{anyhow, Error, Result}; use tracing::error; use turborepo_auth::logout as auth_logout; -use crate::{commands::CommandBase, rewrite_json::unset_path}; +use crate::{cli, cli::Error, commands::CommandBase, rewrite_json::unset_path}; -pub fn logout(base: &mut CommandBase) -> Result<()> { +pub fn logout(base: &mut CommandBase) -> Result<(), Error> { if let Err(err) = remove_token(base) { error!("could not logout. Something went wrong: {}", err); return Err(err); @@ -15,22 +14,22 @@ pub fn logout(base: &mut CommandBase) -> Result<()> { Ok(()) } -fn remove_token(base: &mut CommandBase) -> Result<()> { +fn remove_token(base: &mut CommandBase) -> Result<(), Error> { let global_config_path = base.global_config_path()?; let before = global_config_path .read_existing_to_string_or(Ok("{}")) - .map_err(|e| { - anyhow!( - "Encountered an IO error while attempting to read {}: {}", - global_config_path, - e - ) + .map_err(|e| Error::FailedToReadConfig { + config_path: global_config_path.clone(), + error: e, })?; if let Some(after) = unset_path(&before, &["token"], true)? { global_config_path .create_with_contents(after) - .map_err(Error::from) + .map_err(|e| cli::Error::FailedToSetConfig { + config_path: global_config_path.clone(), + error: e, + }) } else { Ok(()) } diff --git a/crates/turborepo-lib/src/commands/unlink.rs b/crates/turborepo-lib/src/commands/unlink.rs index f017dc4843502..c3166eb27b39d 100644 --- a/crates/turborepo-lib/src/commands/unlink.rs +++ b/crates/turborepo-lib/src/commands/unlink.rs @@ -1,12 +1,12 @@ use std::fs; -use anyhow::{anyhow, Context, Result}; use turborepo_ui::GREY; use crate::{ - cli::LinkTarget, + cli, + cli::{Error, LinkTarget}, commands::CommandBase, - rewrite_json::{self, unset_path}, + rewrite_json::unset_path, }; enum UnlinkSpacesResult { @@ -14,26 +14,35 @@ enum UnlinkSpacesResult { NoSpacesFound, } -fn unlink_remote_caching(base: &mut CommandBase) -> Result<()> { +fn unlink_remote_caching(base: &mut CommandBase) -> Result<(), cli::Error> { let needs_disabling = base.config()?.team_id().is_some() || base.config()?.team_slug().is_some(); let output = if needs_disabling { - let before = base - .local_config_path() + let local_config_path = base.local_config_path(); + + let before = local_config_path .read_existing_to_string_or(Ok("{}")) - .map_err(|e| { - anyhow!( - "Encountered an IO error while attempting to read {}: {}", - base.local_config_path(), - e - ) + .map_err(|error| cli::Error::FailedToReadConfig { + config_path: local_config_path.clone(), + error, })?; let no_id = unset_path(&before, &["teamid"], false)?.unwrap_or(before); let no_slug = unset_path(&no_id, &["teamslug"], false)?.unwrap_or(no_id); - base.local_config_path().ensure_dir()?; - base.local_config_path().create_with_contents(no_slug)?; + local_config_path + .ensure_dir() + .map_err(|error| cli::Error::FailedToSetConfig { + config_path: local_config_path.clone(), + error, + })?; + + local_config_path + .create_with_contents(no_slug) + .map_err(|error| cli::Error::FailedToSetConfig { + config_path: local_config_path.clone(), + error, + })?; "> Disabled Remote Caching" } else { @@ -45,31 +54,38 @@ fn unlink_remote_caching(base: &mut CommandBase) -> Result<()> { Ok(()) } -fn unlink_spaces(base: &mut CommandBase) -> Result<()> { +fn unlink_spaces(base: &mut CommandBase) -> Result<(), cli::Error> { let needs_disabling = base.config()?.team_id().is_some() || base.config()?.team_slug().is_some(); if needs_disabling { - let before = base - .local_config_path() + let local_config_path = base.local_config_path(); + let before = local_config_path .read_existing_to_string_or(Ok("{}")) - .map_err(|e| { - anyhow!( - "Encountered an IO error while attempting to read {}: {}", - base.local_config_path(), - e - ) + .map_err(|e| Error::FailedToReadConfig { + config_path: local_config_path.clone(), + error: e, })?; let no_id = unset_path(&before, &["teamid"], false)?.unwrap_or(before); let no_slug = unset_path(&no_id, &["teamslug"], false)?.unwrap_or(no_id); - base.local_config_path().ensure_dir()?; - base.local_config_path().create_with_contents(no_slug)?; + local_config_path + .ensure_dir() + .map_err(|e| Error::FailedToSetConfig { + config_path: local_config_path.clone(), + error: e, + })?; + + local_config_path + .create_with_contents(no_slug) + .map_err(|e| Error::FailedToSetConfig { + config_path: local_config_path.clone(), + error: e, + })?; } // Space config is _also_ in turbo.json. - let result = - remove_spaces_from_turbo_json(base).context("Could not unlink. Something went wrong")?; + let result = remove_spaces_from_turbo_json(base)?; let output = match (needs_disabling, result) { (_, UnlinkSpacesResult::Unlinked) => "> Unlinked Spaces", @@ -82,7 +98,7 @@ fn unlink_spaces(base: &mut CommandBase) -> Result<()> { Ok(()) } -pub fn unlink(base: &mut CommandBase, target: LinkTarget) -> Result<()> { +pub fn unlink(base: &mut CommandBase, target: LinkTarget) -> Result<(), cli::Error> { match target { LinkTarget::RemoteCache => { unlink_remote_caching(base)?; @@ -94,13 +110,20 @@ pub fn unlink(base: &mut CommandBase, target: LinkTarget) -> Result<()> { Ok(()) } -fn remove_spaces_from_turbo_json(base: &CommandBase) -> Result { +fn remove_spaces_from_turbo_json(base: &CommandBase) -> Result { let turbo_json_path = base.repo_root.join_component("turbo.json"); - let turbo_json = fs::read_to_string(&turbo_json_path)?; + let turbo_json = + fs::read_to_string(&turbo_json_path).map_err(|e| Error::FailedToReadConfig { + config_path: turbo_json_path.clone(), + error: e, + })?; - let output = rewrite_json::unset_path(&turbo_json, &["experimentalSpaces", "id"], true)?; + let output = unset_path(&turbo_json, &["experimentalSpaces", "id"], true)?; if let Some(output) = output { - fs::write(turbo_json_path, output)?; + fs::write(&turbo_json_path, output).map_err(|e| Error::FailedToSetConfig { + config_path: turbo_json_path.clone(), + error: e, + })?; Ok(UnlinkSpacesResult::Unlinked) } else { Ok(UnlinkSpacesResult::NoSpacesFound) diff --git a/crates/turborepo-lib/src/shim.rs b/crates/turborepo-lib/src/shim.rs index 917954cc0dea8..1be947463b6a7 100644 --- a/crates/turborepo-lib/src/shim.rs +++ b/crates/turborepo-lib/src/shim.rs @@ -462,7 +462,7 @@ fn run_correct_turbo( shim_args.invocation_dir.as_path(), ); debug!("Currently running turbo is local turbo."); - cli::run(Some(repo_state), subscriber, ui) + Ok(cli::run(Some(repo_state), subscriber, ui)?) } else { Ok(Payload::Rust(spawn_local_turbo( &repo_state, @@ -479,7 +479,7 @@ fn run_correct_turbo( shim_args.invocation_dir.as_path(), ); debug!("Running command as global turbo"); - cli::run(Some(repo_state), subscriber, ui) + Ok(cli::run(Some(repo_state), subscriber, ui)?) } } @@ -611,7 +611,7 @@ pub fn run() -> Result { // global turbo having handled the inference. We can run without any // concerns. if args.skip_infer { - return cli::run(None, &subscriber, ui); + return Ok(cli::run(None, &subscriber, ui)?); } // If the TURBO_BINARY_PATH is set, we do inference but we do not use @@ -620,7 +620,7 @@ pub fn run() -> Result { if is_turbo_binary_path_set() { let repo_state = RepoState::infer(&args.cwd)?; debug!("Repository Root: {}", repo_state.root); - return cli::run(Some(repo_state), &subscriber, ui); + return Ok(cli::run(Some(repo_state), &subscriber, ui)?); } match RepoState::infer(&args.cwd) { @@ -633,7 +633,7 @@ pub fn run() -> Result { // commands like login/logout/link/unlink to still work debug!("Repository inference failed: {}", err); debug!("Running command as global turbo"); - cli::run(None, &subscriber, ui) + Ok(cli::run(None, &subscriber, ui)?) } } } diff --git a/crates/turborepo-lib/src/tracing.rs b/crates/turborepo-lib/src/tracing.rs index 2b3a939ec7ec2..83ddb445ed1f9 100644 --- a/crates/turborepo-lib/src/tracing.rs +++ b/crates/turborepo-lib/src/tracing.rs @@ -8,6 +8,7 @@ use owo_colors::{ use tracing::{field::Visit, metadata::LevelFilter, trace, Event, Level, Subscriber}; use tracing_appender::{non_blocking::NonBlocking, rolling::RollingFileAppender}; use tracing_chrome::ChromeLayer; +pub use tracing_subscriber::reload::Error; use tracing_subscriber::{ filter::{Filtered, Targets}, fmt::{ @@ -18,7 +19,7 @@ use tracing_subscriber::{ layer, prelude::*, registry::LookupSpan, - reload::{self, Error, Handle}, + reload::{self, Handle}, EnvFilter, Layer, Registry, }; use turborepo_ui::UI; diff --git a/crates/turborepo-paths/src/absolute_system_path_buf.rs b/crates/turborepo-paths/src/absolute_system_path_buf.rs index 985d3b2cf3652..481698ae47967 100644 --- a/crates/turborepo-paths/src/absolute_system_path_buf.rs +++ b/crates/turborepo-paths/src/absolute_system_path_buf.rs @@ -97,6 +97,7 @@ impl AbsoluteSystemPathBuf { } pub fn cwd() -> Result { + // TODO(errors): Unwrap current_dir() Ok(Self(Utf8PathBuf::try_from(std::env::current_dir()?)?)) } diff --git a/crates/turborepo-scm/src/git.rs b/crates/turborepo-scm/src/git.rs index 82e9a2b78bb91..799738da2072a 100644 --- a/crates/turborepo-scm/src/git.rs +++ b/crates/turborepo-scm/src/git.rs @@ -238,7 +238,7 @@ mod tests { use super::previous_content; use crate::{git::changed_files, Error}; - fn setup_repository() -> Result<(TempDir, Repository), Error> { + fn setup_repository() -> Result<(TempDir, Repository), PathError> { let repo_root = tempfile::tempdir()?; let repo = Repository::init(repo_root.path()).unwrap(); let mut config = repo.config().unwrap(); @@ -294,7 +294,7 @@ mod tests { } #[test] - fn test_shallow_clone() -> Result<(), Error> { + fn test_shallow_clone() -> Result<(), PathError> { let tmp_dir = tempfile::tempdir()?; let git_binary = which("git")?; @@ -329,7 +329,7 @@ mod tests { } #[test] - fn test_deleted_files() -> Result<(), Error> { + fn test_deleted_files() -> Result<(), PathError> { let (repo_root, repo) = setup_repository()?; let file = repo_root.path().join("foo.js"); @@ -351,7 +351,7 @@ mod tests { } #[test] - fn test_merge_base() -> Result<(), Error> { + fn test_merge_base() -> Result<(), PathError> { let (repo_root, repo) = setup_repository()?; let first_file = repo_root.path().join("foo.js"); fs::write(first_file, "let z = 0;")?; @@ -398,7 +398,7 @@ mod tests { } #[test] - fn test_changed_files() -> Result<(), Error> { + fn test_changed_files() -> Result<(), PathError> { let (repo_root, repo) = setup_repository()?; let mut index = repo.index().unwrap(); let turbo_root = repo_root.path(); @@ -464,7 +464,7 @@ mod tests { } #[test] - fn test_changed_files_with_root_as_relative() -> Result<(), Error> { + fn test_changed_files_with_root_as_relative() -> Result<(), PathError> { let (repo_root, repo) = setup_repository()?; let file = repo_root.path().join("foo.js"); fs::write(file, "let z = 0;")?; @@ -492,7 +492,7 @@ mod tests { // Tests that we can use a subdir as the turbo_root path // (occurs when the monorepo is nested inside a subdirectory of git repository) #[test] - fn test_changed_files_with_subdir_as_turbo_root() -> Result<(), Error> { + fn test_changed_files_with_subdir_as_turbo_root() -> Result<(), PathError> { let (repo_root, repo) = setup_repository()?; fs::create_dir(repo_root.path().join("subdir"))?; @@ -553,7 +553,7 @@ mod tests { } #[test] - fn test_previous_content() -> Result<(), Error> { + fn test_previous_content() -> Result<(), PathError> { let (repo_root, repo) = setup_repository()?; let root = AbsoluteSystemPathBuf::try_from(repo_root.path()).unwrap(); @@ -590,7 +590,7 @@ mod tests { } #[test] - fn test_revparse() -> Result<(), Error> { + fn test_revparse() -> Result<(), PathError> { let (repo_root, repo) = setup_repository()?; let root = AbsoluteSystemPathBuf::try_from(repo_root.path()).unwrap(); @@ -635,7 +635,7 @@ mod tests { } #[test] - fn test_error_cases() -> Result<(), Error> { + fn test_error_cases() -> Result<(), PathError> { let repo_dir = tempfile::tempdir()?; let repo_does_not_exist = changed_files( repo_dir.path().to_path_buf(), @@ -644,7 +644,7 @@ mod tests { "HEAD", ); - assert_matches!(repo_does_not_exist, Err(Error::GitRequired(_))); + assert_matches!(repo_does_not_exist, Err(PathError::GitRequired(_))); let (repo_root, _repo) = setup_repository()?; let root = AbsoluteSystemPathBuf::try_from(repo_root.path()).unwrap(); @@ -656,14 +656,14 @@ mod tests { "does-not-exist", ); - assert_matches!(commit_does_not_exist, Err(Error::Git(_, _))); + assert_matches!(commit_does_not_exist, Err(PathError::Git(_, _))); let file_does_not_exist = previous_content( repo_root.path().to_path_buf(), "HEAD", root.join_component("does-not-exist").to_string(), ); - assert_matches!(file_does_not_exist, Err(Error::Git(_, _))); + assert_matches!(file_does_not_exist, Err(PathError::Git(_, _))); let turbo_root = tempfile::tempdir()?; let turbo_root_is_not_subdir_of_git_root = changed_files( @@ -675,7 +675,7 @@ mod tests { assert_matches!( turbo_root_is_not_subdir_of_git_root, - Err(Error::Path(PathError::NotParent(_, _), _)) + Err(PathError::Path(PathError::NotParent(_, _), _)) ); Ok(()) diff --git a/crates/turborepo-scm/src/lib.rs b/crates/turborepo-scm/src/lib.rs index 1bf1fe7c2269d..78f1f2c61fda4 100644 --- a/crates/turborepo-scm/src/lib.rs +++ b/crates/turborepo-scm/src/lib.rs @@ -4,7 +4,7 @@ #![deny(clippy::all)] use std::{ - backtrace::{self, Backtrace}, + backtrace, io::Read, process::{Child, Command}, }; @@ -12,7 +12,7 @@ use std::{ use bstr::io::BufReadExt; use thiserror::Error; use tracing::debug; -use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, PathError, RelativeUnixPathBuf}; +use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, RelativeUnixPathBuf}; pub mod git; mod hash_object; @@ -39,7 +39,10 @@ pub enum Error { #[error("io error: {0}")] Io(#[from] std::io::Error, #[backtrace] backtrace::Backtrace), #[error("path error: {0}")] - Path(#[from] PathError, #[backtrace] backtrace::Backtrace), + Path( + #[from] turbopath::PathError, + #[backtrace] backtrace::Backtrace, + ), #[error("could not find git binary")] GitBinaryNotFound(#[from] which::Error), #[error("encoding error: {0}")] @@ -57,17 +60,17 @@ pub enum Error { impl From for Error { fn from(value: wax::BuildError) -> Self { - Error::Glob(Box::new(value), Backtrace::capture()) + Error::Glob(Box::new(value), backtrace::Backtrace::capture()) } } impl Error { pub(crate) fn git_error(s: impl Into) -> Self { - Error::Git(s.into(), Backtrace::capture()) + Error::Git(s.into(), backtrace::Backtrace::capture()) } pub(crate) fn git2_error_context(error: git2::Error, error_context: String) -> Self { - Error::Git2(error, error_context, Backtrace::capture()) + Error::Git2(error, error_context, backtrace::Backtrace::capture()) } } @@ -119,7 +122,7 @@ pub(crate) fn wait_for_success( "'{}' in {}{}{}{}", command, path_text, parse_error_text, exit_text, stderr_text ); - Err(Error::Git(err_text, Backtrace::capture())) + Err(Error::Git(err_text, backtrace::Backtrace::capture())) } #[derive(Debug)] diff --git a/crates/turborepo-scm/src/package_deps.rs b/crates/turborepo-scm/src/package_deps.rs index 47db24fd2bf07..839fa7c0a1aec 100644 --- a/crates/turborepo-scm/src/package_deps.rs +++ b/crates/turborepo-scm/src/package_deps.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use itertools::{Either, Itertools}; use tracing::debug; -use turbopath::{AbsoluteSystemPath, AnchoredSystemPath, PathError, RelativeUnixPathBuf}; +use turbopath::{AbsoluteSystemPath, AnchoredSystemPath, RelativeUnixPathBuf}; use crate::{hash_object::hash_objects, Error, Git, SCM}; @@ -117,7 +117,7 @@ impl Git { .anchor(process_relative_to.resolve(f.as_ref()))? .to_unix()) }) - .collect::, PathError>>()?; + .collect::, Error>>()?; // Note: to_hash is *git repo relative* hash_objects(&self.root, process_relative_to, to_hash, &mut hashes)?; Ok(hashes)