dev (#27)

* Cargo hack for checking all features properly in qa

* StdCapture context manager python, panic_on_err_async macro, bash runner CmdOut contains attempted commands and a pretty formatting method of those commands to see what caused a failure, plus CmdOut is attached to BashErr errors coming out too

.github/workflows/release.yml

@@ -92,7 +92,8 @@ jobs:
         if: ${{ inputs.py_rust_release }} || ${{ inputs.rust_release }}
         with:
           components: rustfmt, clippy
-      - name: Install cargo-hack, used for feature checking in pre-commit.
+
+      - name: "Install cargo-hack, used for feature checking in pre-commit."
         if: ${{ inputs.py_rust_release }} || ${{ inputs.rust_release }}
         uses: taiki-e/install-action@cargo-hack
 

opencollector.yaml

@@ -32,7 +32,7 @@ exporters:
       stream-name: default
   # Writes all opentelemetry logs, traces, metrics to a file, useful for testing:
   file/debug_file_writing:
-    path: /home/runner/work/bitbazaar/bitbazaar/logs/otlp_telemetry_out.log
+    path: /Users/zak/z/code/bitbazaar/logs/otlp_telemetry_out.log
     rotation:
       max_megabytes: 10
       max_days: 3

py/bitbazaar/misc/__init__.py

@@ -1,7 +1,8 @@
 """Miscellaneous utility functions for BitBazaar."""
 
+from ._std_capture import StdCapture
 
-__all__ = ["copy_sig"]
+__all__ = ["copy_sig", "StdCapture"]
 
 import os
 import socket

py/bitbazaar/misc/_std_capture.py

@@ -0,0 +1,53 @@
+import io
+import sys
+import typing as tp
+
+
+class StdCapture(list):
+    r"""Capture stdout/stderr for the duration of a with block.
+
+    (e.g. print statements)
+
+    Example:
+    ```python
+    with StdCapture(stderr=True) as out: # By default only captures stdout
+        print('hello')
+        sys.stderr.write('world')
+    print(out)  # ['hello', 'world']
+    ```
+    """
+
+    _stderr_capture: bool
+    _stdout: "tp.TextIO"
+    _stderr: "tp.TextIO"
+    _buf: "io.StringIO"
+    _out: "list[str]"
+
+    def __init__(self, stderr: bool = False):
+        """Creation of new capturer.
+
+        By default only stdout is captured, stderr=True enables capturing stderr too.
+        """
+        # Prep all instance vars:
+        self.stderr_capture = stderr
+        self._out = []
+        self._stdout = sys.stdout
+        self._stderr = sys.stderr
+        self._buf = io.StringIO()
+
+    def __enter__(self) -> list[str]:
+        """Entering the capturing context."""
+        # Overwrite sinks which are being captured with the buffer:
+        sys.stdout = self._buf
+        if self.stderr_capture:
+            sys.stderr = self._buf
+
+        return self._out
+
+    def __exit__(self, *args):  # type: ignore
+        """On context exit."""
+        # First reset the global streams:
+        sys.stdout = self._stdout
+        sys.stderr = self._stderr
+
+        self._out.extend(self._buf.getvalue().splitlines())

py/tests/misc/test_std_capture.py

@@ -0,0 +1,33 @@
+import sys
+
+from bitbazaar.misc import StdCapture
+
+
+def test_std_capture():
+    orig_stdout = sys.stdout
+    orig_stderr = sys.stderr
+
+    # Confirm the example works:
+    with StdCapture(stderr=True) as out:  # By default only captures stdout
+        print("hello")
+        sys.stderr.write("world")
+    assert out == ["hello", "world"]
+
+    # Confirm no stderr captured if not requested:
+    with StdCapture() as out:
+        sys.stderr.write("world")
+    assert out == []
+
+    # Confirm returned to originals:
+    assert sys.stdout is orig_stdout
+    assert sys.stderr is orig_stderr
+
+    # Confirm would also return if error occurred inside block:
+    try:
+        with StdCapture(stderr=True):
+            raise ValueError("error")
+    except ValueError:
+        pass
+
+    assert sys.stdout is orig_stdout
+    assert sys.stderr is orig_stderr

rust/Cargo.toml

@@ -117,6 +117,7 @@ tokio = { version = '1.35', features = ["rt-multi-thread", "macros"] }
 uuid = { version = "1.6", features = ["v4"] }
 regex = "1"
 serde_json = "1"
+futures = "0.3"
 
 [profile.profiler]
 inherits = "release" # Adds on top of the default release profile

rust/bitbazaar/cli/bash.rs

@@ -3,8 +3,6 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use conch_parser::{lexer::Lexer, parse::DefaultParser};
-
 use super::{errs::ShellErr, shell::Shell, BashErr, CmdOut};
 use crate::prelude::*;
 
@@ -61,9 +59,9 @@ impl Bash {
         }
     }
 
-    /// Add a new command to the bash script.
+    /// Add a new piece of logic to the bash script. E.g. a line of bash.
     ///
