Fix file synchronization was being called after closing the filehandle, so it always failed (#82)

Clean up some other code to avoid throwing errors in the normal path

Author: Ignacio Bonafonte

Sources/Exporters/DatadogExporter/Files/Directory.swift

@@ -38,12 +38,13 @@ internal struct Directory {
     }
 
     /// Returns file with given name.
-    func file(named fileName: String) throws -> File {
+    func file(named fileName: String) -> File? {
         let fileURL = url.appendingPathComponent(fileName, isDirectory: false)
-        guard FileManager.default.fileExists(atPath: fileURL.path) else {
-            throw ExporterError(description: "File does not exist at path: \(fileURL.path)")
+        if FileManager.default.fileExists(atPath: fileURL.path) {
+            return File(url: fileURL)
+        } else {
+            return nil
         }
-        return File(url: fileURL)
     }
 
     /// Returns all files of this directory.

Sources/Exporters/DatadogExporter/Files/File.swift

@@ -73,10 +73,10 @@ internal struct File: WritableFile, ReadableFile {
          */
         if #available(OSX 10.15.4, iOS 13.4, watchOS 6.0, tvOS 13.4, *) {
             defer {
-                try? fileHandle.close()
                 if synchronized {
                     try? fileHandle.synchronize()
                 }
+                try? fileHandle.close()
             }
             try fileHandle.seekToEnd()
             try fileHandle.write(contentsOf: data)

Sources/Exporters/DatadogExporter/Persistence/FilesOrchestrator.swift

@@ -68,7 +68,9 @@ internal class FilesOrchestrator {
     private func reuseLastWritableFileIfPossible(writeSize: UInt64) -> WritableFile? {
         if let lastFileName = lastWritableFileName {
             do {
-                let lastFile = try directory.file(named: lastFileName)
+                guard let lastFile = directory.file(named: lastFileName) else {
+                    return nil
+                }
                 let lastFileCreationDate = fileCreationDateFrom(fileName: lastFile.name)
                 let lastFileAge = dateProvider.currentDate().timeIntervalSince(lastFileCreationDate)
 
@@ -83,7 +85,6 @@ internal class FilesOrchestrator {
                 print("🔥 Failed to read previously used writable file: \(error)")
             }
         }
-
         return nil
     }
 

Tests/ExportersTests/DatadogExporter/Files/DirectoryTests.swift

@@ -68,9 +68,9 @@ class DirectoryTests: XCTestCase {
         defer { directory.delete() }
         _ = try directory.createFile(named: "abcd")
 
-        let file = try directory.file(named: "abcd")
-        XCTAssertEqual(file.url, directory.url.appendingPathComponent("abcd"))
-        XCTAssertTrue(fileManager.fileExists(atPath: file.url.path))
+        let file = directory.file(named: "abcd")
+        XCTAssertEqual(file?.url, directory.url.appendingPathComponent("abcd"))
+        XCTAssertTrue(fileManager.fileExists(atPath: file!.url.path))
     }
 
     func testItRetrievesAllFiles() throws {

Tests/ExportersTests/DatadogExporter/Persistence/FilesOrchestratorTests.swift

@@ -46,7 +46,7 @@ class FilesOrchestratorTests: XCTestCase {
         _ = try orchestrator.getWritableFile(writeSize: 1)
 
         XCTAssertEqual(try temporaryDirectory.files().count, 1)
-        XCTAssertNotNil(try temporaryDirectory.file(named: dateProvider.currentDate().toFileName))
+        XCTAssertNotNil(temporaryDirectory.file(named: dateProvider.currentDate().toFileName))
     }
 
     func testGivenDefaultWriteConditions_whenUsedNextTime_itReusesWritableFile() throws {
@@ -160,12 +160,12 @@ class FilesOrchestratorTests: XCTestCase {
         // Asking for the next file should purge the oldest one.
         let file4 = try orchestrator.getWritableFile(writeSize: oneMB)
         XCTAssertEqual(try temporaryDirectory.files().count, 3)
-        XCTAssertNil(try? temporaryDirectory.file(named: file1.name))
+        XCTAssertNil(temporaryDirectory.file(named: file1.name))
         try file4.append(data: .mock(ofSize: oneMB + 1), synchronized: true)
 
         _ = try orchestrator.getWritableFile(writeSize: oneMB)
         XCTAssertEqual(try temporaryDirectory.files().count, 3)
-        XCTAssertNil(try? temporaryDirectory.file(named: file2.name))
+        XCTAssertNil(temporaryDirectory.file(named: file2.name))
     }
 
     // MARK: - Readable file tests
@@ -203,13 +203,13 @@ class FilesOrchestratorTests: XCTestCase {
 
         dateProvider.advance(bySeconds: 1 + performance.minFileAgeForRead)
         XCTAssertEqual(orchestrator.getReadableFile()?.name, fileNames[0])
-        try temporaryDirectory.file(named: fileNames[0]).delete()
+        try temporaryDirectory.file(named: fileNames[0])?.delete()
         XCTAssertEqual(orchestrator.getReadableFile()?.name, fileNames[1])
-        try temporaryDirectory.file(named: fileNames[1]).delete()
+        try temporaryDirectory.file(named: fileNames[1])?.delete()
         XCTAssertEqual(orchestrator.getReadableFile()?.name, fileNames[2])
-        try temporaryDirectory.file(named: fileNames[2]).delete()
+        try temporaryDirectory.file(named: fileNames[2])?.delete()
         XCTAssertEqual(orchestrator.getReadableFile()?.name, fileNames[3])
-        try temporaryDirectory.file(named: fileNames[3]).delete()
+        try temporaryDirectory.file(named: fileNames[3])?.delete()
         XCTAssertNil(orchestrator.getReadableFile())
     }