diff --git a/CHANGELOG.md b/CHANGELOG.md index e40904ac44..987c3df969 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and its status can be checked out using `dfx canister migration-status`. The optimization functionality provided by `ic_wasm::optimize" cannot handle Wasm modules that contains 64-bit table. Instead of blocking the build, such optimization failure will issue a warning. +### fix: prevent panic on terminals with limited color support + +Fixed a panic that could occur when running `dfx` on terminals lacking color support. The `term` crate has been replaced with raw ANSI escape codes, and colors are now only emitted when stderr is a TTY and the `NO_COLOR` environment variable is not set. + ## Dependencies ### Motoko diff --git a/Cargo.lock b/Cargo.lock index 2833d17123..0b56ba8b97 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1720,7 +1720,6 @@ dependencies = [ "sysinfo", "tar", "tempfile", - "term", "thiserror 1.0.69", "time", "tokio", diff --git a/src/dfx/Cargo.toml b/src/dfx/Cargo.toml index 591c80076f..02501c19fa 100644 --- a/src/dfx/Cargo.toml +++ b/src/dfx/Cargo.toml @@ -107,7 +107,6 @@ supports-color = "2.1.0" sysinfo = "0.28.4" tar.workspace = true tempfile.workspace = true -term = "0.7.0" thiserror.workspace = true time = { workspace = true, features = ["macros", "serde", "serde-human-readable"] } tokio = { workspace = true, features = ["full"] } diff --git a/src/dfx/src/main.rs b/src/dfx/src/main.rs index 62808c269c..69bd20bbf3 100644 --- a/src/dfx/src/main.rs +++ b/src/dfx/src/main.rs @@ -15,6 +15,7 @@ use dfx_core::extension::manager::ExtensionManager; use indicatif::MultiProgress; use std::collections::HashMap; use std::ffi::OsString; +use std::io::{IsTerminal, Write, stderr}; use std::path::PathBuf; use std::time::Instant; use util::default_allowlisted_canisters; @@ -72,8 +73,19 @@ fn setup_logging(opts: &CliOpts) -> (i64, slog::Logger, MultiProgress) { (verbose_level, logger, spinners) } +/// Returns true if colored output should be used on stderr. +/// Respects the NO_COLOR environment variable (https://no-color.org/). +fn use_color() -> bool { + std::env::var_os("NO_COLOR").is_none() && stderr().is_terminal() +} + fn print_error_and_diagnosis(log_level: Option, err: Error, error_diagnosis: DiagnosedError) { - let mut stderr = util::stderr_wrapper::stderr_wrapper(); + let mut stderr = stderr().lock(); + let use_color = use_color(); + + const RED: &str = "\x1b[31m"; + const YELLOW: &str = "\x1b[33m"; + const RESET: &str = "\x1b[0m"; // print error chain stack if log_level.unwrap_or_default() > 0 // DEBUG or more verbose @@ -85,17 +97,15 @@ fn print_error_and_diagnosis(log_level: Option, err: Error, error_diagnosis } let (color, prefix) = if level == 0 { - (term::color::RED, "Error") + (RED, "Error") } else { - (term::color::YELLOW, "Caused by") + (YELLOW, "Caused by") }; - stderr - .fg(color) - .expect("Failed to set stderr output color."); - write!(stderr, "{prefix}: ").expect("Failed to write to stderr."); - stderr - .reset() - .expect("Failed to reset stderr output color."); + if use_color { + write!(stderr, "{color}{prefix}: {RESET}").expect("Failed to write to stderr."); + } else { + write!(stderr, "{prefix}: ").expect("Failed to write to stderr."); + } writeln!(stderr, "{cause}").expect("Failed to write to stderr."); } @@ -103,25 +113,19 @@ fn print_error_and_diagnosis(log_level: Option, err: Error, error_diagnosis // print diagnosis if let Some(explanation) = error_diagnosis.explanation { - stderr - .fg(term::color::RED) - .expect("Failed to set stderr output color."); - write!(stderr, "Error: ").expect("Failed to write to stderr."); - stderr - .reset() - .expect("Failed to reset stderr output color."); - + if use_color { + write!(stderr, "{RED}Error: {RESET}").expect("Failed to write to stderr."); + } else { + write!(stderr, "Error: ").expect("Failed to write to stderr."); + } writeln!(stderr, "{explanation}").expect("Failed to write to stderr."); } if let Some(action_suggestion) = error_diagnosis.action_suggestion { - stderr - .fg(term::color::YELLOW) - .expect("Failed to set stderr output color."); - write!(stderr, "To fix: ").expect("Failed to write to stderr."); - stderr - .reset() - .expect("Failed to reset stderr output color."); - + if use_color { + write!(stderr, "{YELLOW}To fix: {RESET}").expect("Failed to write to stderr."); + } else { + write!(stderr, "To fix: ").expect("Failed to write to stderr."); + } writeln!(stderr, "{action_suggestion}").expect("Failed to write to stderr."); } } diff --git a/src/dfx/src/util/mod.rs b/src/dfx/src/util/mod.rs index 3150c19320..0665063927 100644 --- a/src/dfx/src/util/mod.rs +++ b/src/dfx/src/util/mod.rs @@ -31,7 +31,6 @@ pub mod assets; pub mod clap; pub mod command; pub mod currency_conversion; -pub mod stderr_wrapper; pub mod url; const DECIMAL_POINT: char = '.'; diff --git a/src/dfx/src/util/stderr_wrapper.rs b/src/dfx/src/util/stderr_wrapper.rs deleted file mode 100644 index 12ab550e0b..0000000000 --- a/src/dfx/src/util/stderr_wrapper.rs +++ /dev/null @@ -1,83 +0,0 @@ -use term::{Error, StderrTerminal, Terminal}; - -/// Produces the standard term::StderrTerminal that can write colors. -/// If there is no such terminal available (such as on Github CI), this produces a stderr-wrapper that skips coloring. -pub fn stderr_wrapper() -> Box { - term::stderr().unwrap_or_else(|| { - Box::new(BasicStderr { - stderr: std::io::stderr(), - }) - }) -} - -struct BasicStderr { - stderr: W, -} - -impl Terminal for BasicStderr { - type Output = W; - fn fg(&mut self, _color: term::color::Color) -> term::Result<()> { - Ok(()) - } - - fn bg(&mut self, _color: term::color::Color) -> term::Result<()> { - Ok(()) - } - - fn attr(&mut self, _attr: term::Attr) -> term::Result<()> { - Ok(()) - } - - fn supports_attr(&self, _attr: term::Attr) -> bool { - true - } - - fn reset(&mut self) -> term::Result<()> { - Ok(()) - } - - fn supports_reset(&self) -> bool { - true - } - - fn supports_color(&self) -> bool { - true - } - - fn cursor_up(&mut self) -> term::Result<()> { - Err(Error::NotSupported) - } - - fn delete_line(&mut self) -> term::Result<()> { - Err(Error::NotSupported) - } - - fn carriage_return(&mut self) -> term::Result<()> { - Err(Error::NotSupported) - } - - fn get_ref(&self) -> &Self::Output { - &self.stderr - } - - fn get_mut(&mut self) -> &mut Self::Output { - &mut self.stderr - } - - fn into_inner(self) -> Self::Output - where - Self: Sized, - { - self.stderr - } -} - -impl std::io::Write for BasicStderr { - fn write(&mut self, buf: &[u8]) -> std::io::Result { - self.stderr.write(buf) - } - - fn flush(&mut self) -> std::io::Result<()> { - self.stderr.flush() - } -}