Page MenuHomePhabricator

[lib] Reset Tunnelbroker connection when credentials change
ClosedPublic

Authored by bartek on Jan 26 2024, 1:49 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 1, 4:53 AM
Unknown Object (File)
Fri, Nov 1, 4:53 AM
Unknown Object (File)
Fri, Oct 25, 3:37 AM
Unknown Object (File)
Oct 5 2024, 2:04 PM
Unknown Object (File)
Oct 5 2024, 1:57 PM
Unknown Object (File)
Oct 2 2024, 12:32 PM
Unknown Object (File)
Oct 2 2024, 12:32 PM
Unknown Object (File)
Oct 2 2024, 12:32 PM
Subscribers

Details

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.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage