Improve JSON progress file handling and error reporting

roulpriya · roulpriya · commit 6a0774556508 · 2025-06-24T00:02:03.000+05:30
diff --git a/Documentation/SwiftlyDocs.docc/swiftly-cli-reference.md b/Documentation/SwiftlyDocs.docc/swiftly-cli-reference.md
@@ -86,6 +86,7 @@ written to this file as commands that can be run after the installation.
 
 Progress information will be appended to this file as JSON objects, one per line.
 Each progress entry contains timestamp, progress percentage, and a descriptive message.
+The file must be writable, else an error will be thrown.
 
 
 **--assume-yes:**
diff --git a/Sources/Swiftly/Install.swift b/Sources/Swiftly/Install.swift
@@ -77,6 +77,7 @@ struct Install: SwiftlyCommand {
             discussion: """
             Progress information will be appended to this file as JSON objects, one per line.
             Each progress entry contains timestamp, progress percentage, and a descriptive message.
+            The file must be writable, else an error will be thrown.
             """
         ))
     var progressFile: FilePath?
@@ -313,12 +314,16 @@ struct Install: SwiftlyCommand {
             }
 
             let animation: ProgressAnimationProtocol =
-                progressFile != nil
-                    ? JsonFileProgressReporter(filePath: progressFile!)
-                    : PercentProgressAnimation(
-                        stream: stdoutStream,
-                        header: "Downloading \(version)"
-                    )
+                if let progressFile
+            {
+                try JsonFileProgressReporter(ctx, filePath: progressFile)
+            } else {
+                PercentProgressAnimation(stream: stdoutStream, header: "Downloading \(version)")
+            }
+
+            defer {
+                try? (animation as? JsonFileProgressReporter)?.close()
+            }
 
             var lastUpdate = Date()
 
