Merge pull request #91 from zero-ide/feature/commandrunner-stderr-structured

refactor: split command runner stderr and stdout in failure model

Author: ori0o0p

Sources/Zero/Core/CommandRunner.swift

@@ -1,16 +1,21 @@
 import Foundation
 
 enum CommandRunnerError: LocalizedError {
-    case commandFailed(command: String, arguments: [String], exitCode: Int, output: String)
+    case commandFailed(command: String, arguments: [String], exitCode: Int, stdout: String, stderr: String)
 
     var errorDescription: String? {
         switch self {
-        case .commandFailed(_, _, let exitCode, let output):
-            let trimmedOutput = output.trimmingCharacters(in: .whitespacesAndNewlines)
-            if trimmedOutput.isEmpty {
+        case .commandFailed(_, _, let exitCode, let stdout, let stderr):
+            let trimmedStderr = stderr.trimmingCharacters(in: .whitespacesAndNewlines)
+            if !trimmedStderr.isEmpty {
+                return trimmedStderr
+            }
+
+            let trimmedStdout = stdout.trimmingCharacters(in: .whitespacesAndNewlines)
+            if trimmedStdout.isEmpty {
                 return "Command failed with exit code \(exitCode)."
             }
-            return trimmedOutput
+            return trimmedStdout
         }
     }
 }
@@ -31,14 +36,16 @@ final class CommandRunner: CommandRunning {
 
     func executeStreaming(command: String, arguments: [String] = [], onOutput: @escaping (String) -> Void) throws -> String {
         let process = Process()
-        let pipe = Pipe()
+        let stdoutPipe = Pipe()
+        let stderrPipe = Pipe()
         let dataLock = NSLock()
-        var outputData = Data()
+        var stdoutData = Data()
+        var stderrData = Data()
 
         process.executableURL = URL(fileURLWithPath: command)
         process.arguments = arguments
-        process.standardOutput = pipe
-        process.standardError = pipe
+        process.standardOutput = stdoutPipe
+        process.standardError = stderrPipe
 
         processLock.lock()
         currentProcess = process
@@ -51,13 +58,28 @@ final class CommandRunner: CommandRunning {
             processLock.unlock()
         }
 
-        let fileHandle = pipe.fileHandleForReading
-        fileHandle.readabilityHandler = { handle in
+        let stdoutHandle = stdoutPipe.fileHandleForReading
+        let stderrHandle = stderrPipe.fileHandleForReading
+
+        stdoutHandle.readabilityHandler = { handle in
             let chunkData = handle.availableData
             guard !chunkData.isEmpty else { return }
 
             dataLock.lock()
-            outputData.append(chunkData)
+            stdoutData.append(chunkData)
+            dataLock.unlock()
+
+            if let chunk = String(data: chunkData, encoding: .utf8), !chunk.isEmpty {
+                onOutput(chunk)
+            }
+        }
+
+        stderrHandle.readabilityHandler = { handle in
+            let chunkData = handle.availableData
+            guard !chunkData.isEmpty else { return }
+
+            dataLock.lock()
+            stderrData.append(chunkData)
             dataLock.unlock()
 
             if let chunk = String(data: chunkData, encoding: .utf8), !chunk.isEmpty {
@@ -68,31 +90,45 @@ final class CommandRunner: CommandRunning {
         try process.run()
         process.waitUntilExit()
 
-        fileHandle.readabilityHandler = nil
+        stdoutHandle.readabilityHandler = nil
+        stderrHandle.readabilityHandler = nil
+
+        let trailingStdout = stdoutHandle.readDataToEndOfFile()
+        if !trailingStdout.isEmpty {
+            dataLock.lock()
+            stdoutData.append(trailingStdout)
+            dataLock.unlock()
+
+            if let trailingChunk = String(data: trailingStdout, encoding: .utf8), !trailingChunk.isEmpty {
+                onOutput(trailingChunk)
+            }
+        }
 
-        let trailingData = fileHandle.readDataToEndOfFile()
-        if !trailingData.isEmpty {
+        let trailingStderr = stderrHandle.readDataToEndOfFile()
+        if !trailingStderr.isEmpty {
             dataLock.lock()
-            outputData.append(trailingData)
+            stderrData.append(trailingStderr)
             dataLock.unlock()
 
-            if let trailingChunk = String(data: trailingData, encoding: .utf8), !trailingChunk.isEmpty {
+            if let trailingChunk = String(data: trailingStderr, encoding: .utf8), !trailingChunk.isEmpty {
                 onOutput(trailingChunk)
             }
         }
 
-        let output = String(data: outputData, encoding: .utf8) ?? ""
+        let stdout = String(data: stdoutData, encoding: .utf8) ?? ""
+        let stderr = String(data: stderrData, encoding: .utf8) ?? ""
 
         if process.terminationStatus != 0 {
             throw CommandRunnerError.commandFailed(
                 command: command,
                 arguments: arguments,
                 exitCode: Int(process.terminationStatus),
-                output: output
+                stdout: stdout,
+                stderr: stderr
             )
         }
 
-        return output
+        return stdout + stderr
     }
 
     func cancelCurrentCommand() {

Sources/Zero/Services/DockerService.swift

@@ -156,8 +156,8 @@ struct DockerService: DockerServiceProtocol {
             return zeroError
         }
 
-        if case let CommandRunnerError.commandFailed(binary, arguments, exitCode, output) = error {
-            let debugDetails = "\(context) [binary=\(binary)] [args=\(arguments.joined(separator: " "))] [script=\(command)] [exit=\(exitCode)] [output=\(output)]"
+        if case let CommandRunnerError.commandFailed(binary, arguments, exitCode, stdout, stderr) = error {
+            let debugDetails = "\(context) [binary=\(binary)] [args=\(arguments.joined(separator: " "))] [script=\(command)] [exit=\(exitCode)] [stdout=\(stdout)] [stderr=\(stderr)]"
             return .runtimeCommandFailed(userMessage: context, debugDetails: debugDetails)
         }
 

Sources/Zero/Services/ExecutionService.swift

@@ -217,7 +217,7 @@ class ExecutionService: ObservableObject {
             return "url_error_\(urlError.errorCode)"
         }
 
-        if case let CommandRunnerError.commandFailed(_, _, exitCode, _) = error {
+        if case let CommandRunnerError.commandFailed(_, _, exitCode, _, _) = error {
             return "command_failed_\(exitCode)"
         }
 

Tests/ZeroTests/CommandRunnerTests.swift

@@ -21,15 +21,35 @@ final class CommandRunnerTests: XCTestCase {
         XCTAssertThrowsError(
             try runner.execute(command: "/bin/sh", arguments: ["-c", "echo boom >&2; exit 7"])
         ) { error in
-            guard case let CommandRunnerError.commandFailed(command, arguments, exitCode, output) = error else {
+            guard case let CommandRunnerError.commandFailed(command, arguments, exitCode, stdout, stderr) = error else {
                 XCTFail("Expected CommandRunnerError.commandFailed")
                 return
             }
 
             XCTAssertEqual(command, "/bin/sh")
             XCTAssertEqual(arguments, ["-c", "echo boom >&2; exit 7"])
             XCTAssertEqual(exitCode, 7)
-            XCTAssertTrue(output.contains("boom"))
+            XCTAssertTrue(stdout.isEmpty)
+            XCTAssertTrue(stderr.contains("boom"))
+        }
+    }
+
+    func testExecuteThrowsErrorWithSeparatedStdoutAndStderr() {
+        // Given
+        let runner = CommandRunner()
+
+        // When & Then
+        XCTAssertThrowsError(
+            try runner.execute(command: "/bin/sh", arguments: ["-c", "echo out; echo err >&2; exit 3"])
+        ) { error in
+            guard case let CommandRunnerError.commandFailed(_, _, exitCode, stdout, stderr) = error else {
+                XCTFail("Expected CommandRunnerError.commandFailed")
+                return
+            }
+
+            XCTAssertEqual(exitCode, 3)
+            XCTAssertTrue(stdout.contains("out"))
+            XCTAssertTrue(stderr.contains("err"))
         }
     }
 }

Tests/ZeroTests/DockerServiceTests.swift

@@ -187,10 +187,12 @@ final class DockerServiceTests: XCTestCase {
     func testExecuteShellMapsRunnerFailureToStructuredZeroError() {
         // Given
         let mockRunner = MockCommandRunner()
-        mockRunner.mockError = NSError(
-            domain: "CommandRunner",
-            code: 127,
-            userInfo: [NSLocalizedDescriptionKey: "sh: npm: not found"]
+        mockRunner.mockError = CommandRunnerError.commandFailed(
+            command: "/usr/local/bin/docker",
+            arguments: ["exec", "zero-dev", "sh", "-c", "npm start"],
+            exitCode: 127,
+            stdout: "",
+            stderr: "sh: npm: not found"
         )
         let service = DockerService(runner: mockRunner)
 
@@ -204,7 +206,7 @@ final class DockerServiceTests: XCTestCase {
 
             XCTAssertEqual(userMessage, "Docker shell command failed.")
             XCTAssertTrue(debugDetails.contains("npm start"))
-            XCTAssertTrue(debugDetails.contains("not found"))
+            XCTAssertTrue(debugDetails.contains("stderr=sh: npm: not found"))
         }
     }
 }