Merge pull request #2 from XcodesOrg/betterruntimedonwloaderrors

better error messages when downloading

Author: MattKiazyk

Sources/XcodesKit/Services/Aria2DownloadService.swift

@@ -38,11 +38,16 @@ public struct Aria2DownloadService: Sendable {
 
                 progress.updateFromAria2(string: string)
             },
-            failureHandler: { process in
+            failureHandler: { process, stdout, stderr in
                 if let aria2cError = Aria2CError(exitStatus: process.terminationStatus) {
                     return aria2cError
                 } else {
-                    return ProcessExecutionError(process: process, standardOutput: "", standardError: "")
+                    return ProcessExecutionError(
+                        process: process,
+                        terminationStatus: process.terminationStatus,
+                        standardOutput: stdout,
+                        standardError: stderr
+                    )
                 }
             },
             successHandler: {

Sources/XcodesKit/Services/XcodebuildRuntimeDownloadService.swift

@@ -34,8 +34,13 @@ public struct XcodebuildRuntimeDownloadService: Sendable {
             outputHandler: { string, progress in
                 progress.updateFromXcodebuild(text: string)
             },
-            failureHandler: { process in
-                ProcessExecutionError(process: process, standardOutput: "", standardError: "")
+            failureHandler: { process, stdout, stderr in
+                ProcessExecutionError(
+                    process: process,
+                    terminationStatus: process.terminationStatus,
+                    standardOutput: stdout,
+                    standardError: stderr
+                )
             }
         ).stream()
     }

Sources/XcodesKit/Shell/ProcessProgressStream.swift

@@ -3,7 +3,7 @@ import os
 
 final class ProcessProgressStreamRunner: Sendable {
     typealias OutputHandler = @Sendable (String, Progress) -> Void
-    typealias FailureHandler = @Sendable (Process) -> Error
+    typealias FailureHandler = @Sendable (Process, String, String) -> Error
     typealias SuccessHandler = @Sendable () -> Error?
 
     private let process: Process
@@ -54,20 +54,21 @@ final class ProcessProgressStreamRunner: Sendable {
         let stdErrPipe = Pipe()
         process.standardError = stdErrPipe
 
-        let handleData: @Sendable (FileHandle) -> Void = { [weak self] handle in
+        let handleData: @Sendable (FileHandle, OutputStream) -> Void = { [weak self] handle, stream in
             guard let self else { return }
             let data = handle.availableData
             guard data.isEmpty == false else { return }
 
             let string = String(decoding: data, as: UTF8.self)
             self.continuation.withLock {
+                self.append(data, to: stream)
                 self.outputHandler(string, self.progress)
                 _ = $0?.yield(self.progress)
             }
         }
 
-        stdOutPipe.fileHandleForReading.readabilityHandler = handleData
-        stdErrPipe.fileHandleForReading.readabilityHandler = handleData
+        stdOutPipe.fileHandleForReading.readabilityHandler = { handleData($0, .stdout) }
+        stdErrPipe.fileHandleForReading.readabilityHandler = { handleData($0, .stderr) }
 
         process.terminationHandler = { [weak self] process in
             self?.finish(process: process)
@@ -95,7 +96,8 @@ final class ProcessProgressStreamRunner: Sendable {
         consumeRemainingOutput()
 
         guard process.terminationReason == .exit, process.terminationStatus == 0 else {
-            finish(throwing: failureHandler(process))
+            let output = output()
+            finish(throwing: failureHandler(process, output.stdout, output.stderr))
             return
         }
 
@@ -126,20 +128,53 @@ final class ProcessProgressStreamRunner: Sendable {
     }
 
     private func consumeRemainingOutput() {
-        consumeRemainingOutput(from: process.standardOutput as? Pipe)
-        consumeRemainingOutput(from: process.standardError as? Pipe)
+        consumeRemainingOutput(from: process.standardOutput as? Pipe, stream: .stdout)
+        consumeRemainingOutput(from: process.standardError as? Pipe, stream: .stderr)
     }
 
-    private func consumeRemainingOutput(from pipe: Pipe?) {
+    private func consumeRemainingOutput(from pipe: Pipe?, stream: OutputStream) {
         guard let pipe else { return }
 
         let data = pipe.fileHandleForReading.readDataToEndOfFile()
         guard data.isEmpty == false else { return }
 
         let string = String(decoding: data, as: UTF8.self)
         continuation.withLock {
+            append(data, to: stream)
             outputHandler(string, progress)
             _ = $0?.yield(progress)
         }
     }
+
+    private let capturedOutput = OSAllocatedUnfairLock<OutputStorage>(initialState: OutputStorage())
+
+    private func append(_ data: Data, to stream: OutputStream) {
+        capturedOutput.withLock {
+            switch stream {
+            case .stdout:
+                $0.stdout.append(data)
+            case .stderr:
+                $0.stderr.append(data)
+            }
+        }
+    }
+
+    private func output() -> (stdout: String, stderr: String) {
+        capturedOutput.withLock {
+            (
+                String(data: $0.stdout, encoding: .utf8) ?? "",
+                String(data: $0.stderr, encoding: .utf8) ?? ""
+            )
+        }
+    }
+
+    private enum OutputStream {
+        case stdout
+        case stderr
+    }
+
+    private struct OutputStorage: Sendable {
+        var stdout = Data()
+        var stderr = Data()
+    }
 }

Tests/XcodesKitTests/XcodesKitTests.swift

@@ -2041,7 +2041,7 @@ final class XcodesKitTests: XCTestCase {
                 collector.append(string)
                 progress.updateFromXcodebuild(text: string)
             },
-            failureHandler: { process in
+            failureHandler: { process, _, _ in
                 ProcessExecutionError(process: process, standardOutput: "", standardError: "")
             }
         ).stream()
@@ -2074,7 +2074,7 @@ final class XcodesKitTests: XCTestCase {
             outputHandler: { string, _ in
                 collector.append(string)
             },
-            failureHandler: { process in
+            failureHandler: { process, _, _ in
                 ProcessExecutionError(process: process, standardOutput: "", standardError: "")
             }
         ).stream()
@@ -2087,27 +2087,27 @@ final class XcodesKitTests: XCTestCase {
 
     func testProcessProgressStreamRunnerThrowsFailureHandlerError() async {
         enum TestError: Error, Equatable {
-            case failed(Int32)
+            case failed(Int32, String, String)
         }
 
         let process = Process()
         process.executableURL = URL(fileURLWithPath: "/bin/sh")
-        process.arguments = ["-c", "exit 12"]
+        process.arguments = ["-c", "printf 'stdout text'; printf 'stderr text' >&2; exit 12"]
 
         let stream = ProcessProgressStreamRunner(
             process: process,
             progress: Progress(),
             outputHandler: { _, _ in },
-            failureHandler: { process in
-                TestError.failed(process.terminationStatus)
+            failureHandler: { process, stdout, stderr in
+                TestError.failed(process.terminationStatus, stdout, stderr)
             }
         ).stream()
 
         do {
             for try await _ in stream {}
             XCTFail("Expected process failure to throw")
         } catch {
-            XCTAssertEqual(error as? TestError, .failed(12))
+            XCTAssertEqual(error as? TestError, .failed(12, "stdout text", "stderr text"))
         }
     }