From 90f32a84bb582c5c2a4902e1ba783ab73adcc1cf Mon Sep 17 00:00:00 2001 From: Julian Kahnert Date: Mon, 19 Oct 2026 08:52:33 +0200 Subject: [PATCH] Stop file logging from crashing launches without a writable log file FileLogHandler opened the day's log file inside Logger.init and called fatalError when that failed. FlowKit Controller build 518 crashed this way twice on 2026-10-18 during background launches, before didFinishLaunchingWithOptions ran. - FileHandlerOutputStream opens the file on the first write and reopens it on the next write after an open or write error; the first failure in a row is logged, the entries in between reach OSLog only - initLogging shares one stream across all logger labels, so creating a Logger no longer touches the file system - logFileDirectory() is public and the App Logs screen reads from it; it read Documents/ while the files have been written to Documents/logs/ since #183 Fixes #215 --- .../Features/LogViewerFeature.swift | 2 +- Sources/Shared/FileLogHandler.swift | 60 +++++++++++-------- Sources/Shared/Log.swift | 17 +++--- Sources/Shared/LogReader.swift | 2 +- Tests/SharedTests/FileLogHandlerTests.swift | 36 +++++++++++ 5 files changed, 82 insertions(+), 35 deletions(-) create mode 100644 Tests/SharedTests/FileLogHandlerTests.swift diff --git a/Sources/Controller/Features/LogViewerFeature.swift b/Sources/Controller/Features/LogViewerFeature.swift index 46237f1e..e4d37ffe 100644 --- a/Sources/Controller/Features/LogViewerFeature.swift +++ b/Sources/Controller/Features/LogViewerFeature.swift @@ -42,7 +42,7 @@ struct LogViewerFeature: Sendable { return .run { send in // 24 hours let since = Date().addingTimeInterval(-86400) - let entries = LogReader.readEntries(since: since, basePath: .documentsDirectory) + let entries = LogReader.readEntries(since: since, basePath: logFileDirectory()) await send(.logEntriesLoaded(entries)) } diff --git a/Sources/Shared/FileLogHandler.swift b/Sources/Shared/FileLogHandler.swift index cbad93be..0dbc26d8 100644 --- a/Sources/Shared/FileLogHandler.swift +++ b/Sources/Shared/FileLogHandler.swift @@ -12,31 +12,48 @@ import Logging extension FileLogHandler { actor FileHandlerOutputStream { private let url: URL - private var nextRotationDate: Date - private var fileHandle: FileHandle + private var nextRotationDate = Date.distantPast + private var fileHandle: FileHandle? + private var hasReportedFailure = false init(basePath url: URL) { self.url = url - self.fileHandle = Self.getNewFileHandle(basePath: url) - self.nextRotationDate = Calendar.current.nextDate(after: Date(), - matching: DateComponents(hour: 0, minute: 0, second: 0), - matchingPolicy: .nextTime)! } func write(_ string: String) { - if Date() > nextRotationDate { - self.fileHandle = Self.getNewFileHandle(basePath: url) - self.nextRotationDate = Calendar.current.nextDate(after: Date(), - matching: DateComponents(hour: 0, minute: 0, second: 0), - matchingPolicy: .nextTime)! + // Opened on the first write and reopened after a failure: before the first unlock the + // app can be woken in the background while its container is still unreadable. + if fileHandle == nil || Date() > nextRotationDate { + do { + fileHandle = try Self.getNewFileHandle(basePath: url) + nextRotationDate = Calendar.current.nextDate(after: Date(), + matching: DateComponents(hour: 0, minute: 0, second: 0), + matchingPolicy: .nextTime) ?? .distantPast + hasReportedFailure = false + } catch { + handleFailure(error) + } } - if let data = string.data(using: .utf8) { - fileHandle.write(data) + guard let fileHandle, let data = string.data(using: .utf8) else { return } + do { + try fileHandle.write(contentsOf: data) + } catch { + handleFailure(error) } } - private static func getNewFileHandle(basePath: URL) -> FileHandle { + /// Drops the handle so the next write reopens the file. Only the first failure in a row is + /// reported, because the report is a log entry itself and comes back through this stream. + private func handleFailure(_ error: any Error) { + fileHandle = nil + guard !hasReportedFailure else { return } + hasReportedFailure = true + Logger(label: "FileLogHandler").error("Log file unavailable, skipping file logging until it can be opened", + metadata: ["path": "\(url.path)", "error": "\(error)"]) + } + + private static func getNewFileHandle(basePath: URL) throws -> FileHandle { let formatter = DateFormatter() formatter.dateFormat = "yyyy-MM-dd" formatter.timeZone = .current @@ -44,19 +61,14 @@ extension FileLogHandler { // format date to "path/to/2024-10-29.txt" let url = basePath.appendingPathComponent(formatter.string(from: date)).appendingPathExtension("txt") + try FileManager.default.createDirectory(at: basePath, withIntermediateDirectories: true) if !FileManager.default.fileExists(atPath: url.path) { - guard FileManager.default.createFile(atPath: url.path, contents: nil, attributes: nil) else { - fatalError("Failed to create log file at \(url.path)") - } + try Data().write(to: url, options: .withoutOverwriting) } - do { - let fileHandle = try FileHandle(forWritingTo: url) - fileHandle.seekToEndOfFile() - return fileHandle - } catch { - fatalError("Failed to open log file at \(url.path): \(error)") - } + let fileHandle = try FileHandle(forWritingTo: url) + try fileHandle.seekToEnd() + return fileHandle } } } diff --git a/Sources/Shared/Log.swift b/Sources/Shared/Log.swift index 499b8fac..0682c2dc 100644 --- a/Sources/Shared/Log.swift +++ b/Sources/Shared/Log.swift @@ -32,6 +32,8 @@ public func initLogging(withFileLogging: Bool, logLevel: Logger.Level) { if let logBasePath { deleteLogFiles(olderThanDays: 7, in: logBasePath) } + // One stream for all labels keeps file access out of `Logger.init`, which runs at launch. + let fileStream = logBasePath.map { FileLogHandler.FileHandlerOutputStream(basePath: $0) } LoggingSystem.bootstrap { label in var handlers: [LogHandler] = [] @@ -42,9 +44,8 @@ public func initLogging(withFileLogging: Bool, logLevel: Logger.Level) { handlers.append(StreamLogHandler.standardOutput(label: label)) #endif - if let logBasePath { - let stream = FileLogHandler.FileHandlerOutputStream(basePath: logBasePath) - handlers.append(FileLogHandler(label: label, stream: stream)) + if let fileStream { + handlers.append(FileLogHandler(label: label, stream: fileStream)) } var mpxHandler = MultiplexLogHandler(handlers) @@ -53,18 +54,16 @@ public func initLogging(withFileLogging: Bool, logLevel: Logger.Level) { } } -/// Directory the `FileLogHandler` writes its daily log files to — a `logs` subfolder so the log -/// files don't clutter the app's Documents directory. -private func logFileDirectory() -> URL { +/// Directory of the daily log files that `FileLogHandler` writes and `LogReader` reads — a `logs` +/// subfolder so the log files don't clutter the app's Documents directory. +public func logFileDirectory() -> URL { #if os(iOS) || os(macOS) || os(watchOS) || os(tvOS) || os(visionOS) let base = URL.documentsDirectory #else // Linux fallback: use the temporary directory. let base = FileManager.default.temporaryDirectory #endif - let dir = base.appendingPathComponent("logs") - try? FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) - return dir + return base.appendingPathComponent("logs") } /// Deletes daily log files (`*.txt`) older than `days` so they don't accumulate on disk diff --git a/Sources/Shared/LogReader.swift b/Sources/Shared/LogReader.swift index b1b85d4e..e21fbe64 100644 --- a/Sources/Shared/LogReader.swift +++ b/Sources/Shared/LogReader.swift @@ -11,7 +11,7 @@ public enum LogReader { /// Read log entries from JSONL files within a time window. /// - Parameters: /// - since: Only return entries at or after this date. - /// - basePath: Directory containing the log files (defaults to Documents). + /// - basePath: Directory containing the log files, usually `logFileDirectory()`. /// - maxEntries: Maximum number of entries to return. /// - Returns: Entries sorted newest-first, capped at `maxEntries`. public static func readEntries(since: Date, basePath: URL, maxEntries: Int = 5000) -> [LogEntry] { diff --git a/Tests/SharedTests/FileLogHandlerTests.swift b/Tests/SharedTests/FileLogHandlerTests.swift new file mode 100644 index 00000000..3e376fd8 --- /dev/null +++ b/Tests/SharedTests/FileLogHandlerTests.swift @@ -0,0 +1,36 @@ +// +// FileLogHandlerTests.swift +// SharedTests +// + +import Foundation +@testable import Shared +import Testing + +struct FileLogHandlerTests { + @Test func writeToUnavailableLogDirectoryDoesNotTerminateTheProcess() async { + await #expect(processExitsWith: .success) { + let directory = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) + // A regular file where the log directory belongs makes every open fail, as a protected + // container does before the first unlock. + FileManager.default.createFile(atPath: directory.path, contents: nil) + let stream = FileLogHandler.FileHandlerOutputStream(basePath: directory) + await stream.write("lost\n") + } + } + + @Test func writeOpensTheLogFileOnceTheDirectoryBecomesAvailable() async throws { + let directory = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) + try #require(FileManager.default.createFile(atPath: directory.path, contents: nil)) + let stream = FileLogHandler.FileHandlerOutputStream(basePath: directory) + + await stream.write("lost\n") + try FileManager.default.removeItem(at: directory) + await stream.write("kept\n") + + let logFiles = try FileManager.default.contentsOfDirectory(at: directory, includingPropertiesForKeys: nil) + let contents = try logFiles.map { try String(contentsOf: $0, encoding: .utf8) }.joined() + #expect(contents == "kept\n") + try FileManager.default.removeItem(at: directory) + } +}