[FCM] Fix app launch crash #15559
[FCM] Fix app launch crash #15559
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a crash on app launch related to the FCM database. The changes correctly ensure that the database directory exists before attempting to create the database file, which was the likely cause of the crash. The logic for opening the database has also been refactored for clarity and robustness, including a new mechanism to handle potentially corrupt database files by deleting them. I've included one minor suggestion to improve error logging during this deletion process. Overall, this is a great improvement that should resolve the reported crash.
| // If the file existed, it might be corrupt. Let's remove it and fail. | ||
| if (fileExists) { | ||
| // This is synchronous, but it's on the database queue, so it's safe. | ||
| [[NSFileManager defaultManager] removeItemAtPath:path error:nil]; |
There was a problem hiding this comment.
It's good practice to handle the error that can occur when removing the item at the specified path. While the NSAssert will likely crash the app in debug builds, logging the error from removeItemAtPath:error: can provide valuable diagnostic information, especially in release builds where assertions are disabled and the app might attempt to recover on the next launch.
NSError *removeError = nil;
if (![[NSFileManager defaultManager] removeItemAtPath:path error:&removeError]) {
FIRMessagingLoggerError(kFIRMessagingMessageCodeRmq2PersistentStoreErrorOpeningDatabase,
@"Failed to remove corrupt database file at %@: %@", path,
removeError);
}
paulb777
requested review from
leojaygoogle and
ncooke3
and removed request for
leojaygoogle and
ncooke3
|
More investigation needed. Converted to draft. |
| NSString *errorMessage = | ||
| [NSString stringWithFormat:@"Could not create RMQ database at path %@, error: %@", path, | ||
| errorString]; | ||
| FIRMessagingLoggerError(kFIRMessagingMessageCodeRmq2PersistentStoreErrorCreatingDatabase, | ||
| @"%@", errorMessage); | ||
| NSAssert(NO, errorMessage); |
There was a problem hiding this comment.
More investigation needed. Converted to draft.
From linked issue:
Fatal Exception: NSInternalInconsistencyException
Could not create RMQ database at path /var/mobile/Containers/Data/Application/372D4296-3F35-46FD-B3C9-0A20877B8378/Library/Application Support/Google/FirebaseMessaging/rmq2.sqlite, error: 14 - unable to open database file
This highlighted else clause is where the execution flow reaches the assertion. The assertion's error message is only used here. Entering the else clause means database path exists, so the error has to do with opening a database we know exists in the filesystem.
|
Closing in favor of #15573 |
3 participants
Fix #14880