[FCM] Recovery logic for a corrupt database#15573
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. |
paulb777
force-pushed
the
pb-msg-file-corrupt
branch
from
7e4798d to
3cd2386
Compare
|
/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 introduces important recovery logic for a corrupt database, which is a great improvement. The implementation in FIRMessagingRmqManager.m looks mostly good, but I have a few suggestions for improvement regarding error handling and code structure. Additionally, I have a question about the unit test testInitWhenDatabaseIsBrokenThenDatabaseIsDeleted. This test seems to expect an assertion and that the database file is ultimately deleted, which appears to contradict the goal of successfully recovering and recreating the database. Perhaps I'm misunderstanding the test's scenario, and I'd appreciate some clarification.
|
The QS failures are a tooling issue. I think it has to do with when a PR gets force pushed to. Investigating a fix for that. It should be non-blocking for this PR. |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| "Will delete and try to recreate it.", | ||
| path); | ||
| NSError *removeError; | ||
| if (![[NSFileManager defaultManager] removeItemAtPath:path error:&removeError]) { |
There was a problem hiding this comment.
Do we know the exact error code SQLite returns when the database is corrupt? Deleting the database for all error codes seems a bit extreme.
There was a problem hiding this comment.
According to #14880 , the error code is 14
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
How about updating the code to only delete the file if the result is SQLITE_CANTOPEN?
https://sqlite.org/rescode.html
(14) SQLITE_CANTOPEN
The SQLITE_CANTOPEN result code indicates that SQLite was unable to open a file. The file in question might be a primary database file or one of several temporary disk files.
There was a problem hiding this comment.
Sorry about the quick merge. Addressing feedback in #15678
4 participants
Fix #14880. More context in discussion at #15559