FileLogger: use throwing FileHandle API so a failed write cannot cras… - #31
Open
MartinPrusa wants to merge 1 commit into
Open
MartinPrusa wants to merge 1 commit into
MartinPrusa wants to merge 1 commit into
Conversation
…h the app `seekToEndOfFile()` and `write(_:)` raise an Objective-C `NSFileHandleOperationException` when the underlying write fails (e.g. "No space left on device"). Such an exception cannot be caught from Swift, so a full device storage took the whole app down. Switch to `seekToEnd()` / `write(contentsOf:)` / `close()`, which throw a Swift error that lands in the existing `catch` and is reported through `loggerForInternalErrors` instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
David2XN
requested changes
Sep 12, 2026
| } | ||
| } | ||
|
|
||
| func test_failed_write_is_reported_instead_of_crashing() throws { |
Contributor
There was a problem hiding this comment.
This does not test actual fix at all. I think verify this fix with a unit would be to difficult, so I propose simply removing this and merge it without test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
FileLogger.log(:) appends through the legacy FileHandle.seekToEndOfFile() / write(:) API. When the underlying write fails (typically ENOSPC, "No space left on device", when the device storage is full) Foundation raises an Objective-C NSFileHandleOperationException. That exception cannot be caught from Swift, so the whole app crashes on the logger's background queue.
Fix
Use the throwing FileHandle API (seekToEnd(), write(contentsOf:), close()), available since iOS 13.4 / macOS 10.15.4, well below the package's platform floor. A failed write now throws a Swift error that lands in the existing catch and is reported through loggerForInternalErrors; the entry is dropped and the app keeps running.
currentWritableFileHandle is now internal (was private) so the test can inject a failing handle.
Test
test_failed_write_is_reported_instead_of_crashing swaps the writable handle for a read-only one and logs again. With the old API the test process dies with NSFileHandleOperationException: Bad file descriptor; with this change the error is reported and the previously written content stays intact.