[lib][web][native] Stop logging out from the Socket
ClosedPublic
Actions

Authored by inka on Dec 14 2023, 4:31 AM.

Details

Reviewers

Commits

Summary

issue: https://linear.app/comm/issue/ENG-5914/stop-calling-logout-from-socket
We want to stop logging out from the Socket component. Temporarily, after this action is dispatched we do call logout from KeyserverConnectionHandler (see D10331). So the behaviour from users perspective stays the same

Test Plan

Forced the dispatch from all of those three places, checked that the user is logged out and no errors show up

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

inka requested review of this revision.Dec 14 2023, 4:48 AM

This revision is now accepted and ready to land.Dec 14 2023, 9:43 AM

I'm not sure @ginsu makes sense as a reviewer for this stack... he really doesn't have context. I'm not sure the other reviewers do either.

How about @tomek? He's been looking at this closely with you, right?

lib/socket/socket.react.js
696 ↗	(On Diff #34626)	Shouldn't this be more specific? This doesn't happen on any socket crash loop... it only happens when we're seeing a socket crash loop after policy acknowledgement
native/socket.react.js
124 ↗	(On Diff #34626)	I'm concerned that we're using `socket_crash_loop` in two places. Can you explain the differences between these two callsites, and why it makes sense to treat them the same?

This revision now requires changes to proceed.Dec 15 2023, 10:12 AM

inka added inline comments.

lib/socket/socket.react.js
696 ↗	(On Diff #34626)	I will amend D10287
native/socket.react.js
124 ↗	(On Diff #34626)	Both of those places are called in case of a socket crash loop connected to `noDataAfterPolicyAcknowledgment`. And in both those cases we were not able to recover from the socket crash loop. Either because the user is an ENS user and we just have no way of recovering for them (this call site here), or the user is a password user, but the recovery failed for some other reason (the call site in lib/socket/socket.react.js). It's worth noting that after @varun creates the new auth endpoint on the keyserver, the ENS users will be able to retry, like password users can now. So this whole `if (!accountHasPassword(currentUserInfo))` will be just removed, and only the call site in lib will remain. I think there was a task tracking this, but I can't find it. @varun do you know which task it was?