-
Notifications
You must be signed in to change notification settings - Fork 392
Fix FileHandler
's potential crash on writing
#5675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📸 Snapshot Test76 modified, 799 unchanged
🛸 Powered by Emerge Tools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, great improvements 🙌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nitpick but looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful! 😗👌 Just a suggestion/question.
func append(line: String) throws { | ||
RCTestAssertNotMainThread() | ||
|
||
self.fileHandle.seekToEndOfFile() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's also seekToEnd()
, which is maybe newer? I genuinely don't know haha.
self.fileHandle.seekToEndOfFile() | |
self.fileHandle.seekToEnd() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, great catch! Thank you! Done in f16e0b9
Fixes #5667
Motivation
The
FileHandler
append(line:)
method was using an older API that would produce a crash when writing to the file fails (e.g. not enough disk space available) instead of throwing an error. See https://developer.apple.com/documentation/foundation/filehandle/write(_:).Description
This PR changes it to use the newer
FileHandle
'swrite(contentsOf:)
API