fixing tests and revising logic for exception handling

Sources/Formic/CommandOutput.swift

@@ -52,6 +52,12 @@ public struct CommandOutput: Sendable {
     public static func generalFailure(msg: String) -> CommandOutput {
         CommandOutput(returnCode: -1, stdOut: nil, stdErr: msg.data(using: .utf8))
     }
+
+    /// Creates a command out that represents an exception thrown failure, and has no output.
+    public static func exceptionFailure() -> CommandOutput {
+        CommandOutput(returnCode: -1, stdOut: nil, stdErr: nil)
+    }
+
 }
 
 extension CommandOutput: Hashable {}

Sources/Formic/Engine/CommandExecutionResult.swift

@@ -11,7 +11,7 @@ public struct CommandExecutionResult: Sendable {
     /// The number of retries needed for the command.
     public let retries: Int
     /// The description of the exception thrown while invoking the command, if any.
-    public let exception: String?
+    public let exception: (any Error)?
 
     /// Creates an annotated command execution result.
     /// - Parameters:
@@ -23,7 +23,7 @@ public struct CommandExecutionResult: Sendable {
     ///   - exception: The description of the exception thrown while invoking the command, if any.
     public init(
         command: any Command, host: Host, output: CommandOutput, duration: Duration,
-        retries: Int, exception: String?
+        retries: Int, exception: (any Error)?
     ) {
         self.command = command
         self.host = host
@@ -163,7 +163,7 @@ extension CommandExecutionResult: Equatable {
     public static func == (lhs: CommandExecutionResult, rhs: CommandExecutionResult) -> Bool {
         lhs.command.id == rhs.command.id && lhs.host == rhs.host
             && lhs.output == rhs.output && lhs.duration == rhs.duration && lhs.retries == rhs.retries
-            && lhs.exception == rhs.exception
+            && lhs.exception?.localizedDescription == rhs.exception?.localizedDescription
     }
 }
 
@@ -177,6 +177,6 @@ extension CommandExecutionResult: Hashable {
         hasher.combine(output)
         hasher.combine(duration)
         hasher.combine(retries)
-        hasher.combine(exception)
+        hasher.combine(exception?.localizedDescription)
     }
 }

Sources/Formic/Engine/Engine.swift

@@ -32,15 +32,18 @@ public actor Engine {
     ) async throws -> [CommandExecutionResult] {
         var results: [CommandExecutionResult] = []
         for command in commands {
+            //print("running command: \(command)")
             let result = try await run(host: host, command: command)
             results.append(result)
             if displayProgress {
                 print(result.consoleOutput(verbosity: verbosity))
             }
             if result.representsFailure() {
+                //print("result: \(result) represents failure - breaking")
                 break
             }
         }
+        print("returning \(results.count) CEResults")
         return results
     }
 
@@ -85,8 +88,10 @@ public actor Engine {
         var numberOfRetries: Int = -1
         var durationOfLastAttempt: Duration = .zero
         var outputOfLastAttempt: CommandOutput = .empty
+        var capturedException: (any Error)? = nil
 
         repeat {
+            capturedException = nil
             numberOfRetries += 1
             let start = clock.now
             do {
@@ -111,18 +116,23 @@ public actor Engine {
             } catch {
                 // catch inner exception conditions and treat as a failure to allow for
                 // retries and timeouts to be handled.
-                print("Caught error while executing command: \(command)")
-                print("error: \(error)")
-                outputOfLastAttempt = .generalFailure(msg: error.localizedDescription)
+                capturedException = error
+                // mark as a failure due to the exception capture - if not marked
+                // as a failure explicitly, .empty is assumed to be a success.
+                outputOfLastAttempt = .exceptionFailure()
             }
             durationOfLastAttempt = clock.now - start
 
+            // if successful, return the output immediately
             if outputOfLastAttempt.returnCode == 0 {
                 return CommandExecutionResult(
                     command: command, host: host, output: outputOfLastAttempt,
                     duration: durationOfLastAttempt, retries: numberOfRetries,
                     exception: nil)
-            } else {
+            }
+
+            // otherwise, prep for possible retry
+            if command.retry.retryOnFailure && numberOfRetries < command.retry.maxRetries {
                 let delay = command.retry.strategy.delay(for: numberOfRetries, withJitter: true)
                 print("delaying for \(delay) due to failure before retrying command: \(command)")
                 try await Task.sleep(for: delay)
@@ -132,6 +142,6 @@ public actor Engine {
         return CommandExecutionResult(
             command: command, host: host, output: outputOfLastAttempt,
             duration: durationOfLastAttempt, retries: numberOfRetries,
-            exception: nil)
+            exception: capturedException)
     }
 }

Tests/formicTests/CommandExecutionOutputTests.swift

@@ -28,7 +28,8 @@ func testEmojiForExecutionOuput() async throws {
     #expect(
         CommandExecutionResult(
             command: cmd, host: .localhost, output: failureOutput, duration: .milliseconds(1),
-            retries: 0, exception: "Error desc"
+            retries: 0, exception: TestError.unknown(msg: "Error desc")
+
         ).emojiString() == "🚫")
 
 }
@@ -55,7 +56,7 @@ func testConsoleOutputForExecutionOuput() async throws {
 
     let exceptionResult = CommandExecutionResult(
         command: cmd, host: .localhost, output: failureOutput, duration: .milliseconds(1), retries: 0,
-        exception: "exception reported")
+        exception: TestError.unknown(msg: "exception reported"))
 
     //TODO: This is probably more sanely refactored into parameterized tests
 

Tests/formicTests/EngineTests.swift

@@ -129,24 +129,31 @@ func testEngineRunPlaybookWithFailure() async throws {
 }
 
 @Test("Direct engine execution - playbook w/ exception")
-func testEngineRunPlaybookWithException() async throws {
+func testEngineRunPlaybookCommandsWithException() async throws {
     let engine = Engine()
     let cmd1 = ShellCommand("uname")
     let cmd2 = ShellCommand("whoami")
 
-    await #expect(
-        throws: TestError.self,
-        performing: {
-            let _ = try await withDependencies { dependencyValues in
-                dependencyValues.localSystemAccess = TestFileSystemAccess()
-                dependencyValues.commandInvoker = TestCommandInvoker()
-                    .addSuccess(command: "uname", presentOutput: "Darwin\n")
-                    .addException(
-                        command: "whoami", errorToThrow: TestError.unknown(msg: "Process failed in something"))
-            } operation: {
-                try await engine.run(hosts: [.localhost], displayProgress: false, commands: [cmd1, cmd2])
-            }
-        })
+    let output = try await withDependencies { dependencyValues in
+        dependencyValues.localSystemAccess = TestFileSystemAccess()
+        dependencyValues.commandInvoker = TestCommandInvoker()
+            .addSuccess(command: "uname", presentOutput: "Darwin\n")
+            .addException(
+                command: "whoami", errorToThrow: TestError.unknown(msg: "Process whoami failed in something"))
+    } operation: {
+        try await engine.run(hosts: [.localhost], displayProgress: false, commands: [cmd1, cmd2])
+    }
+
+    #expect(output.count == 1)  // # of hosts
+    #expect(output[.localhost]?.count == 2)  // # of commands returned
+    let failedCmdResult = try #require(output[.localhost]?.last)
+
+    #expect(failedCmdResult.exception?.localizedDescription == "Unknown error: Process whoami failed in something")
+    #expect(failedCmdResult.output.returnCode == -1)
+    #expect(failedCmdResult.retries == 0)
+    #expect(failedCmdResult.representsFailure() == true)
+    #expect(failedCmdResult.output.stderrString == nil)
+    #expect(failedCmdResult.output.stdoutString == nil)
 }
 
 @Test("verify timeout is triggered on long command")
@@ -165,16 +172,19 @@ func testCommandTimeout() async throws {
     let mockCmdInvoker = TestCommandInvoker()
         .addSuccess(command: "uname", presentOutput: "Linux\n", delay: .seconds(2))
 
-    await #expect(
-        throws: CommandError.self, "Slow command should invoke timeout",
-        performing: {
-            let _ = try await withDependencies { dependencyValues in
-                dependencyValues.localSystemAccess = TestFileSystemAccess()
-                dependencyValues.commandInvoker = mockCmdInvoker
-            } operation: {
-                return try await engine.run(host: fakeHost, command: cmd1)
-            }
-        })
+    let outputFromRun = try await withDependencies { dependencyValues in
+        dependencyValues.localSystemAccess = TestFileSystemAccess()
+        dependencyValues.commandInvoker = mockCmdInvoker
+    } operation: {
+        return try await engine.run(host: fakeHost, command: cmd1)
+    }
+    let exceptionFromOutput = try #require(outputFromRun.exception)
+    #expect(exceptionFromOutput.localizedDescription == "Timeout exceeded for command: uname")
+    #expect(outputFromRun.output.returnCode == -1)
+    #expect(outputFromRun.retries == 0)
+    #expect(outputFromRun.representsFailure() == true)
+    #expect(outputFromRun.output.stderrString == nil)
+    #expect(outputFromRun.output.stdoutString == nil)
 }
 
 @Test("verify retry works as expected")