Page MenuHomePhabricator

[lib] refactor sequential handling of Tunnelbroker messages
ClosedPublic

Authored by kamil on Aug 14 2024, 4:15 AM.
Tags
None
Referenced Files
F3388994: D13078.diff
Fri, Nov 29, 5:01 PM
F3388617: D13078.diff
Fri, Nov 29, 3:13 PM
Unknown Object (File)
Tue, Nov 26, 9:16 AM
Unknown Object (File)
Tue, Nov 26, 7:38 AM
Unknown Object (File)
Wed, Nov 20, 3:10 AM
Unknown Object (File)
Wed, Nov 20, 3:09 AM
Unknown Object (File)
Wed, Nov 20, 2:44 AM
Unknown Object (File)
Sat, Nov 9, 10:15 AM
Subscribers

Details

Summary

This is a fix for ENG-8968.

In the previous implementation, peerToPeerMessageHandler was not always reactive to state changes, this scenario was possible:

  1. peerToPeerMessageHandler is a callback and memoized based on current dependencies.
  2. tunnelbrokerMessageListener is invoked two times in a row with create_thread, and send_text_message. There is a promise created for create_thread and immediately there is a second one spawned for send_text_message (with the memoized value of peerToPeerMessageHandler).
  3. Promise for create_thread is resolved, it causes some changes in the thread store.
  4. Promise for send_text_message is starting, but it is using the memoized value of peerToPeerMessageHandler (from point 1), at that time, the DM thread is not yet in the store, so this fails because there is no thread associated with the text message.

This fix makes sure, that we always use an updated version of peerToPeerMessageHandler. Instead of changing promises, use messages queue, peerToPeerMessageHandler is called right before execution, not ahead of time.

Test Plan
  1. Open two clients, A and B.
  2. Makes B goes offline.
  3. On A, send thread creation and a couple of text messages.
  4. Open Client B.
  5. Thread and all DM messages are visible.

Additionally, testing this effect does not do any unnecessary computations.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil edited the test plan for this revision. (Show Details)
kamil edited the summary of this revision. (Show Details)
kamil added a reviewer: inka.
kamil published this revision for review.Aug 14 2024, 4:30 AM
lib/tunnelbroker/peer-to-peer-message-handler.js
30 ↗(On Diff #43377)

Usually we want to prevent direct modification of state

This revision is now accepted and ready to land.Aug 19 2024, 6:18 AM

prevent modification of state