[lib] Wait for tunnelbroker and keyserver connection when updating relationships
Needs RevisionPublic
Actions

Authored by angelika on Tue, Dec 17, 12:53 PM.

Details

Reviewers

tomek
kamil

Summary

https://linear.app/comm/issue/ENG-9547/dms-created-when-ashoat-opened-web-app-after-not-touching-it-for-a

Implemented solution described by Ashoat here https://linear.app/comm/issue/ENG-9547/dms-created-when-ashoat-opened-web-app-after-not-touching-it-for-a#comment-5c104d82

waitForConnection is a bit ambiguous name but I couldn't think of a better one. waitForTunnelbrokerAndKeyserverConnection seems to long? I'm open to suggestions

Test Plan

Ideally it would be better to test it with Farcaster but I don't have Farcaster account so to simulate the issue:

Have mobile and web clients that do not know each other
Put a sleep() call before connecting to tunnelbroker socket (for example 20s)
Close web client
Send a DM on mobile to web. Make sure it's sent (checkmark)
Open web client
Quickly (before sleep from step 2 ends) add mobile to friends (with Settings -> friends -> add friends)

Before the changes the thread was duplicated
After the changes the app waits for tunnelbroker connection and only one thread is created

Diff Detail

Repository

rCOMM Comm

Branch

graszka22/ENG-9547

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

angelika created this revision.Tue, Dec 17, 12:53 PM

Herald added a subscriber: ashoat. · View Herald TranscriptTue, Dec 17, 12:53 PM

Harbormaster completed remote builds in B33169: Diff 46560.Tue, Dec 17, 1:12 PM

angelika requested review of this revision.Tue, Dec 17, 1:12 PM

I have a couple of questions about this solution:

Are we sure we want to wait for the connection in each place where useUpdateRelationships is used? There are a lot of them.
Should we wait again after e.g. Tunnelbroker gets disconnected, or should we just wait for the initial connection?
Do we want to have a timeout?

lib/hooks/wait-for-connection.js
17–39	This logic looks a bit broken to me: What if the `waitForConnection` function was called multiple times? We're resolving only the most recent promise because creating a new one results in overriding the ref. Is it guaranteed that if `waitForConnection` is called when `isConnectedOrUnreachable` is false, and then turns to true, the effect will always fire late enough so that the ref already contains the resolve function?