-    /// Multiple added commands will be treated as e.g. lines in a bash script.
+    /// Multiple commands added to a [`Bash`] instance will be treated as newline separated.
     pub fn cmd(self, cmd: impl Into<String>) -> Self {
         let mut cmds = self.cmds;
         cmds.push(cmd.into());
@@ -99,33 +97,34 @@ impl Bash {
     /// Execute the current contents of the bash script.
     pub fn run(self) -> Result<CmdOut, BashErr> {
         if self.cmds.is_empty() {
-            return Ok(CmdOut {
-                stdout: "".to_string(),
-                stderr: "".to_string(),
-                code: 0,
-            });
+            return Ok(CmdOut::empty());
         }
 
-        let cmd_str = self.cmds.join("\n");
-        let lex = Lexer::new(cmd_str.chars());
-        let parser = DefaultParser::new(lex);
-
-        let top_cmds = parser
-            .into_iter()
-            .collect::<core::result::Result<Vec<_>, _>>()
-            .change_context(BashErr::BashSyntaxError)?;
+        let mut shell = Shell::new(self.env_vars, self.root_dir)
+            .map_err(|e| shell_to_bash_err(CmdOut::empty(), e))?;
 
-        match Shell::exec(self.root_dir.as_deref(), self.env_vars, top_cmds) {
-            Ok(cmd_out) => Ok(cmd_out),
-            Err(e) => match e.current_context() {
-                ShellErr::Exit => Err(e.change_context(BashErr::InternalError).attach_printable(
-                    "Exit's should be handled and transformed internally in Shell::exec.",
-                )),
-                ShellErr::InternalError => Err(e.change_context(BashErr::InternalError)),
-                ShellErr::BashFeatureUnsupported => {
-                    Err(e.change_context(BashErr::BashFeatureUnsupported))
-                }
-            },
+        if let Err(e) = shell.execute_command_strings(self.cmds) {
+            return Err(shell_to_bash_err(shell.into(), e));
         }
+
+        Ok(shell.into())
+    }
+}
+
+fn shell_to_bash_err(
+    mut cmd_out: CmdOut,
+    e: error_stack::Report<ShellErr>,
+) -> error_stack::Report<BashErr> {
+    // Doesn't really make sense, but set the exit code to 1 if 0, as technically the command errored even though it was the runner itself that errored and the command might not have been attempted.
+    if cmd_out.code == 0 {
+        cmd_out.code = 1;
+    }
+    match e.current_context() {
+        ShellErr::Exit => e.change_context(BashErr::InternalError(cmd_out)).attach_printable(
+            "Shouldn't occur, shell exit errors should have been managed internally, not an external error.",
+        ),
+        ShellErr::InternalError => e.change_context(BashErr::InternalError(cmd_out)),
+        ShellErr::BashFeatureUnsupported => e.change_context(BashErr::BashFeatureUnsupported(cmd_out)),
+        ShellErr::BashSyntaxError => e.change_context(BashErr::BashSyntaxError(cmd_out)),
     }
 }

rust/bitbazaar/cli/builtins/cd.rs

@@ -82,11 +82,7 @@ pub fn cd(shell: &mut Shell, args: &[String]) -> Result<CmdOut, BuiltinErr> {
         .chdir(target_path)
         .change_context(BuiltinErr::InternalError)?;
 
-    Ok(CmdOut {
-        stdout: "".to_string(),
-        stderr: "".to_string(),
-        code: 0,
-    })
+    Ok(CmdOut::empty())
 }
 
 // Should be tested quite well in cli/mod.rs and other builtin tests.

rust/bitbazaar/cli/builtins/echo.rs

@@ -38,6 +38,7 @@ pub fn echo(_shell: &mut Shell, args: &[String]) -> Result<CmdOut, BuiltinErr> {
         stdout,
         stderr: "".to_string(),
         code: 0,
+        attempted_commands: vec![], // This is a top level attribute, in theory should have a different struct for internal.
     })
 }
 

rust/bitbazaar/cli/builtins/mod.rs

@@ -24,6 +24,7 @@ macro_rules! bad_call {
             stdout: "".to_string(),
             stderr: format!($($arg)*),
             code: 1,
+            attempted_commands: vec![], // This is a top level attribute, in theory should have a different struct for internal.
         })
     };
 }
@@ -50,5 +51,6 @@ fn std_err_echo(_shell: &mut Shell, args: &[String]) -> Result<CmdOut, BuiltinEr
         stdout: "".to_string(),
         stderr: args.join(" "),
         code: 0,
+        attempted_commands: vec![], // This is a top level attribute, in theory should have a different struct for internal.
     })
 }

rust/bitbazaar/cli/builtins/pwd.rs

@@ -23,6 +23,7 @@ pub fn pwd(shell: &mut Shell, args: &[String]) -> Result<CmdOut, BuiltinErr> {
         stdout: format!("{}\n", pwd),
         stderr: "".to_string(),
         code: 0,
+        attempted_commands: vec![], // This is a top level attribute, in theory should have a different struct for internal.
     })
 }
 

