Repository navigation
Fix three activity log problems - #26283
Merged
Merged
Conversation
Contributor
|
@jkmassel This PR has conflicts. |
- "Delete Logs" on the Activity Logs screen now deletes every log file, as its alert says, and reloads the list from disk. Removes the now-unused `deleteArchivedLogs`. - The new support screen skips log files it can't read instead of failing the whole list. - Crash reports attach the log file that was being written at the time of the event, instead of always the newest one.
Contributor
Author
|
Sorry about that, fixed! |
crazytonyli
approved these changes
Oct 30, 2026
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
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.
Fixes the three activity log problems listed in #26282. All three are on
trunk. None comes from #26277.Changes
1. "Delete Logs" now deletes every log
The Activity Logs alert says "Are you sure you want to delete all activity logs?", but the button deleted only the older files. It then cleared the list without reloading it. The list looked empty, and the next visit showed the old "Current" log again.
deleteAllLogs(), as the alert promises. This matches the new support screen's "Delete all Logs" and sign-out. The list then reloads fromLogFileStore.deleteArchivedLogs, which nothing else uses.CocoaLumberjack notices that its open file was deleted and starts a new one. Reopening the screen still shows a "Current" row, but that file now starts with the "All log files erased." line.
2. One unreadable file no longer fails the log list
SupportDataProvider.swift:
WpLogDataProviderskips files it can't read. Before, the whole list failed if a file disappeared between listing the folder and reading its details. This was found by reading the code. We did not reproduce it in the app; a unit test covers it.3. Crash reports attach the log from the time of the crash
EventLoggingDataProvider.swift:
logFilePath(forErrorLevel:at:)now returns the newest file created at or before the event's timestamp. Before, it always returned the newest file. A crash is sent on the next launch, and that launch may already have started a new file. If no file has a creation date, it falls back to the newest file. Events sent now (non-fatal events, the Zendesk log upload) still get the newest file, as before.Test plan
LogFileTests.swift:Test run with 5 tests in 2 suites passed.LogFileStoreTests:Test run with 8 tests in 3 suites passed. The simulator checks below ran before stacking. Stacking changed only how the files are listed (AppLogFilefromLogFileStore), not the fixes.EventLoggingDataProviderTestscovers the crash-time file, the newest file for the current session, the fallback, and no files at all.WpLogDataProviderTestscovers a file that no longer exists.Library/Caches/Logs. A temporary launch argument opened the screen directly and was not committed.Files in
Library/Caches/Logsafter confirming Delete Logs:-force-crash-logging YES). A temporary launch argument wrote a marker line to the log and then crashed. A temporaryNSLogprinted the file this PR picks and the file the old code would pick. Neither is committed.In both runs, Sentry's crash event carried the crash time, and the chosen log uploaded (
Finished uploading encrypted log). For B, the crashed file was marked archived before the relaunch to force a new file. That is what happens when the logger rolls over at launch.Closes #26282.