Skip to content

Commit

Permalink
Starting on thiserror refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
nicholaslyang authored and nicholaslyang committed Oct 23, 2023
1 parent 338cdb7 commit 9c5fa5b
Show file tree
Hide file tree
Showing 19 changed files with 263 additions and 157 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/turborepo-auth/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
14 changes: 7 additions & 7 deletions crates/turborepo-auth/src/auth/login.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<Cow<'a, str>> {
login_server: impl LoginServer,
) -> Result<Cow<'a, str>, Error> {
// Check if token exists first.
if let Some(token) = existing_token {
if let Ok(response) = api_client.get_user(token).await {
Expand Down Expand Up @@ -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);

Expand Down
28 changes: 16 additions & 12 deletions crates/turborepo-auth/src/auth/sso.rs
Original file line number Diff line number Diff line change
@@ -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<String> {
let host = hostname::get()?;
fn make_token_name() -> Result<String, Error> {
let host = hostname::get().map_err(FailedToMakeSSOTokenName)?;

Ok(format!(
"Turbo CLI on {} via {DEFAULT_SSO_PROVIDER}",
Expand All @@ -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<Cow<'a, str>> {
login_server: impl server::SSOLoginServer,
) -> Result<Cow<'a, str>, 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 {
Expand Down Expand Up @@ -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);

Expand Down
12 changes: 12 additions & 0 deletions crates/turborepo-auth/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::io;

use thiserror::Error;

#[derive(Debug, Error)]
Expand All @@ -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),
}
16 changes: 6 additions & 10 deletions crates/turborepo-auth/src/server/login_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -18,27 +20,20 @@ pub trait LoginServer {
port: u16,
login_url_base: String,
login_token: Arc<OnceCell<String>>,
) -> 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(
&self,
port: u16,
login_url_base: String,
login_token: Arc<OnceCell<String>>,
) -> Result<()> {
) -> Result<(), Error> {
let handle = axum_server::Handle::new();
let route_handle = handle.clone();
let app = Router::new()
Expand All @@ -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"))
}
}
19 changes: 7 additions & 12 deletions crates/turborepo-auth/src/server/sso_server.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -20,22 +21,15 @@ pub struct SsoPayload {

#[async_trait]
pub trait SSOLoginServer {
async fn run(&self, port: u16, verification_token: Arc<OnceCell<String>>) -> Result<()>;
async fn run(&self, port: u16, verification_token: Arc<OnceCell<String>>) -> 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<OnceCell<String>>) -> Result<()> {
async fn run(&self, port: u16, verification_token: Arc<OnceCell<String>>) -> Result<(), Error> {
let handle = axum_server::Handle::new();
let route_handle = handle.clone();
let app = Router::new()
Expand All @@ -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<String>, Url)> {
fn get_token_and_redirect(payload: SsoPayload) -> Result<(Option<String>, 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))?;
Expand Down
47 changes: 47 additions & 0 deletions crates/turborepo-lib/src/cli/error.rs
Original file line number Diff line number Diff line change
@@ -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),
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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";
Expand Down Expand Up @@ -201,14 +203,13 @@ pub enum LinkTarget {
}

impl Args {
pub fn new() -> Result<Self> {
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,
Expand Down Expand Up @@ -282,7 +283,7 @@ impl Args {
clap_args.test_run = true;
}

Ok(clap_args)
clap_args
}

pub fn get_tasks(&self) -> &[String] {
Expand Down Expand Up @@ -631,14 +632,15 @@ pub async fn run(
repo_state: Option<RepoState>,
#[allow(unused_variables)] logger: &TurboSubscriber,
ui: UI,
) -> Result<Payload> {
let mut cli_args = Args::new()?;
) -> Result<Payload, Error> {
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 = <Args as CommandFactory>::command();
let _ = cmd.print_help();
Expand All @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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?
Expand Down

0 comments on commit 9c5fa5b

Please sign in to comment.