[TRUNK-17384] Change default for errors to not give failure code

cli/Cargo.toml

@@ -25,6 +25,8 @@ third-party = { path = "../third-party" }
 display = { path = "../display" }
 log = "0.4.14"
 exitcode = "1.1.1"
+gix = { version = "0.74.0", default-features = false, features = [
+], optional = true }
 http = "1.1.0"
 tokio = { version = "*", default-features = false, features = [
   "rt-multi-thread",
@@ -80,3 +82,5 @@ vergen = { version = "8.3.1", features = [
 
 [features]
 force-sentry-env-dev = []
+default = ["git-access"]
+git-access = ["dep:gix"]

cli/src/context.rs

@@ -42,6 +42,7 @@ use tempfile::TempDir;
 #[cfg(target_os = "macos")]
 use xcresult::xcresult::XCResult;
 
+use crate::error_report::InterruptingError;
 use crate::{
     context_quarantine::{
         FailedTestsExtractor, QuarantineContext, QuarantineFetchStatus, gather_quarantine_context,
@@ -216,7 +217,9 @@ pub fn gather_post_test_context<U: AsRef<Path>>(
     )?;
 
     if !allow_empty_test_results && file_set_builder.no_files_found() {
-        return Err(anyhow::anyhow!("No test output files found to upload."));
+        return Err(anyhow::anyhow!(InterruptingError::new(
+            "No test output files found to upload."
+        )));
     }
 
     tracing::info!("Total files pack and upload: {}", file_set_builder.count());

cli/src/error_report.rs

@@ -2,8 +2,8 @@ use api::client::get_api_host;
 use display::end_output::EndOutput;
 use http::StatusCode;
 use superconsole::{
-    style::{style, Attribute, Stylize},
     Line, Span,
+    style::{Attribute, Stylize, style},
 };
 
 const HELP_TEXT: &str = "For more help, contact us at https://slack.trunk.io/";
@@ -12,6 +12,8 @@ const CONNECTION_REFUSED_CONTEXT: &str = concat!("Unable to connect to trunk's s
 pub(crate) const UNAUTHORIZED_CONTEXT: &str =
     concat!("Unathorized access, your Trunk organization URL slug or token may be incorrect",);
 
+const GIX_ERROR_CONTEXT: &str = "Unable to open git repository";
+
 fn add_settings_url_to_context(domain: String, org_url_slug: &String) -> String {
     let settings_url = format!("{}/{}/settings", domain.replace("api", "app"), org_url_slug);
     format!(
@@ -20,10 +22,32 @@ fn add_settings_url_to_context(domain: String, org_url_slug: &String) -> String
     )
 }
 
+pub struct InterruptingError {
+    message: String,
+}
+impl core::fmt::Display for InterruptingError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.message)
+    }
+}
+impl core::fmt::Debug for InterruptingError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{:?}", self.message)
+    }
+}
+impl std::error::Error for InterruptingError {}
+impl InterruptingError {
+    pub fn new<T: AsRef<str>>(message: T) -> Self {
+        Self {
+            message: message.as_ref().into(),
+        }
+    }
+}
+
 pub struct Context {
     pub base_message: Option<String>,
     pub org_url_slug: String,
-    pub exit_code: i32,
+    pub exit_code: Option<i32>,
 }
 
 pub struct ErrorReport {
@@ -43,29 +67,44 @@ impl ErrorReport {
         }
     }
 
-    fn find_exit_code(error: &anyhow::Error) -> i32 {
+    fn find_exit_code(error: &anyhow::Error) -> Option<i32> {
         if is_connection_refused(error) {
             tracing::warn!(CONNECTION_REFUSED_CONTEXT);
-            return exitcode::OK;
+            return None;
         }
 
         if is_unauthorized(error) {
             tracing::warn!(UNAUTHORIZED_CONTEXT);
-            return exitcode::SOFTWARE;
+            return Some(exitcode::SOFTWARE);
+        }
+
+        if is_gix_error(error) {
+            tracing::warn!(GIX_ERROR_CONTEXT);
+            return Some(exitcode::SOFTWARE);
+        }
+
+        if let Some(message) = get_interrupting_message(error) {
+            tracing::warn!("{}", message);
+            return Some(exitcode::SOFTWARE);
         }
 
         tracing::error!("{}", error);
         tracing::error!(hidden_in_console = true, "Caused by error: {:#?}", error);
-        exitcode::SOFTWARE
+        None
+    }
+
+    pub fn should_block_quarantining(&self) -> bool {
+        is_unauthorized(&self.error)
+            || is_gix_error(&self.error)
+            || get_interrupting_message(&self.error).is_some()
     }
 }
 
 fn is_connection_refused(error: &anyhow::Error) -> bool {
-    if let Some(io_error) = error.root_cause().downcast_ref::<std::io::Error>() {
-        io_error.kind() == std::io::ErrorKind::ConnectionRefused
-    } else {
-        false
-    }
+    error
+        .root_cause()
+        .downcast_ref::<std::io::Error>()
+        .is_some()
 }
 
 fn is_unauthorized(error: &anyhow::Error) -> bool {
@@ -82,6 +121,29 @@ fn is_unauthorized(error: &anyhow::Error) -> bool {
     }
 }
 
+fn is_gix_error(error: &anyhow::Error) -> bool {
+    #[cfg(feature = "git-access")]
+    {
+        for cause in error.chain() {
+            if cause.downcast_ref::<gix::open::Error>().is_some() {
+                return true;
+            }
+        }
+        false
+    }
+    #[cfg(not(feature = "git-access"))]
+    {
+        false
+    }
+}
+
+fn get_interrupting_message(error: &anyhow::Error) -> Option<String> {
+    error
+        .root_cause()
+        .downcast_ref::<InterruptingError>()
+        .map(|e| e.message.clone())
+}
+
 impl EndOutput for ErrorReport {
     fn output(&self) -> anyhow::Result<Vec<Line>> {
         let Context {
@@ -135,23 +197,34 @@ impl EndOutput for ErrorReport {
         }
         lines.push(Line::from_iter([Span::new_unstyled(HELP_TEXT)?]));
         lines.push(Line::default());
-        if exit_code == &exitcode::OK {
-            lines.push(Line::from_iter([
+        match exit_code {
+            Some(exitcode::OK) => lines.push(Line::from_iter([
                 Span::new_unstyled("No errors occurred, returning default exit code: ")?,
-                Span::new_styled(style(exit_code.to_string()).attribute(Attribute::Bold))?,
-            ]));
-        } else if exit_code == &exitcode::SOFTWARE {
-            // SOFTWARE is used to indicate that the upload command failed
-            lines.push(Line::from_iter([
-                Span::new_unstyled("Errors occurred during execution, returning exit code: ")?,
-                Span::new_styled(style(exit_code.to_string()).attribute(Attribute::Bold))?,
-            ]));
-        } else {
-            // Should be an unused codepath, but we log it for completeness
-            lines.push(Line::from_iter([
-                Span::new_unstyled("Errors occurred during execution, returning exit code: ")?,
-                Span::new_styled(style(exit_code.to_string()).attribute(Attribute::Bold))?,
-            ]));
+                Span::new_styled(style(exitcode::OK.to_string()).attribute(Attribute::Bold))?,
+            ])),
+            Some(exitcode::SOFTWARE) => {
+                // SOFTWARE is used to indicate that the upload command failed due to user error
+                lines.push(Line::from_iter([
+                    Span::new_unstyled("Errors occurred during execution, returning exit code: ")?,
+                    Span::new_styled(
+                        style(exitcode::SOFTWARE.to_string()).attribute(Attribute::Bold),
+                    )?,
+                ]));
+            }
+            Some(other_code) => {
+                // Should be an unused codepath, but we log it for completeness
+                lines.push(Line::from_iter([
+                    Span::new_unstyled("Errors occurred during execution, returning exit code: ")?,
+                    Span::new_styled(style(other_code.to_string()).attribute(Attribute::Bold))?,
+                ]));
+            }
+            None => {
+                // If uploads fail because trunk is down, we fall back to whatever came out of quarantining
+                // to minimize customer impact
+                lines.push(Line::from_iter([Span::new_unstyled(
+                    "Errors occurred during execution, using quarantining exit code",
+                )?]));
+            }
         }
         Ok(lines)
     }

cli/src/main.rs

@@ -136,6 +136,7 @@ fn main() -> anyhow::Result<()> {
                         .error_report
                         .as_ref()
                         .map(|e| e.context.exit_code)
+                        .flatten()
                         .unwrap_or(result_ptr.quarantine_context.exit_code);
                     close_out_and_exit(exit_code, guard, render_sender, render_handle)
                 }
@@ -162,7 +163,7 @@ fn main() -> anyhow::Result<()> {
                 }
                 Err(error) => {
                     let error_report = ErrorReport::new(error, org_url_slug, None);
-                    let exit_code = error_report.context.exit_code;
+                    let exit_code = error_report.context.exit_code.unwrap_or(exitcode::OK);
                     send_message(
                         DisplayMessage::Final(
                             Arc::new(error_report),

cli/src/upload_command.rs

@@ -454,7 +454,7 @@
         .quarantine_results
         .clone();
 
     // trunk-ignore(clippy/assigning_clones)
     meta.failed_tests = quarantine_context.failures.clone();
 
     let upload_started_at = chrono::Utc::now();
@@ -588,7 +588,14 @@
         if let Some(error_report) = self.error_report.as_ref() {
             output.push(Line::default());
             output.extend(error_report.output()?);
-            return Ok(output);
+            if error_report.should_block_quarantining() {
+                return Ok(output);
+            } else {
+                output.push(Line::from_iter([Span::new_unstyled(
+                    "Proceeding with test report...",
+                )?]));
+                output.push(Line::default());
+            }
         }
 
         // Add the bundle upload ID message

cli/src/validate_command.rs

@@ -29,7 +29,10 @@ use superconsole::{
     style::{Attribute, Color, Stylize, style},
 };
 
-use crate::{context::fall_back_to_binary_parse, report_limiting::ValidationReport};
+use crate::{
+    context::fall_back_to_binary_parse, error_report::InterruptingError,
+    report_limiting::ValidationReport,
+};
 
 #[derive(Args, Clone, Debug)]
 pub struct ValidateArgs {
@@ -522,7 +525,7 @@ async fn validate(
     if file_set_builder.no_files_found() {
         let msg = "No test output files found to validate";
         tracing::warn!(msg);
-        return Err(anyhow::anyhow!(msg));
+        return Err(anyhow::anyhow!(InterruptingError::new(msg)));
     }
     let file_set_results = gen_file_set_results(&file_set_builder);