diff --git a/Sources/Observability/AppLogger.swift b/Sources/Observability/AppLogger.swift index a96ab83ca..b497922fd 100644 --- a/Sources/Observability/AppLogger.swift +++ b/Sources/Observability/AppLogger.swift @@ -53,7 +53,13 @@ private actor AppLogFileWriter { private func openHandleIfNeeded() { guard handle == nil, let path = logPath else { return } handle = FileHandle(forWritingAtPath: path) - handle?.seekToEndOfFile() + // seekToEnd() (error-returning) instead of the legacy seekToEndOfFile(), + // which raises an uncatchable ObjC NSException on failure and hard-crashes. + do { + try handle?.seekToEnd() + } catch { + fputs("⚠️ LOGGER | failed to seek debug log: \(ObservabilityTextRedactor.redact(error.localizedDescription))\n", stderr) + } } private func closeHandle() { diff --git a/Sources/Observability/EventReporter.swift b/Sources/Observability/EventReporter.swift index bb949cfd1..f119bb2fd 100644 --- a/Sources/Observability/EventReporter.swift +++ b/Sources/Observability/EventReporter.swift @@ -55,7 +55,7 @@ private actor EventFileWriter { func flushForShutdown() { guard prepareIfNeeded() else { return } flushBufferedInfoEvents() - handle?.synchronizeFile() + try? handle?.synchronize() } private func lineData(for event: ObservabilityEvent) -> Data? { @@ -144,11 +144,16 @@ private actor EventFileWriter { do { handle = try FileHandle(forWritingTo: fileURL) - approximateSize = handle?.seekToEndOfFile() ?? 0 + // seekToEnd() (error-returning) instead of the legacy seekToEndOfFile(), + // which raises an uncatchable ObjC NSException on failure. Any error here + // is caught below and treated as a failed prepare rather than a crash. + approximateSize = try handle?.seekToEnd() ?? 0 isPrepared = true return true } catch { fputs("⚠️ EVENT | failed to open local event log: \(ObservabilityTextRedactor.redact(error.localizedDescription))\n", stderr) + try? handle?.close() + handle = nil return false } } diff --git a/Sources/Observability/LockedFileAppender.swift b/Sources/Observability/LockedFileAppender.swift index 6f6706b05..aa00e7746 100644 --- a/Sources/Observability/LockedFileAppender.swift +++ b/Sources/Observability/LockedFileAppender.swift @@ -14,7 +14,15 @@ enum LockedFileAppender { flock(fd, LOCK_EX) defer { flock(fd, LOCK_UN) } - handle.seekToEndOfFile() - handle.write(data) + do { + // Use the Swift error-returning variants. The legacy + // seekToEndOfFile()/write(_:) raise ObjC NSExceptions on I/O failure + // (disk full, closed fd, rotated file), which Swift cannot catch — so + // they hard-crash the app. Diagnostic logging must never crash the app. + try handle.seekToEnd() + try handle.write(contentsOf: data) + } catch { + fputs("⚠️ LOG | diagnostic append failed: \(ObservabilityTextRedactor.redact(error.localizedDescription))\n", stderr) + } } } diff --git a/Sources/Observability/ReliabilityPacketRecorder.swift b/Sources/Observability/ReliabilityPacketRecorder.swift index 35f31ebfd..8c541a1b7 100644 --- a/Sources/Observability/ReliabilityPacketRecorder.swift +++ b/Sources/Observability/ReliabilityPacketRecorder.swift @@ -40,7 +40,7 @@ private actor ReliabilityPacketFileWriter { approximateSize += UInt64(lineData.count) if approximateSize > TranscriptedConstants.jsonlLogRotationThreshold { // Close so the next append re-prepares, which rotates the file. - handle.synchronizeFile() + try? handle.synchronize() try? handle.close() self.handle = nil isPrepared = false @@ -50,7 +50,7 @@ private actor ReliabilityPacketFileWriter { func flushForShutdown() { guard isPrepared, let handle else { return } - handle.synchronizeFile() + try? handle.synchronize() } private func prepareIfNeeded() -> Bool { @@ -61,7 +61,15 @@ private actor ReliabilityPacketFileWriter { } handle = opened - approximateSize = opened.seekToEndOfFile() + // seekToEnd() (error-returning) instead of the legacy seekToEndOfFile(), + // which raises an uncatchable ObjC NSException on failure and hard-crashes. + // On failure, fall back to a zero size estimate; appends stay crash-safe. + do { + approximateSize = try opened.seekToEnd() + } catch { + fputs("⚠️ RELIABILITY | failed to seek reliability log: \(ObservabilityTextRedactor.redact(error.localizedDescription))\n", stderr) + approximateSize = 0 + } isPrepared = true return true } diff --git a/Sources/TranscriptedCore/Logging/FileLogger.swift b/Sources/TranscriptedCore/Logging/FileLogger.swift index 0a02732a3..5194ce7a6 100644 --- a/Sources/TranscriptedCore/Logging/FileLogger.swift +++ b/Sources/TranscriptedCore/Logging/FileLogger.swift @@ -52,7 +52,9 @@ final class FileLogger: @unchecked Sendable { // Open file handle for appending fileHandle = try? FileHandle(forWritingTo: logFileURL) - fileHandle?.seekToEndOfFile() + // seekToEnd() (error-returning) instead of the legacy seekToEndOfFile(), + // which raises an uncatchable ObjC NSException on failure and hard-crashes. + try? fileHandle?.seekToEnd() // Heal oversized inherited logs on startup even if this process never reaches 100 writes. trimIfNeeded() @@ -74,7 +76,7 @@ final class FileLogger: @unchecked Sendable { func flush() { guard !isDisabled else { return } queue.sync { [weak self] in - self?.fileHandle?.synchronizeFile() + try? self?.fileHandle?.synchronize() } } @@ -99,8 +101,15 @@ final class FileLogger: @unchecked Sendable { // when multiple app instances write to the same log file simultaneously let fd = handle.fileDescriptor flock(fd, LOCK_EX) - handle.seekToEndOfFile() - handle.write(data) + do { + // Error-returning variants: the legacy seekToEndOfFile()/write(_:) + // raise uncatchable ObjC NSExceptions on I/O failure, which would + // hard-crash the app from this background logging path. + try handle.seekToEnd() + try handle.write(contentsOf: data) + } catch { + fputs("⚠️ LOGGER | log append failed: \(LogPrivacySanitizer.sanitizeText(error.localizedDescription))\n", stderr) + } flock(fd, LOCK_UN) } @@ -112,7 +121,7 @@ final class FileLogger: @unchecked Sendable { private func trimIfNeeded() { // Flush buffered writes before reading — otherwise Data(contentsOf:) may miss recent entries - fileHandle?.synchronizeFile() + try? fileHandle?.synchronize() // Open a separate fd for an advisory lock that spans the close/reopen cycle. // The main fileHandle's lock is released when it's closed during trim, @@ -133,7 +142,7 @@ final class FileLogger: @unchecked Sendable { let newContent = trimmed.joined(separator: "\n") + "\n" // Close handle, rewrite file, reopen - fileHandle?.closeFile() + try? fileHandle?.close() try? newContent.write(to: logFileURL, atomically: true, encoding: .utf8) // Security: atomic rewrite creates a replacement inode that may inherit default @@ -148,7 +157,8 @@ final class FileLogger: @unchecked Sendable { FileManager.default.restrictToOwnerOnly(atPath: logFileURL.path) fileHandle = try? FileHandle(forWritingTo: logFileURL) } - fileHandle?.seekToEndOfFile() + // seekToEnd() (error-returning) instead of the crash-prone seekToEndOfFile(). + try? fileHandle?.seekToEnd() } private func escapeJSON(_ string: String) -> String { @@ -161,8 +171,8 @@ final class FileLogger: @unchecked Sendable { } deinit { - fileHandle?.synchronizeFile() - fileHandle?.closeFile() + try? fileHandle?.synchronize() + try? fileHandle?.close() } private static func isEnabledFlag(_ value: String?) -> Bool { diff --git a/Sources/TranscriptedCore/Speaker/RetroactiveSpeakerUpdater.swift b/Sources/TranscriptedCore/Speaker/RetroactiveSpeakerUpdater.swift index c168bbe71..6549bf04a 100644 --- a/Sources/TranscriptedCore/Speaker/RetroactiveSpeakerUpdater.swift +++ b/Sources/TranscriptedCore/Speaker/RetroactiveSpeakerUpdater.swift @@ -1286,9 +1286,11 @@ extension TranscriptSaver { static func extractTranscriptId(from url: URL) -> UUID? { guard let handle = try? FileHandle(forReadingFrom: url) else { return nil } - let header = handle.readData(ofLength: 2048) + // read(upToCount:) (error-returning) instead of the legacy readData(ofLength:), + // which raises an uncatchable ObjC NSException on I/O failure and hard-crashes. + let header = try? handle.read(upToCount: 2048) try? handle.close() - guard let text = String(data: header, encoding: .utf8) else { return nil } + guard let header, let text = String(data: header, encoding: .utf8) else { return nil } return extractTranscriptId(fromFrontmatter: text) } diff --git a/Tests/ObservabilityLogWriterTests.swift b/Tests/ObservabilityLogWriterTests.swift index 8a24e62ce..88b8f0680 100644 --- a/Tests/ObservabilityLogWriterTests.swift +++ b/Tests/ObservabilityLogWriterTests.swift @@ -192,6 +192,69 @@ func testObservabilityLogWriter() { ) } + runSuite("LockedFileAppender swallows write failures instead of crashing the app") { + // The 1.1.48 crash was EventFileWriter.write → LockedFileAppender.append → + // NSFileHandle writeData: → objc_exception_throw → terminate. The legacy + // seekToEndOfFile()/write(_:) raise uncatchable ObjC NSExceptions on I/O + // failure; the error-returning variants must swallow them. If this suite + // regressed, appending to a closed handle would abort the whole runner. + let fm = FileManager.default + let root = fm.temporaryDirectory.appendingPathComponent("LockedFileAppenderCrashSafety-\(UUID().uuidString)", isDirectory: true) + let logURL = root.appendingPathComponent("events.jsonl", isDirectory: false) + try? fm.createDirectory(at: root, withIntermediateDirectories: true) + fm.createFile(atPath: logURL.path, contents: nil) + defer { try? fm.removeItem(at: root) } + + // Open the log read-only, then hand it to the appender as if it were a + // write handle. The fd is live (so flock + fileDescriptor behave), but the + // write fails with EBADF — the same failure shape as the 1.1.48 disk write + // that raised NSFileHandleOperationException from writeData: and crashed. + guard let readOnlyHandle = try? FileHandle(forReadingFrom: logURL) else { + assertTrue(false, "expected to open a read handle") + return + } + defer { try? readOnlyHandle.close() } + + // Must return normally — reaching the next line proves no NSException propagated. + LockedFileAppender.append(Data("{\"crash\":\"safe\"}\n".utf8), to: readOnlyHandle) + + assertTrue(true, "LockedFileAppender.append returned without terminating the process") + + let contents = (try? String(contentsOf: logURL, encoding: .utf8)) ?? "" + assertTrue(contents.isEmpty, "a failed append should be a no-op on disk, not a crash") + } + + runSuite("Diagnostic file writers avoid the NSException-throwing legacy FileHandle APIs") { + // Guards every shipped logging site the 1.1.49 stability pass converted. The + // legacy seekToEndOfFile()/write(_:)/readData(ofLength:) raise uncatchable + // ObjC NSExceptions on I/O failure; a revert to any of them re-arms the crash. + let crashProneAPIs = [ + "seekToEndOfFile()", + "readData(ofLength:", + ".write(data)", + "synchronizeFile()", + "closeFile()", + ] + let sites = [ + "Sources/Observability/LockedFileAppender.swift", + "Sources/Observability/EventReporter.swift", + "Sources/Observability/ReliabilityPacketRecorder.swift", + "Sources/Observability/AppLogger.swift", + "Sources/TranscriptedCore/Logging/FileLogger.swift", + "Sources/TranscriptedCore/Speaker/RetroactiveSpeakerUpdater.swift", + ] + for site in sites { + let source = strippedOfComments(readObservabilityTestRepoTextFile(site)) + assertFalse(source.isEmpty, "expected to read \(site)") + for api in crashProneAPIs { + assertFalse( + source.contains(api), + "\(site) must not call the NSException-throwing \(api) outside comments" + ) + } + } + } + runSuite("LockedFileAppender keeps concurrent log records line-delimited") { let fm = FileManager.default let root = fm.temporaryDirectory.appendingPathComponent("ObservabilityLogWriterTests-\(UUID().uuidString)", isDirectory: true) @@ -240,3 +303,18 @@ private func readObservabilityTestRepoTextFile(_ relativePath: String) -> String .appendingPathComponent(relativePath) return (try? String(contentsOf: url, encoding: .utf8)) ?? "" } + +// Drop `//` line comments so a contract check for a crash-prone API name does not +// trip on the explanatory comments that intentionally reference it. Coarse (it does +// not model strings), which is fine for the diagnostic-logging sources it scans. +private func strippedOfComments(_ source: String) -> String { + source + .split(separator: "\n", omittingEmptySubsequences: false) + .map { line -> Substring in + if let range = line.range(of: "//") { + return line[line.startIndex..