HomePhabricator
Diffusion Comm 33f2b7fa09e3

[lib] Reset Tunnelbroker connection when credentials change

Description

[lib] Reset Tunnelbroker connection when credentials change

Summary:
Attempts to fix ENG-6101.

The problem is that the tunnelbroker connection is not reset when the credentials change, so the
new connection (with new credentials) is established while the old connection is still active.

After a few different approaches, by trial and error, I found this way is the most reliable, in contrast to some complex if-statement conditions.

The flow is as follows:

  1. Credentials change
  2. If the connection is active (either waiting for Tunnelbroker Init Response or connected), but not pending disconnection, then disconnect it by calling socket.close().
  3. The socket.onclose event handler will be called, which will set connected = false.
  4. The connected hook dependency will trigger the effect hook again to establish a new connection.

A few notes:

  • I introduced initMessageChanged because it turned to be the most reliable way of checking it, without unnecessary re-renders.
  • I find that relying on socket.current?.readyState is more reliable than checking socket.current != null.
  • The four states described in the comments somehow overlap with WebSocket.readyState states, with the exception of "CONNECTING" phase being longer because it waits for the initialization response.

Test Plan:
Tested on a few slightly modified scenarions (including anonymous sessions added in next diffs). General test plan is as follows:

  1. Start the app with no credentials - the effect hook should early return.
  2. Add credentials (initMessage) - hook should connect.
  3. After a few seconds, set credentials to something else - hook should disconnect and reconnect (flow described above).
  4. After a few seconds, reset credentials to null - hook should disconnect, further hook calls should early return.

Added some console.logs to help debugging, here's the gist and log outputs:
https://gist.github.com/barthap/64471efe13dd50dec2301b070f25b990

Added a socketID counter to identify which session is being connected/disconnected (according to above test plans steps).

  • socketID=0 - [step 1] initial no-credentials uninitialized session.
  • socketID=1 - [step 2] first session with credentials (above is named "anonymous" due to my testing stack).
  • socketID=2 - [step 3] second session with credentials.

There's no socketID=3 for step 4 - it should not be created.

Reviewers: kamil

Reviewed By: kamil

Subscribers: ashoat, tomek

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

Details

Provenance
bartekAuthored on Jan 26 2024, 1:20 AM
Reviewer
kamil
Differential Revision
D10829: [lib] Reset Tunnelbroker connection when credentials change
Parents
rCOMMc8f5cd547ca5: [native] Bump `@redux-devtools/cli` to `^2.0.0`
Branches
Unknown
Tags
Unknown