HomePhabricator
Diffusion Comm 736f8d5b50f6

[lib] Don't drop messages in MessageStore that don't have a corresponding…

Description

[lib] Don't drop messages in MessageStore that don't have a corresponding ThreadInfo

Summary:
This diff addresses ENG-4452: Messages creating threads are being swallowed.

The root cause is that the code in updateMessageStoreWithLatestThreadInfos required that messages in the MessageStore had a corresponding ThreadInfo in the ThreadStore. More details on this requirement:

  1. This requirement was enforced every time that updateMessageStoreWithLatestThreadInfos was called. This function is triggered on a variety of Redux actions, including state syncs, new messages or updates being pushed to the client, and the user scrolling in the MessageList.
  2. The code that enforces this requirement in updateMessageStoreWithLatestThreadInfos functions by first filtering the ThreadStore based on isThreadWatched, and then filtering the MessageStoreThreads based on the filtered ThreadStore. Pending threads would normally pass isThreadWatched, but since those pending threads don't appear in ThreadStore, they were excluded by this algorithm.
  3. This requirement appears to have been in our code for over 2 years. I didn't get back far enough, but it's likely that it predates the introduction of "pending threads".

The issue resolved by this diff occurred because the "pending threads" code persist pending messages to the MessageStore, but does NOT persist pending threads to the ThreadStore. The aforementioned requirement broke this behavior by filtering out messages that didn't have a corresponding thread in the ThreadStore.

Test Plan:
I was able to get a repro of the underlying issue in my local environment:

  1. Add a sleep(5000) at the start of createThread on the keyserver
  2. Try creating a sidebar by writing a message. Wait for the operation to time out and for FailedSend to appear
  3. Minimize the Comm window so that it disconnects from the keyserver. Then maximize it again so it reconnects to the keyserver
  4. Upon reconnecting, the message you wrote will disappear

After this diff, the issue no longer repros.

Reviewers: atul, kamil, tomek, inka

Reviewed By: tomek

Differential Revision: https://phab.comm.dev/D9222

Details

Provenance
ashoatAuthored on Sep 18 2023, 1:01 PM
Reviewer
tomek
Differential Revision
D9222: [lib] Don't drop messages in MessageStore that don't have a corresponding ThreadInfo
Parents
rCOMM19efb9a5fb8e: [web] Fix capitalization of roles modal
Branches
Unknown
Tags
Unknown