[lib] Populate messageStore.threads from threadInfos before setMessageStoreMessages
ClosedPublic
Actions

Authored by ashoat on Nov 18 2022, 6:07 AM.

Details

Reviewers

atul
tomek

Commits

rCOMM61c4a3f1c829: [lib] Populate messageStore.threads from threadInfos before…

Summary

See ENG-2275, specifically this comment.

We were seeing an exception being thrown because messageStore.threads was missing a thread that was in SQLite. Calling updateMessageStoreWithLatestThreadInfos before this exception might help address the issue by making sure messageStore.threads has an entry for every threadInfo that we have in SQLite.

The exception may still be thrown after this diff, but it would require SQLite's threads table to be inconsistent with its messages table, rather than just redux-persist's messageStore.threads being inconsistent with SQLite's messages table.

Test Plan

Confirm that issue in ENG-2275 no longer repros

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ashoat created this revision.Nov 18 2022, 6:07 AM

Harbormaster completed remote builds in B13548: Diff 18567.Nov 18 2022, 6:18 AM

ashoat requested review of this revision.Nov 18 2022, 6:18 AM

ashoat added a parent revision: D5668: [native] Replace hacky call to imported dispatch in intentionalSaveMedia.Nov 18 2022, 6:18 AM

ashoat added a child revision: D5670: [lib] Introduce newThread helper to message-reducer.js.

ashoat added inline comments.Nov 18 2022, 6:24 AM

lib/reducers/message-reducer.js
189–193 ↗	(On Diff #18567)	These should never have been typed as `$ReadOnly`. The arrays are constructed anew in the function, so it's okay for the caller to do whatever they want with them
571–575 ↗	(On Diff #18567)	Same as above
1364 ↗	(On Diff #18567)	I removed this early exit since `processMessageStoreOperations` is a clean no-op when `messageStoreOperations` is empty anyways
1381–1384 ↗	(On Diff #18567)	This is the same as just returning `processedMessageStore` directly

Actually tested now

Harbormaster completed remote builds in B13590: Diff 18617.Nov 19 2022, 10:45 AM

Tested this and it works to prevent crash, but the messages are all still lost since when the SAVE_MESSAGES action gets processed it ends up triggering updateMessageStoreWithLatestThreadInfos with an empty ThreadStore and thus generating a remove_messages_for_threads op to delete messages from SQLite. D5667 is still needed to prevent the SAVE_MESSAGES action from firing, but this change is still a good change.