-
Notifications
You must be signed in to change notification settings - Fork 960
feat(error): display colored error labels on TTY stderr #466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@googleworkspace/cli": minor | ||
| --- | ||
|
|
||
| Display colored error and warning labels on TTY stderr |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -148,32 +148,66 @@ impl GwsError { | |||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /// Returns true when stderr is connected to an interactive terminal, | ||||||||||||||||||||||||||||||||||
| /// meaning ANSI color codes will be visible to the user. | ||||||||||||||||||||||||||||||||||
| fn stderr_supports_color() -> bool { | ||||||||||||||||||||||||||||||||||
| use std::io::IsTerminal; | ||||||||||||||||||||||||||||||||||
| std::io::stderr().is_terminal() && std::env::var_os("NO_COLOR").is_none() | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /// Wrap `text` in ANSI bold + the given color code, resetting afterwards. | ||||||||||||||||||||||||||||||||||
| /// Returns the plain text unchanged when stderr is not a TTY. | ||||||||||||||||||||||||||||||||||
| fn colorize(text: &str, ansi_color: &str) -> String { | ||||||||||||||||||||||||||||||||||
| if stderr_supports_color() { | ||||||||||||||||||||||||||||||||||
| format!("\x1b[1;{ansi_color}m{text}\x1b[0m") | ||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||
| text.to_string() | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /// Format a colored error label for the given error variant. | ||||||||||||||||||||||||||||||||||
| fn error_label(err: &GwsError) -> String { | ||||||||||||||||||||||||||||||||||
| match err { | ||||||||||||||||||||||||||||||||||
| GwsError::Api { .. } => colorize("error[api]:", "31"), // red | ||||||||||||||||||||||||||||||||||
| GwsError::Auth(_) => colorize("error[auth]:", "31"), // red | ||||||||||||||||||||||||||||||||||
| GwsError::Validation(_) => colorize("error[validation]:", "33"), // yellow | ||||||||||||||||||||||||||||||||||
| GwsError::Discovery(_) => colorize("error[discovery]:", "31"), // red | ||||||||||||||||||||||||||||||||||
| GwsError::Other(_) => colorize("error:", "31"), // red | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /// Formats any error as a JSON object and prints to stdout. | ||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||
| /// For `accessNotConfigured` errors (HTTP 403, reason `accessNotConfigured`), | ||||||||||||||||||||||||||||||||||
| /// additional human-readable guidance is printed to stderr explaining how to | ||||||||||||||||||||||||||||||||||
| /// enable the API in GCP. The JSON output on stdout is unchanged (machine-readable). | ||||||||||||||||||||||||||||||||||
| /// A human-readable colored label is printed to stderr when connected to a | ||||||||||||||||||||||||||||||||||
| /// TTY. For `accessNotConfigured` errors (HTTP 403, reason | ||||||||||||||||||||||||||||||||||
| /// `accessNotConfigured`), additional guidance is printed to stderr. | ||||||||||||||||||||||||||||||||||
| /// The JSON output on stdout is unchanged (machine-readable). | ||||||||||||||||||||||||||||||||||
| pub fn print_error_json(err: &GwsError) { | ||||||||||||||||||||||||||||||||||
| let json = err.to_json(); | ||||||||||||||||||||||||||||||||||
| println!( | ||||||||||||||||||||||||||||||||||
| "{}", | ||||||||||||||||||||||||||||||||||
| serde_json::to_string_pretty(&json).unwrap_or_default() | ||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Print a colored summary line to stderr so humans can quickly scan | ||||||||||||||||||||||||||||||||||
| // the error type without parsing JSON. | ||||||||||||||||||||||||||||||||||
| eprintln!("{} {}", error_label(err), err); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Print actionable guidance to stderr for accessNotConfigured errors | ||||||||||||||||||||||||||||||||||
| if let GwsError::Api { | ||||||||||||||||||||||||||||||||||
| reason, enable_url, .. | ||||||||||||||||||||||||||||||||||
| } = err | ||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||
| if reason == "accessNotConfigured" { | ||||||||||||||||||||||||||||||||||
| eprintln!(); | ||||||||||||||||||||||||||||||||||
| eprintln!("💡 API not enabled for your GCP project."); | ||||||||||||||||||||||||||||||||||
| let hint = colorize("hint:", "36"); // cyan | ||||||||||||||||||||||||||||||||||
| eprintln!("{hint} API not enabled for your GCP project."); | ||||||||||||||||||||||||||||||||||
| if let Some(url) = enable_url { | ||||||||||||||||||||||||||||||||||
| eprintln!(" Enable it at: {url}"); | ||||||||||||||||||||||||||||||||||
| eprintln!(" Enable it at: {url}"); | ||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||
| eprintln!(" Visit the GCP Console → APIs & Services → Library to enable the required API."); | ||||||||||||||||||||||||||||||||||
| eprintln!(" Visit the GCP Console → APIs & Services → Library to enable the required API."); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| eprintln!(" After enabling, wait a few seconds and retry your command."); | ||||||||||||||||||||||||||||||||||
| eprintln!(" After enabling, wait a few seconds and retry your command."); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
@@ -327,4 +361,42 @@ mod tests { | |||||||||||||||||||||||||||||||||
| // enable_url key should not appear in JSON when None | ||||||||||||||||||||||||||||||||||
| assert!(json["error"]["enable_url"].is_null()); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // --- colored output tests --- | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||||||||||||
| #[serial_test::serial] | ||||||||||||||||||||||||||||||||||
| fn test_colorize_respects_no_color_env() { | ||||||||||||||||||||||||||||||||||
| // NO_COLOR is the de-facto standard for disabling colors. | ||||||||||||||||||||||||||||||||||
| // When set, colorize() should return the plain text. | ||||||||||||||||||||||||||||||||||
| // Uses #[serial] because it modifies a global env var. | ||||||||||||||||||||||||||||||||||
| std::env::set_var("NO_COLOR", "1"); | ||||||||||||||||||||||||||||||||||
| let result = colorize("hello", "31"); | ||||||||||||||||||||||||||||||||||
| std::env::remove_var("NO_COLOR"); | ||||||||||||||||||||||||||||||||||
| assert_eq!(result, "hello"); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+373
to
+376
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test modifies a global environment variable, which can lead to flaky tests if not handled carefully. If
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah good point, though in practice colorize() is a simple string format that won't panic. the test also runs in a single thread with serial_test so no parallel interference. catch_unwind feels like overkill here but happy to add if the maintainer wants it.
Comment on lines
+373
to
+376
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test modifies a global environment variable, which can lead to flaky tests if not handled carefully. If the call to To make this test more robust, you can wrap the fallible operation in
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above — colorize is trivial and won't panic, but can add the guard if preferred |
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
anshul-garg27 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||||||||||||
| fn test_error_label_contains_variant_name() { | ||||||||||||||||||||||||||||||||||
| let api_err = GwsError::Api { | ||||||||||||||||||||||||||||||||||
| code: 400, | ||||||||||||||||||||||||||||||||||
| message: "bad".to_string(), | ||||||||||||||||||||||||||||||||||
| reason: "r".to_string(), | ||||||||||||||||||||||||||||||||||
| enable_url: None, | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
| let label = error_label(&api_err); | ||||||||||||||||||||||||||||||||||
| assert!(label.contains("error[api]:")); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| let auth_err = GwsError::Auth("fail".to_string()); | ||||||||||||||||||||||||||||||||||
| assert!(error_label(&auth_err).contains("error[auth]:")); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| let val_err = GwsError::Validation("bad input".to_string()); | ||||||||||||||||||||||||||||||||||
| assert!(error_label(&val_err).contains("error[validation]:")); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| let disc_err = GwsError::Discovery("missing".to_string()); | ||||||||||||||||||||||||||||||||||
| assert!(error_label(&disc_err).contains("error[discovery]:")); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| let other_err = GwsError::Other(anyhow::anyhow!("oops")); | ||||||||||||||||||||||||||||||||||
| assert!(error_label(&other_err).contains("error:")); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.