Page MenuHomePhabricator

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

Authored by ashoat on Sep 18 2023, 1:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jul 13, 9:23 AM
Unknown Object (File)
Sun, Jul 7, 11:32 PM
Unknown Object (File)
Sun, Jul 7, 8:10 AM
Unknown Object (File)
Thu, Jul 4, 8:36 PM
Unknown Object (File)
Tue, Jul 2, 10:35 AM
Unknown Object (File)
Sat, Jun 29, 1:05 PM
Unknown Object (File)
Mon, Jun 24, 2:35 PM
Unknown Object (File)
Mon, Jun 24, 2:35 PM
Subscribers
None

Details

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.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable