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
F3048012: D9222.diff
Tue, Oct 22, 9:26 PM
Unknown Object (File)
Tue, Oct 15, 4:25 AM
Unknown Object (File)
Tue, Oct 15, 4:25 AM
Unknown Object (File)
Tue, Oct 15, 4:25 AM
Unknown Object (File)
Tue, Oct 15, 4:25 AM
Unknown Object (File)
Tue, Oct 15, 4:24 AM
Unknown Object (File)
Sun, Oct 13, 11:06 AM
Unknown Object (File)
Sep 18 2024, 4:20 AM
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