Session fixes, pt. 1 #3762
Session fixes, pt. 1 #3762
Conversation
|
Your Render PR Server URL is https://social-app-pr-3762.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-coo77dcf7o1s73dikijg. |
|
| clearCurrentAccount() // back user out to login | ||
| setTimeout(() => { | ||
| Toast.show(_(msg`Sorry! We need you to enter your password.`)) |
There was a problem hiding this comment.
This is new but it just shows a toast.
This happens only if something fails (e.g. await selectAccount) above. I don't think we currently have a situation actually triggering this (selectAccount calls initSession but initSession doesn't yet have any codepaths that propagate errors up — so effectively I don't think this toast would ever show until we make more changes).
| logger.error('choose account: initSession failed', { | ||
| message: e.message, | ||
| }) | ||
| onSelectAccount(account) |
There was a problem hiding this comment.
This is the main change here. It ensures that if there's an error from initSession (which, in my understanding, technically can't happen yet — but we could propagate errors in the future properly, which would active this codepath). If this codepath starts firing in the future, an error during initSession will now preselect the account in which you're trying to log in (and ask you to enter the password). Previously, you'd silently stay on the previous "select an account" form with no visual feedback about the failure.
(I'm actually not sure this is better. It's not ideal that we kick you to log in even if the problem isn't solved by a password — e.g. if it's a network issue. But let's capture the intent of who you were trying to log in as, and show some visual feedback.)
We know this state exists so we can iterate on it later.
| */ | ||
| const currentAccountSchema = accountSchema.extend({ | ||
| service: z.string().optional(), | ||
| handle: z.string().optional(), |
There was a problem hiding this comment.
This relaxes the type a little bit so that did is the only required field. We should be careful because this means the app can write a less "complete" version of currentAccount, and if ever roll this back, the old version of the code would not be able to work with that (and would also believe falsehoods about it due to wrong types where these fields were required).
In this PR, there are no changes to how currentAccount is written though — so that should be fine.
| if (session.currentAccount?.did !== state.currentAccount?.did) { | ||
| const selectedAccount = session.accounts.find( | ||
| a => a.did === session.currentAccount?.did, | ||
| ) |
There was a problem hiding this comment.
This just effectively ensures we don't read session.currentAccount and instead read session.accounts.find(a => a.did === session.currentAccount?.did). This way, we don't rely on session.currentAccount for anything other than the DID — so the source of truth is in session.accounts. In theory we could add a new field like session.currentDid instead for a cleaner break, but doesn't seem worth it yet.
So session.accounts is always the source of truth for the actual data. However, note that session.currentAccount?.did plays a decisive role in whether we're going to "find" an account or not. This keeps the existing code (e.g. logout relying on setting currentAccount to undefined) working.
There was a problem hiding this comment.
All of this makes. I did some:
- Switching of accounts
- Log in/out
- Switching branches just to see what would happen w/ schema changes. Seems that switching back to main still works, so that's good (although this is an unlikely scenario for real users anyway)
- Killed my network, failed the switch, then connected again. Things worked fine there. (Obviously the error handling for the network failure isn't good right now)
* origin/main: (39 commits) [Clipclops] Separate messages if there's a time gap (#3777) [Clipclops] Header for chat (#3775) [Clipclops] adjust scroll position, keyboard color (#3771) [Clipclops] Add clop sent time to clipclop (#3772) [Clipclops] Use API data for clipclop list (#3769) [Clipclops] New clipclop dialog (#3750) Session fixes, pt. 1 (#3762) use keyboardDismissMode rather than onScrollBeginDrag (#3767) [Clipclops] Add screen to view and send clip clops (#3754) switch branch for installing `react-native-bottom-sheet` (#3760) Fix List onMomentumScrollEnd (#3759) Release 1.80 (#3757) Update zh-TW translations (#3678) Italian localization (#3684) Update catalan messages.po (#3697) Update Korean localization (#3698) Update Japanese translation (#3734) Update zh-CN translations (#3716) Send Bluesky feeds and suggested follows more data (#3695) android: fix various places still using default Material Teal (#3555) ...
Extracted from #3728.
These changes are supposed to be 100% safe.
See individual commits. I'll comment inline on a few things.
Test Plan
Careful reading. The point of these is that largely you can apply them without testing because they "obviously" work.
Also, some logging in / logging out / switching accounts.
Should be no observable changes.