Skip to content

Commit

Permalink
fix: use error logger in Rust turborepo (vercel#3253)
Browse files Browse the repository at this point in the history
Moves messaging in Rust land to use the standard logger where
applicable:
 - clap errors will now be printed via the error logger
- `turborepo_lib::main` now logs any errors via the error logger instead
of having the caller decide how to handle the error

The following are other uses of standard (`println!/write!`) print
functionality that weren't changed:
 - clap help text
 - `turbo --version`
 - `turbo bin`


This PR does not change the Go side of error logging, but there should
only be a single place that needs updating:
[root.go](https://github.com/vercel/turbo/blob/main/cli/internal/cmd/root.go#L100)

Example of new output:
![Screen Shot 2023-01-10 at 1 18 43
PM](https://user-images.githubusercontent.com/4131117/211664782-02d149c3-61d8-469d-b0e3-1855bc66c102.png)

Co-authored-by: Thomas Knickman <[email protected]>
  • Loading branch information
chris-olszewski and tknickman authored Jan 12, 2023
1 parent 3442473 commit e143b19
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 7 deletions.
6 changes: 4 additions & 2 deletions cli/integration_tests/bad_flag.t
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,24 @@ Setup

Bad flag should print misuse text
$ ${TURBO} --bad-flag
error: Found argument '--bad-flag' which wasn't expected, or isn't valid in this context
ERROR Found argument '--bad-flag' which wasn't expected, or isn't valid in this context

If you tried to supply '--bad-flag' as a value rather than a flag, use '-- --bad-flag'

Usage: turbo [OPTIONS] [COMMAND]

For more information try '--help'

[1]

Bad flag with an implied run command should display run flags
$ ${TURBO} build --bad-flag
error: Found argument '--bad-flag' which wasn't expected, or isn't valid in this context
ERROR Found argument '--bad-flag' which wasn't expected, or isn't valid in this context

If you tried to supply '--bad-flag' as a value rather than a flag, use '-- --bad-flag'

Usage: turbo <--cache-dir <CACHE_DIR>|--cache-workers <CACHE_WORKERS>|--concurrency <CONCURRENCY>|--continue|--dry-run [<DRY_RUN>]|--single-package|--filter <FILTER>|--force|--global-deps <GLOBAL_DEPS>|--graph [<GRAPH>]|--ignore <IGNORE>|--include-dependencies|--no-cache|--no-daemon|--no-deps|--output-logs <OUTPUT_LOGS>|--only|--parallel|--profile <PROFILE>|--remote-only|--scope <SCOPE>|--since <SINCE>|TASKS|PASS_THROUGH_ARGS>

For more information try '--help'

[1]
3 changes: 2 additions & 1 deletion cli/integration_tests/basic_monorepo/verbosity.t
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,10 @@ Verbosity level 2

Make sure users can only use one verbosity flag
$ ${TURBO} build -v --verbosity=1
error: The argument '-v...' cannot be used with '--verbosity <COUNT>'
ERROR The argument '-v...' cannot be used with '--verbosity <COUNT>'

Usage: turbo [OPTIONS] [COMMAND]

For more information try '--help'

[1]
20 changes: 19 additions & 1 deletion crates/turborepo-lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::{env, io, mem, path::PathBuf, process};
use anyhow::{anyhow, Result};
use clap::{ArgAction, CommandFactory, Parser, Subcommand, ValueEnum};
use clap_complete::{generate, Shell};
use log::error;
use serde::Serialize;

use crate::{
Expand Down Expand Up @@ -150,10 +151,27 @@ impl Args {
pub fn new() -> Result<Self> {
let mut clap_args = match Args::try_parse() {
Ok(args) => args,
Err(e) if e.use_stderr() => {
// Don't use error logger when displaying help text
Err(e)
if matches!(
e.kind(),
clap::error::ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand
) =>
{
let _ = e.print();
process::exit(1);
}
Err(e) if e.use_stderr() => {
let err_str = e.to_string();
// A cleaner solution would be to implement our own clap::error::ErrorFormatter
// but that would require copying the default formatter just to remove this
// line: https://docs.rs/clap/latest/src/clap/error/format.rs.html#100
error!(
"{}",
err_str.strip_prefix("error: ").unwrap_or(err_str.as_str())
);
process::exit(1);
}
// If the clap error shouldn't be printed to stderr it indicates help text
Err(e) => {
let _ = e.print();
Expand Down
11 changes: 9 additions & 2 deletions crates/turborepo-lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mod package_manager;
mod shim;

use anyhow::Result;
use log::error;

pub use crate::cli::Args;
use crate::package_manager::PackageManager;
Expand All @@ -23,6 +24,12 @@ pub fn get_version() -> &'static str {
.0
}

pub fn main() -> Result<Payload> {
shim::run()
pub fn main() -> Payload {
match shim::run() {
Ok(payload) => payload,
Err(err) => {
error!("{}", err.to_string());
Payload::Rust(Err(err))
}
}
}
2 changes: 1 addition & 1 deletion crates/turborepo/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fn run_go_binary(args: Args) -> Result<i32> {
// This function should not expanded. Please add any logic to
// `turborepo_lib::main` instead
fn main() -> Result<()> {
let exit_code = match turborepo_lib::main()? {
let exit_code = match turborepo_lib::main() {
Payload::Rust(res) => res.unwrap_or(1),
Payload::Go(state) => run_go_binary(*state)?,
};
Expand Down

0 comments on commit e143b19

Please sign in to comment.