rust/bitbazaar/cli/builtins/set.rs

@@ -9,19 +9,11 @@ pub fn set(shell: &mut Shell, args: &[String]) -> Result<CmdOut, BuiltinErr> {
         match arg.as_str() {
             "+e" => {
                 shell.set_e = false;
-                return Ok(CmdOut {
-                    stdout: "".to_string(),
-                    stderr: "".to_string(),
-                    code: 0,
-                });
+                return Ok(CmdOut::empty());
             }
             "-e" => {
                 shell.set_e = true;
-                return Ok(CmdOut {
-                    stdout: "".to_string(),
-                    stderr: "".to_string(),
-                    code: 0,
-                });
+                return Ok(CmdOut::empty());
             }
             _ => {}
         }

rust/bitbazaar/cli/cmd_out.rs

@@ -7,9 +7,23 @@ pub struct CmdOut {
     pub stderr: String,
     /// The exit code of the command:
     pub code: i32,
+
+    /// The commands that were run, will include all commands that were attempted.
+    /// I.e. if a command fails, it will be the last command in this vec, the remaining were not attempted.
+    pub attempted_commands: Vec<String>,
 }
 
 impl CmdOut {
+    /// Create a new CmdOut with empty stdout, stderr, and a zero exit code.
+    pub(crate) fn empty() -> Self {
+        Self {
+            stdout: "".to_string(),
+            stderr: "".to_string(),
+            code: 0,
+            attempted_commands: Vec::new(),
+        }
+    }
+
     /// Returns true when the command exited with a zero exit code.
     pub fn success(&self) -> bool {
         self.code == 0
@@ -29,4 +43,27 @@ impl CmdOut {
             self.stderr.clone()
         }
     }
+
+    /// Pretty format the attempted commands, with the exit code included on the final line.
+    pub fn fmt_attempted_commands(&self) -> String {
+        if !self.attempted_commands.is_empty() {
+            let mut out = "Attempted commands:\n".to_string();
+            for (index, cmd) in self.attempted_commands.iter().enumerate() {
+                // Indent the commands by a bit of whitespace:
+                out.push_str("   ");
+                // Add cmd number:
+                out.push_str(format!("{}. ", index).as_str());
+                out.push_str(cmd.trim());
+                // Newline if not last:
+                if index < self.attempted_commands.len() - 1 {
+                    out.push('\n');
+                }
+            }
+            // On the last line, add <-- exited with code: X
+            out.push_str(&format!(" <-- exited with code: {}", self.code));
+            out
+        } else {
+            "No commands!".to_string()
+        }
+    }
 }

rust/bitbazaar/cli/errs.rs

@@ -1,21 +1,42 @@
+use super::CmdOut;
+
 /// User facing error type for Bash functionality.
-#[derive(Debug, strum::Display)]
+#[derive(Debug)]
 pub enum BashErr {
     /// BashSyntaxError
-    #[strum(serialize = "BashSyntaxError: couldn't parse bash script.")]
-    BashSyntaxError,
+    BashSyntaxError(CmdOut),
 
     /// BashFeatureUnsupported
-    #[strum(
-        serialize = "BashFeatureUnsupported: feature in script is valid bash, but unsupported."
-    )]
-    BashFeatureUnsupported,
+    BashFeatureUnsupported(CmdOut),
 
     /// InternalError
-    #[strum(
-        serialize = "InternalError: this shouldn't occur, open an issue at https://github.com/zakstucke/bitbazaar/issues"
-    )]
-    InternalError,
+    InternalError(CmdOut),
+}
+
+impl BashErr {
+    /// Get the CmdOut from the error.
+    pub fn cmd_out(&self) -> &CmdOut {
+        match self {
+            BashErr::BashSyntaxError(cmd_out) => cmd_out,
+            BashErr::BashFeatureUnsupported(cmd_out) => cmd_out,
+            BashErr::InternalError(cmd_out) => cmd_out,
+        }
+    }
+}
+
+impl std::fmt::Display for BashErr {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            BashErr::BashSyntaxError(cmd_out) => write!(f, "BashSyntaxError: couldn't parse bash script.\n{}", cmd_out.fmt_attempted_commands()),
+            BashErr::BashFeatureUnsupported(cmd_out) => {
+                write!(f, "BashFeatureUnsupported: feature in script is valid bash, but unsupported.\n{}", cmd_out.fmt_attempted_commands())
+            }
+            BashErr::InternalError(cmd_out) => write!(
+                f,
+                "InternalError: this shouldn't occur, open an issue at https://github.com/zakstucke/bitbazaar/issues\n{}", cmd_out.fmt_attempted_commands()
+            ),
+        }
+    }
 }
 
 impl error_stack::Context for BashErr {}
@@ -24,6 +45,7 @@ impl error_stack::Context for BashErr {}
 #[derive(Debug, strum::Display)]
 pub enum ShellErr {
     BashFeatureUnsupported,
+    BashSyntaxError,
     Exit,
     InternalError,
 }