Page MenuHomePhabricator

Retrieve messages from temporary message storage in AppDelegate
ClosedPublic

Authored by marcin on Jul 5 2022, 5:09 AM.
Tags
None
Referenced Files
F3363284: D4445.diff
Mon, Nov 25, 1:26 AM
Unknown Object (File)
Wed, Nov 13, 4:45 AM
Unknown Object (File)
Tue, Nov 12, 10:59 PM
Unknown Object (File)
Tue, Nov 12, 10:58 PM
Unknown Object (File)
Tue, Nov 12, 10:54 PM
Unknown Object (File)
Tue, Nov 12, 10:53 PM
Unknown Object (File)
Mon, Nov 11, 11:24 AM
Unknown Object (File)
Mon, Nov 11, 11:17 AM

Details

Summary

This differential uses TemporaryMessageStorage API to retrieve messages and store them in SQLite before JavaScript is started.

Test Plan

Send several messages when mobile app is not running. Launch app in a debugger and ensure that all messages are retrieved from temporary message store. Then before last message is written to the database, download database from XCode and ensure that all messages seen so far are written to the database.

Diff Detail

Repository
rCOMM Comm
Branch
marcin/eng-1218
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

marcin requested review of this revision.Jul 5 2022, 6:03 AM

"temporal" is probably the wrong word here, I think you're looking for "temporary". Here is the definition of "temporal" according to Google:

Screen Shot 2022-07-05 at 3.05.10 PM.png (315×610 px, 100 KB)

"temporal" is probably the wrong word here, I think you're looking for "temporary". Here is the definition of "temporal" according to Google:

Understood, I will rename the class and refactor log messages if necessary.

Rename Temporal to Temporary

marcin retitled this revision from Retrieve messages from temporal message storage in AppDelegate to Retrieve messages from temporary message storage in AppDelegate.Jul 7 2022, 7:15 AM
marcin edited the summary of this revision. (Show Details)
marcin edited the test plan for this revision. (Show Details)
tomek added inline comments.
native/ios/Comm/AppDelegate.mm
80 ↗(On Diff #14280)

Can this operation fail / throw an exception? We should probably avoid crashing the app if that happens.

This revision is now accepted and ready to land.Jul 7 2022, 8:28 AM
native/ios/Comm/AppDelegate.mm
80 ↗(On Diff #14280)

This operation is not expected to throw any exception we do not handle inside it implementation - filesystem and semaphore operations are handled inside its implementation, so I do not expect this operation to crash the app. On one hand it might be reasonable to add some try ... catch to prevent app from crashing in case I didn't predict all possible exceptions that might be thrown. On the other hand adding try ... catch might prevent us from being informed that there are some bugs, since application will still work correctly without this operation (though some messages might appear with delay during app startup). Therefore I would not add any try catch statement here

This revision was landed with ongoing or failed builds.Jul 20 2022, 6:18 AM
This revision was automatically updated to reflect the committed changes.