diff --git a/Sources/Swiftly/JsonFileProgressReporter.swift b/Sources/Swiftly/JsonFileProgressReporter.swift
@@ -11,27 +11,30 @@ enum ProgressInfo: Codable {
 struct JsonFileProgressReporter: ProgressAnimationProtocol {
     let filePath: FilePath
     private let encoder: JSONEncoder
+    private let ctx: SwiftlyCoreContext
+    private let fileHandle: FileHandle
 
-    init(filePath: FilePath, encoder: JSONEncoder = JSONEncoder()) {
+    init(_ ctx: SwiftlyCoreContext, filePath: FilePath, encoder: JSONEncoder = JSONEncoder()) throws
+    {
+        self.ctx = ctx
         self.filePath = filePath
         self.encoder = encoder
+        self.fileHandle = try FileHandle(forWritingTo: URL(fileURLWithPath: filePath.string))
     }
 
     private func writeProgress(_ progress: ProgressInfo) {
         let jsonData = try? self.encoder.encode(progress)
-        guard let jsonData = jsonData, let jsonString = String(data: jsonData, encoding: .utf8)
-        else {
-            print("Failed to encode progress entry to JSON")
+        guard let jsonData = jsonData else {
+            Task { [ctx = self.ctx] in
+                await ctx.message("Failed to encode progress entry to JSON")
+            }
             return
         }
 
-        let jsonLine = jsonString + "\n"
-
-        do {
-            try jsonLine.append(to: self.filePath)
-        } catch {
-            print("Failed to write progress entry to \(self.filePath): \(error)")
-        }
+        self.fileHandle.seekToEndOfFile()
+        self.fileHandle.write(jsonData)
+        self.fileHandle.write("\n".data(using: .utf8) ?? Data())
+        self.fileHandle.synchronizeFile()
     }
 
     func update(step: Int, total: Int, text: String) {
@@ -49,10 +52,11 @@ struct JsonFileProgressReporter: ProgressAnimationProtocol {
     }
 
     func clear() {
-        do {
-            try FileManager.default.removeItem(atPath: self.filePath.string)
-        } catch {
-            print("Failed to clear progress file at \(self.filePath): \(error)")
-        }
+        self.fileHandle.truncateFile(atOffset: 0)
+        self.fileHandle.synchronizeFile()
+    }
+
+    func close() throws {
+        try self.fileHandle.close()
     }
 }
diff --git a/Sources/SwiftlyCore/FileManager+FilePath.swift b/Sources/SwiftlyCore/FileManager+FilePath.swift
@@ -60,7 +60,7 @@ public enum FileSystem {
         case mode(Int)
     }
 
-    public static func create(_ options: CreateOptions..., file: FilePath, contents: Data?) async throws {
+    public static func create(_ options: CreateOptions..., file: FilePath, contents: Data? = nil) async throws {
         try await Self.create(options, file: file, contents: contents)
     }
 
@@ -195,15 +195,6 @@ extension String {
             try self.write(to: path, atomically: true, encoding: enc)
             return
         }
-
-        let fileHandle = try FileHandle(forWritingTo: URL(fileURLWithPath: path.string))
-        defer { fileHandle.closeFile() }
-        fileHandle.seekToEndOfFile()
-        if let data = self.data(using: enc) {
-            fileHandle.write(data)
-        } else {
-            throw SwiftlyError(message: "Failed to convert string to data with encoding \(enc)")
-        }
     }
 
     public init(contentsOf path: FilePath, encoding enc: String.Encoding = .utf8) throws {
diff --git a/Tests/SwiftlyTests/JsonFileProgressReporterTests.swift b/Tests/SwiftlyTests/JsonFileProgressReporterTests.swift
@@ -1,133 +1,163 @@
 import Foundation
-@testable import Swiftly
-@testable import SwiftlyCore
 import SystemPackage
 import Testing
 
-@Suite("JsonFileProgressReporter Tests")
-struct JsonFileProgressReporterTests {
-    @Test("Test update method writes progress to file")
-    func testUpdateWritesProgressToFile() throws {
+@testable import Swiftly
+@testable import SwiftlyCore
+
+@Suite struct JsonFileProgressReporterTests {
+    @Test("Test update method writes progress to file as valid JSONNL")
+    func testUpdateWritesProgressToFile() async throws {
         let tempFile = fs.mktemp(ext: ".json")
-        let reporter = JsonFileProgressReporter(filePath: tempFile)
+        try await fs.create(.mode(Int(0o644)), file: tempFile)
+        defer { try? FileManager.default.removeItem(atPath: tempFile.string) }
+        let reporter = try JsonFileProgressReporter(SwiftlyTests.ctx, filePath: tempFile)
 
         reporter.update(step: 1, total: 10, text: "Processing item 1")
-
-        let fileContent = try String(contentsOfFile: tempFile.string)
-
-        #expect(fileContent.contains("Processing item 1"))
-        #expect(fileContent.contains("\"percent\":10"))
-        #expect(fileContent.contains("\"step\""))
-        #expect(fileContent.contains("\"timestamp\""))
-
-        try FileManager.default.removeItem(atPath: tempFile.string)
+        try reporter.close()
+
+        let decoder = JSONDecoder()
+
+        let info = try String(contentsOfFile: tempFile.string).split(separator: "\n")
+            .filter {
+                !$0.isEmpty
+            }.map {
+                try decoder.decode(
+                    ProgressInfo.self,
+                    from: Data($0.trimmingCharacters(in: .whitespacesAndNewlines).utf8)
+                )
+            }
+
+        #expect(info.count == 1)
+
+        if case let .step(timestamp, percent, text) = info.first {
+            #expect(text == "Processing item 1")
+            #expect(percent == 10)
+            #expect(timestamp.timeIntervalSince1970 > 0)
+        } else {
+            Issue.record("Expected step info but got \(info[0])")
+            return
+        }
     }
 
     @Test("Test complete method writes completion status")
-    func testCompleteWritesCompletionStatus() throws {
+    func testCompleteWritesCompletionStatus() async throws {
         let tempFile = fs.mktemp(ext: ".json")
-        let reporter = JsonFileProgressReporter(filePath: tempFile)
-
-        reporter.complete(success: true)
-
-        let fileContent = try String(contentsOfFile: tempFile.string)
-
-        #expect(fileContent.contains("\"success\":true"))
-        #expect(fileContent.contains("\"complete\""))
+        try await fs.create(.mode(Int(0o644)), file: tempFile)
+        defer { try? FileManager.default.removeItem(atPath: tempFile.string) }
 
-        try FileManager.default.removeItem(atPath: tempFile.string)
-    }
+        let reporter = try JsonFileProgressReporter(SwiftlyTests.ctx, filePath: tempFile)
 
-    @Test("Test complete method writes failure status")
-    func testCompleteWritesFailureStatus() throws {
-        let tempFile = fs.mktemp(ext: ".json")
-        let reporter = JsonFileProgressReporter(filePath: tempFile)
+        let status = Bool.random()
+        reporter.complete(success: status)
+        try reporter.close()
 
-        reporter.complete(success: false)
+        let decoder = JSONDecoder()
 
-        let fileContent = try String(contentsOfFile: tempFile.string)
+        let info = try String(contentsOfFile: tempFile.string).split(separator: "\n")
+            .filter {
+                !$0.isEmpty
+            }.map {
+                try decoder.decode(ProgressInfo.self, from: Data($0.utf8))
+            }
 
-        #expect(fileContent.contains("\"success\":false"))
-        #expect(fileContent.contains("\"complete\""))
+        #expect(info.count == 1)
 
-        try FileManager.default.removeItem(atPath: tempFile.string)
+        if case let .complete(success) = info.first {
+            #expect(success == status)
+        } else {
+            Issue.record("Expected completion info but got \(info)")
+            return
+        }
     }
 
     @Test("Test percentage calculation")
-    func testPercentageCalculation() throws {
+    func testPercentageCalculation() async throws {
         let tempFile = fs.mktemp(ext: ".json")
-        let reporter = JsonFileProgressReporter(filePath: tempFile)
+        try await fs.create(.mode(Int(0o644)), file: tempFile)
+        defer { try? FileManager.default.removeItem(atPath: tempFile.string) }
+        let reporter = try JsonFileProgressReporter(SwiftlyTests.ctx, filePath: tempFile)
 
         reporter.update(step: 25, total: 100, text: "Quarter way")
-
-        let fileContent = try String(contentsOfFile: tempFile.string)
-
-        #expect(fileContent.contains("\"percent\":25"))
+        try reporter.close()
+
+        let decoder = JSONDecoder()
+        let info = try String(contentsOfFile: tempFile.string).split(separator: "\n")
+            .filter {
+                !$0.isEmpty
+            }.map {
+                try decoder.decode(ProgressInfo.self, from: Data($0.utf8))
+            }
+        #expect(info.count == 1)
+        if case let .step(_, percent, text) = info.first {
+            #expect(percent == 25)
+            #expect(text == "Quarter way")
+        } else {
+            Issue.record("Expected step info but got \(info)")
+            return
+        }
 
         try FileManager.default.removeItem(atPath: tempFile.string)
     }
 
-    @Test("Test clear method removes file")
-    func testClearRemovesFile() throws {
+    @Test("Test clear method truncates the file")
+    func testClearTruncatesFile() async throws {
         let tempFile = fs.mktemp(ext: ".json")
-        let reporter = JsonFileProgressReporter(filePath: tempFile)
+        try await fs.create(.mode(Int(0o644)), file: tempFile)
+        defer { try? FileManager.default.removeItem(atPath: tempFile.string) }
+        let reporter = try JsonFileProgressReporter(SwiftlyTests.ctx, filePath: tempFile)
+        defer { try? reporter.close() }
 
         reporter.update(step: 1, total: 2, text: "Test")
 
-        #expect(FileManager.default.fileExists(atPath: tempFile.string))
+        #expect(try String(contentsOf: tempFile).lengthOfBytes(using: String.Encoding.utf8) > 0)
 
         reporter.clear()
 
-        #expect(!FileManager.default.fileExists(atPath: tempFile.string))
+        #expect(try String(contentsOf: tempFile).lengthOfBytes(using: String.Encoding.utf8) == 0)
     }
 
     @Test("Test multiple progress updates create multiple lines")
-    func testMultipleUpdatesCreateMultipleLines() throws {
+    func testMultipleUpdatesCreateMultipleLines() async throws {
         let tempFile = fs.mktemp(ext: ".json")
-        let reporter = JsonFileProgressReporter(filePath: tempFile)
+        try await fs.create(.mode(Int(0o644)), file: tempFile)
+        defer { try? FileManager.default.removeItem(atPath: tempFile.string) }
 
-        reporter.update(step: 1, total: 3, text: "Step 1")
-        reporter.update(step: 2, total: 3, text: "Step 2")
-        reporter.complete(success: true)
+        let reporter = try JsonFileProgressReporter(SwiftlyTests.ctx, filePath: tempFile)
 
-        let fileContent = try String(contentsOfFile: tempFile.string)
-        let lines = fileContent.components(separatedBy: .newlines).filter { !$0.isEmpty }
+        reporter.update(step: 5, total: 100, text: "Processing item 5")
+        reporter.update(step: 10, total: 100, text: "Processing item 10")
+        reporter.update(step: 50, total: 100, text: "Processing item 50")
+        reporter.update(step: 100, total: 100, text: "Processing item 100")
 
-        #expect(lines.count == 3)
-        #expect(lines[0].contains("Step 1"))
-        #expect(lines[1].contains("Step 2"))
-        #expect(lines[2].contains("\"success\":true"))
-
-        try FileManager.default.removeItem(atPath: tempFile.string)
-    }
-
-    @Test("Test zero step edge case")
-    func testZeroStepEdgeCase() throws {
-        let tempFile = fs.mktemp(ext: ".json")
-        let reporter = JsonFileProgressReporter(filePath: tempFile)
-
-        reporter.update(step: 0, total: 10, text: "Starting")
-
-        let fileContent = try String(contentsOfFile: tempFile.string)
-
-        #expect(fileContent.contains("\"percent\":0"))
-        #expect(fileContent.contains("Starting"))
-
-        try FileManager.default.removeItem(atPath: tempFile.string)
-    }
-
-    @Test("Test full completion edge case")
-    func testFullCompletionEdgeCase() throws {
-        let tempFile = fs.mktemp(ext: ".json")
-        let reporter = JsonFileProgressReporter(filePath: tempFile)
-
-        reporter.update(step: 100, total: 100, text: "Done")
-
-        let fileContent = try String(contentsOfFile: tempFile.string)
-
-        #expect(fileContent.contains("\"percent\":100"))
-        #expect(fileContent.contains("Done"))
-
-        try FileManager.default.removeItem(atPath: tempFile.string)
+        reporter.complete(success: true)
+        try? reporter.close()
+
+        let decoder = JSONDecoder()
+        let info = try String(contentsOfFile: tempFile.string).split(separator: "\n")
+            .filter {
+                !$0.isEmpty
+            }.map {
+                try decoder.decode(ProgressInfo.self, from: Data($0.utf8))
+            }
+
+        #expect(info.count == 5)
+
+        for (idx, pct) in [5, 10, 50, 100].enumerated() {
+            if case let .step(_, percent, text) = info[idx] {
+                #expect(text == "Processing item \(pct)")
+                #expect(percent == pct)
+            } else {
+                Issue.record("Expected step info but got \(info[idx])")
+                return
+            }
+        }
+
+        if case let .complete(success) = info[4] {
+            #expect(success == true)
+        } else {
+            Issue.record("Expected completion info but got \(info[4])")
+            return
+        }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ public enum FileSystem {`
`60`	`60`	`case mode(Int)`
`61`	`61`	`}`
`62`	`62`
`63`		`- public static func create(_ options: CreateOptions..., file: FilePath, contents: Data?) async throws {`
	`63`	`+ public static func create(_ options: CreateOptions..., file: FilePath, contents: Data? = nil) async throws {`
`64`	`64`	`try await Self.create(options, file: file, contents: contents)`
`65`	`65`	`}`
`66`	`66`
`@@ -195,15 +195,6 @@ extension String {`
`195`	`195`	`try self.write(to: path, atomically: true, encoding: enc)`
`196`	`196`	`return`
`197`	`197`	`}`
`198`		`-`
`199`		`- let fileHandle = try FileHandle(forWritingTo: URL(fileURLWithPath: path.string))`
`200`		`- defer { fileHandle.closeFile() }`
`201`		`- fileHandle.seekToEndOfFile()`
`202`		`- if let data = self.data(using: enc) {`
`203`		`- fileHandle.write(data)`
`204`		`- } else {`
`205`		`- throw SwiftlyError(message: "Failed to convert string to data with encoding \(enc)")`
`206`		`- }`
`207`	`198`	`}`
`208`	`199`
`209`	`200`	`public init(contentsOf path: FilePath, encoding enc: String.Encoding = .utf8) throws {`