From c9eac6c83e3fdc2bd10efc20a39c402e86aa2af2 Mon Sep 17 00:00:00 2001 From: William Grant Date: Wed, 27 Mar 2024 21:27:53 +1100 Subject: [PATCH] feat: handle failing to find a display name from the swarm the user can enter it manually, improved error handling, logging, sync post login --- ts/components/registration/hooks/index.tsx | 31 +++---- .../registration/stages/RestoreAccount.tsx | 57 ++++++++----- ts/session/apis/snode_api/swarmPolling.ts | 26 ++++-- ts/state/onboarding/ducks/registration.ts | 60 +++++++------- ts/state/onboarding/selectors/registration.ts | 80 +++++++++---------- ts/util/accountManager.ts | 29 ++++--- ts/util/storage.ts | 2 + 7 files changed, 164 insertions(+), 121 deletions(-) diff --git a/ts/components/registration/hooks/index.tsx b/ts/components/registration/hooks/index.tsx index dc77f49d4..30d9080cf 100644 --- a/ts/components/registration/hooks/index.tsx +++ b/ts/components/registration/hooks/index.tsx @@ -1,6 +1,6 @@ import { AnyAction } from '@reduxjs/toolkit'; import { isEmpty } from 'lodash'; -import { useEffect } from 'react'; +import { useCallback, useEffect } from 'react'; import { useDispatch } from 'react-redux'; import { ONBOARDING_TIMES } from '../../../session/constants'; import { trigger } from '../../../shims/events'; @@ -9,6 +9,7 @@ import { setAccountRestorationStep, } from '../../../state/onboarding/ducks/registration'; import { registrationDone } from '../../../util/accountManager'; +import { setSignWithRecoveryPhrase } from '../../../util/storage'; let interval: NodeJS.Timeout; @@ -33,6 +34,14 @@ export const useRecoveryProgressEffect = (props: UseRecoveryProgressEffectProps) const dispatch = useDispatch(); + const recoveryComplete = useCallback(async () => { + await setSignWithRecoveryPhrase(true); + await registrationDone(ourPubkey, displayName); + + window.log.debug(`WIP: [onboarding] restore account: loggin in for ${displayName}`); + trigger('openInbox'); + }, [displayName, ourPubkey]); + useEffect(() => { if (step === AccountRestoration.Loading) { interval = setInterval(() => { @@ -44,7 +53,7 @@ export const useRecoveryProgressEffect = (props: UseRecoveryProgressEffectProps) clearInterval(interval); // if we didn't get the display name in time, we need to enter it manually window.log.debug( - `WIP: [useRecoveryProgressEffect] AccountRestoration.Loading We didn't get the display name in time, so we need to enter it manually` + `WIP: [onboarding] restore account: We failed with a time out when fetching a display, so we had to enter it manually` ); dispatch(setAccountRestorationStep(AccountRestoration.DisplayName)); } @@ -68,15 +77,12 @@ export const useRecoveryProgressEffect = (props: UseRecoveryProgressEffectProps) interval = setInterval(() => { clearInterval(interval); if (!isEmpty(displayName)) { - window.log.debug( - `WIP: [useRecoveryProgressEffect] AccountRestoration.Complete Finished progress` - ); dispatch(setAccountRestorationStep(AccountRestoration.Complete)); } else { - dispatch(setAccountRestorationStep(AccountRestoration.DisplayName)); window.log.debug( - `WIP: [onboarding] AccountRestoration.DisplayName failed to fetch display name so we need to enter it manually` + `WIP: [onboarding] restore account: We failed with an error when fetching a display name, so we had to enter it manually` ); + dispatch(setAccountRestorationStep(AccountRestoration.DisplayName)); } }, ONBOARDING_TIMES.RECOVERY_FINISHED); } @@ -84,20 +90,15 @@ export const useRecoveryProgressEffect = (props: UseRecoveryProgressEffectProps) if (step === AccountRestoration.Complete) { clearInterval(interval); if (!isEmpty(ourPubkey) && !isEmpty(displayName)) { - window.log.debug( - `WIP: [onboarding] AccountRestoration.Complete opening inbox for ${displayName}` - ); - - // eslint-disable-next-line more/no-then - void registrationDone(ourPubkey, displayName).then(() => trigger('openInbox')); + void recoveryComplete(); } else { window.log.debug( - `WIP: [onboarding] AccountRestoration.Complete failed to find the pubkey and display name` + `WIP: [onboarding] restore account: We don't have a pubkey or display name` ); dispatch(setAccountRestorationStep(AccountRestoration.DisplayName)); } } return () => clearInterval(interval); - }, [dispatch, displayName, ourPubkey, progress, setProgress, step]); + }, [dispatch, displayName, ourPubkey, progress, recoveryComplete, setProgress, step]); }; diff --git a/ts/components/registration/stages/RestoreAccount.tsx b/ts/components/registration/stages/RestoreAccount.tsx index e667476c6..c72ed60fd 100644 --- a/ts/components/registration/stages/RestoreAccount.tsx +++ b/ts/components/registration/stages/RestoreAccount.tsx @@ -25,7 +25,7 @@ import { useRecoveryPasswordError, } from '../../../state/onboarding/selectors/registration'; import { registerSingleDevice, signInByLinkingDevice } from '../../../util/accountManager'; -import { setSignInByLinking, setSignWithRecoveryPhrase } from '../../../util/storage'; +import { setSignInByLinking } from '../../../util/storage'; import { Flex } from '../../basic/Flex'; import { SessionButton, SessionButtonColor } from '../../basic/SessionButton'; import { SpacerLG, SpacerSM } from '../../basic/Text'; @@ -43,23 +43,26 @@ import { displayNameIsValid, resetRegistration, sanitizeDisplayNameOrToast } fro * Ask for a display name, as we will drop incoming ConfigurationMessages if any are saved on the swarm. * We will handle a ConfigurationMessage */ -async function signInWithNewDisplayName(signInDetails: RecoverDetails) { +async function signInWithNewDisplayName(signInDetails: RecoverDetails, dispatch: Dispatch) { const { displayName, recoveryPassword, errorCallback } = signInDetails; - window.log.debug(`WIP: [signInWithNewDisplayName] starting sign in with new display name....`); try { - const trimName = displayNameIsValid(displayName); + const validDisplayName = displayNameIsValid(displayName); await resetRegistration(); - await registerSingleDevice(recoveryPassword, 'english', trimName); - await setSignInByLinking(false); - await setSignWithRecoveryPhrase(true); + await registerSingleDevice( + recoveryPassword, + 'english', + validDisplayName, + async (pubkey: string) => { + dispatch(setHexGeneratedPubKey(pubkey)); + dispatch(setDisplayName(validDisplayName)); + dispatch(setAccountRestorationStep(AccountRestoration.Finishing)); + } + ); } catch (e) { await resetRegistration(); - errorCallback(e); - window.log.debug( - `WIP: [signInWithNewDisplayName] exception during registration: ${e.message || e}` - ); + void errorCallback(e); } } @@ -72,16 +75,19 @@ async function signInAndFetchDisplayName( /** this is used to trigger the loading animation further down the registration pipeline */ loadingAnimationCallback: () => void; }, + abortSignal: AbortSignal, dispatch: Dispatch ) { const { recoveryPassword, loadingAnimationCallback } = signInDetails; try { await resetRegistration(); + const promiseLink = signInByLinkingDevice( recoveryPassword, 'english', - loadingAnimationCallback + loadingAnimationCallback, + abortSignal ); const promiseWait = PromiseUtils.waitForTask(done => { @@ -90,7 +96,6 @@ async function signInAndFetchDisplayName( async (ourPubkey: string, displayName: string) => { window.Whisper.events.off('configurationMessageReceived'); await setSignInByLinking(false); - await setSignWithRecoveryPhrase(false); dispatch(setHexGeneratedPubKey(ourPubkey)); dispatch(setDisplayName(displayName)); dispatch(setAccountRestorationStep(AccountRestoration.Finishing)); @@ -130,6 +135,7 @@ export const RestoreAccount = () => { return; } + const abortController = new AbortController(); try { window.log.debug( `WIP: [onboarding] restore account: recoverAndFetchDisplayName() is starting recoveryPassword: ${recoveryPassword}` @@ -145,6 +151,7 @@ export const RestoreAccount = () => { dispatch(setAccountRestorationStep(AccountRestoration.Loading)); }, }, + abortController.signal, dispatch ); } catch (e) { @@ -153,6 +160,11 @@ export const RestoreAccount = () => { ); if (e instanceof NotFoundError || e instanceof TaskTimedOutError) { + if (e instanceof TaskTimedOutError) { + // abort fetching the display name from our swarm + window.log.debug(`WIP: [onboarding] restore account: aborting!`); + abortController.abort(); + } dispatch(setAccountRestorationStep(AccountRestoration.DisplayName)); return; } @@ -177,16 +189,17 @@ export const RestoreAccount = () => { window.log.debug( `WIP: [onboarding] restore account: recoverAndEnterDisplayName() is starting recoveryPassword: ${recoveryPassword} displayName: ${displayName}` ); - dispatch(setProgress(0)); - await signInWithNewDisplayName({ - displayName, - recoveryPassword, - errorCallback: e => { - dispatch(setDisplayNameError(e.message || String(e))); - throw e; + await signInWithNewDisplayName( + { + displayName, + recoveryPassword, + errorCallback: e => { + dispatch(setDisplayNameError(e.message || String(e))); + throw e; + }, }, - }); - dispatch(setAccountRestorationStep(AccountRestoration.Complete)); + dispatch + ); } catch (e) { window.log.debug( `WIP: [onboarding] restore account: restoration with new display name failed! Error: ${e.message || e}` diff --git a/ts/session/apis/snode_api/swarmPolling.ts b/ts/session/apis/snode_api/swarmPolling.ts index ce89b10a7..a53fb3ccb 100644 --- a/ts/session/apis/snode_api/swarmPolling.ts +++ b/ts/session/apis/snode_api/swarmPolling.ts @@ -659,8 +659,13 @@ export class SwarmPolling { // return the cached value return this.lastHashes[nodeEdKey][pubkey][namespace]; } - public async pollOnceForOurDisplayName(): Promise { + + public async pollOnceForOurDisplayName(abortSignal?: AbortSignal): Promise { try { + if (abortSignal?.aborted) { + throw new NotFoundError('[pollOnceForOurDisplayName] aborted right away'); + } + const pubkey = UserUtils.getOurPubKeyFromCache(); const polledPubkey = pubkey.key; const swarmSnodes = await snodePool.getSwarmFor(polledPubkey); @@ -675,6 +680,12 @@ export class SwarmPolling { toPollFrom = sample(notPolled) as Snode; } + if (abortSignal?.aborted) { + throw new NotFoundError( + '[pollOnceForOurDisplayName] aborted after selecting nodes to poll from' + ); + } + const resultsFromUserProfile = await SnodeAPIRetrieve.retrieveDisplayName( toPollFrom, pubkey.key @@ -683,7 +694,13 @@ export class SwarmPolling { // check if we just fetched the details from the config namespaces. // If yes, merge them together and exclude them from the rest of the messages. if (!resultsFromUserProfile?.length) { - throw new Error('resultsFromUserProfile is empty'); + throw new NotFoundError('resultsFromUserProfile is empty'); + } + + if (abortSignal?.aborted) { + throw new NotFoundError( + '[pollOnceForOurDisplayName] aborted after retrieving user profile config messages' + ); } const userConfigMessages = resultsFromUserProfile @@ -692,7 +709,7 @@ export class SwarmPolling { const userConfigMessagesMerged = flatten(compact(userConfigMessages)); if (!userConfigMessagesMerged.length) { - throw new Error('after merging there are no user config messages'); + throw new NotFoundError('after merging there are no user config messages'); } const displayName = await this.handleSharedConfigMessages(userConfigMessagesMerged, true); @@ -700,7 +717,7 @@ export class SwarmPolling { throw new NotFoundError('Got a config message from network but without a displayName...'); } - window.log.debug(`WIP: [pollOnceForOurDisplayName] displayName ${displayName}`); + // window.log.debug(`[pollOnceForOurDisplayName] displayName found ${displayName}`); return displayName; } catch (e) { if (e.message === ERROR_CODE_NO_CONNECT) { @@ -710,7 +727,6 @@ export class SwarmPolling { } else if (!window.inboxStore?.getState().onionPaths.isOnline) { window.inboxStore?.dispatch(updateIsOnline(true)); } - window.log.debug(`WIP: [pollOnceForOurDisplayName] no displayName found`, e); throw e; } } diff --git a/ts/state/onboarding/ducks/registration.ts b/ts/state/onboarding/ducks/registration.ts index 9c9fcc466..d4ed0b7c2 100644 --- a/ts/state/onboarding/ducks/registration.ts +++ b/ts/state/onboarding/ducks/registration.ts @@ -34,29 +34,29 @@ export enum AccountRestoration { export type OnboardDirection = 'backward' | 'forward'; export type OnboardingState = { + step: Onboarding; + direction: OnboardDirection; + accountCreationStep: AccountCreation; + accountRestorationStep: AccountRestoration; + progress: number; recoveryPassword: string; recoveryPasswordError: string | undefined; hexGeneratedPubKey: string; displayName: string; displayNameError: string | undefined; - progress: number; - step: Onboarding; - accountCreationStep: AccountCreation; - accountRestorationStep: AccountRestoration; - direction: OnboardDirection; }; const initialState: OnboardingState = { + step: Onboarding.Start, + direction: 'forward', + accountCreationStep: AccountCreation.DisplayName, + accountRestorationStep: AccountRestoration.RecoveryPassword, + progress: 0, recoveryPassword: '', recoveryPasswordError: undefined, hexGeneratedPubKey: '', displayName: '', displayNameError: undefined, - progress: 0, - step: Onboarding.Start, - accountRestorationStep: AccountRestoration.RecoveryPassword, - accountCreationStep: AccountCreation.DisplayName, - direction: 'forward', }; export const registrationSlice = createSlice({ @@ -67,6 +67,21 @@ export const registrationSlice = createSlice({ window.log.debug(`WIP: [onboarding] resetOnboardingState() called`); return { ...initialState }; }, + setOnboardingStep(state, action: PayloadAction) { + return { ...state, step: action.payload }; + }, + setDirection(state, action: PayloadAction) { + return { ...state, direction: action.payload }; + }, + setAccountCreationStep(state, action: PayloadAction) { + return { ...state, accountCreationStep: action.payload }; + }, + setAccountRestorationStep(state, action: PayloadAction) { + return { ...state, accountRestorationStep: action.payload }; + }, + setProgress(state, action: PayloadAction) { + return { ...state, progress: action.payload }; + }, setRecoveryPassword(state, action: PayloadAction) { return { ...state, recoveryPassword: action.payload }; }, @@ -82,35 +97,20 @@ export const registrationSlice = createSlice({ setDisplayNameError(state, action: PayloadAction) { return { ...state, displayNameError: action.payload }; }, - setProgress(state, action: PayloadAction) { - return { ...state, progress: action.payload }; - }, - setOnboardingStep(state, action: PayloadAction) { - return { ...state, step: action.payload }; - }, - setAccountCreationStep(state, action: PayloadAction) { - return { ...state, accountCreationStep: action.payload }; - }, - setAccountRestorationStep(state, action: PayloadAction) { - return { ...state, accountRestorationStep: action.payload }; - }, - setDirection(state, action: PayloadAction) { - return { ...state, direction: action.payload }; - }, }, }); export const { resetOnboardingState, + setOnboardingStep, + setDirection, + setAccountCreationStep, + setAccountRestorationStep, + setProgress, setRecoveryPassword, setRecoveryPasswordError, setHexGeneratedPubKey, setDisplayName, setDisplayNameError, - setProgress, - setOnboardingStep, - setAccountCreationStep, - setAccountRestorationStep, - setDirection, } = registrationSlice.actions; export default registrationSlice.reducer; diff --git a/ts/state/onboarding/selectors/registration.ts b/ts/state/onboarding/selectors/registration.ts index 02de29b61..ba22b5191 100644 --- a/ts/state/onboarding/selectors/registration.ts +++ b/ts/state/onboarding/selectors/registration.ts @@ -14,95 +14,95 @@ const getRegistration = (state: OnboardingStoreState): OnboardingState => { return state.registration; }; -const getRecoveryPassword = createSelector( +const getOnboardingStep = createSelector( getRegistration, - (state: OnboardingState): string => state.recoveryPassword + (state: OnboardingState): Onboarding => state.step ); -const getRecoveryPasswordError = createSelector( +const getDirection = createSelector( getRegistration, - (state: OnboardingState): string | undefined => state.recoveryPasswordError + (state: OnboardingState): OnboardDirection => state.direction ); -const getHexGeneratedPubKey = createSelector( +const getAccountCreationStep = createSelector( getRegistration, - (state: OnboardingState): string => state.hexGeneratedPubKey + (state: OnboardingState): AccountCreation => state.accountCreationStep ); -const getDisplayName = createSelector( +const getAccountRestorationStep = createSelector( getRegistration, - (state: OnboardingState): string => state.displayName + (state: OnboardingState): AccountRestoration => state.accountRestorationStep ); -const getDisplayNameError = createSelector( +const getProgress = createSelector( getRegistration, - (state: OnboardingState): string | undefined => state.displayNameError + (state: OnboardingState): number => state.progress ); -const getProgress = createSelector( +const getRecoveryPassword = createSelector( getRegistration, - (state: OnboardingState): number => state.progress + (state: OnboardingState): string => state.recoveryPassword ); -const getOnboardingStep = createSelector( +const getRecoveryPasswordError = createSelector( getRegistration, - (state: OnboardingState): Onboarding => state.step + (state: OnboardingState): string | undefined => state.recoveryPasswordError ); -const getAccountCreationStep = createSelector( +const getHexGeneratedPubKey = createSelector( getRegistration, - (state: OnboardingState): AccountCreation => state.accountCreationStep + (state: OnboardingState): string => state.hexGeneratedPubKey ); -const getAccountRestorationStep = createSelector( +const getDisplayName = createSelector( getRegistration, - (state: OnboardingState): AccountRestoration => state.accountRestorationStep + (state: OnboardingState): string => state.displayName ); -const getDirection = createSelector( +const getDisplayNameError = createSelector( getRegistration, - (state: OnboardingState): OnboardDirection => state.direction + (state: OnboardingState): string | undefined => state.displayNameError ); // #endregion // #region Hooks -export const useRecoveryPassword = () => { - return useSelector(getRecoveryPassword); -}; - -export const useRecoveryPasswordError = () => { - return useSelector(getRecoveryPasswordError); +export const useOnboardStep = () => { + return useSelector(getOnboardingStep); }; -export const useOnboardHexGeneratedPubKey = () => { - return useSelector(getHexGeneratedPubKey); +export const useOnboardDirection = () => { + return useSelector(getDirection); }; -export const useDisplayName = () => { - return useSelector(getDisplayName); +export const useOnboardAccountCreationStep = () => { + return useSelector(getAccountCreationStep); }; -export const useDisplayNameError = () => { - return useSelector(getDisplayNameError); +export const useOnboardAccountRestorationStep = () => { + return useSelector(getAccountRestorationStep); }; export const useProgress = () => { return useSelector(getProgress); }; -export const useOnboardStep = () => { - return useSelector(getOnboardingStep); +export const useRecoveryPassword = () => { + return useSelector(getRecoveryPassword); }; -export const useOnboardAccountCreationStep = () => { - return useSelector(getAccountCreationStep); +export const useRecoveryPasswordError = () => { + return useSelector(getRecoveryPasswordError); }; -export const useOnboardAccountRestorationStep = () => { - return useSelector(getAccountRestorationStep); +export const useOnboardHexGeneratedPubKey = () => { + return useSelector(getHexGeneratedPubKey); }; -export const useOnboardDirection = () => { - return useSelector(getDirection); +export const useDisplayName = () => { + return useSelector(getDisplayName); +}; + +export const useDisplayNameError = () => { + return useSelector(getDisplayNameError); }; // #endregion diff --git a/ts/util/accountManager.ts b/ts/util/accountManager.ts index 51c0458d6..3addf6db0 100644 --- a/ts/util/accountManager.ts +++ b/ts/util/accountManager.ts @@ -67,12 +67,13 @@ const generateKeypair = async ( export async function signInByLinkingDevice( mnemonic: string, mnemonicLanguage: string, - loadingAnimationCallback: () => void + loadingAnimationCallback: () => void, + abortSignal: AbortSignal ) { - if (!mnemonic) { + if (isEmpty(mnemonic)) { throw new Error('Session always needs a mnemonic. Either generated or given by the user'); } - if (!mnemonicLanguage) { + if (isEmpty(mnemonicLanguage)) { throw new Error('We always needs a mnemonicLanguage'); } @@ -84,7 +85,7 @@ export async function signInByLinkingDevice( await saveRecoveryPhrase(mnemonic); const pubKeyString = toHex(identityKeyPair.pubKey); - const displayName = await getSwarmPollingInstance().pollOnceForOurDisplayName(); + const displayName = await getSwarmPollingInstance().pollOnceForOurDisplayName(abortSignal); if (isEmpty(pubKeyString)) { throw new Error("We don't have a pubkey from the recovery password..."); @@ -106,15 +107,16 @@ export async function signInByLinkingDevice( export async function registerSingleDevice( generatedMnemonic: string, mnemonicLanguage: string, - displayName: string + displayName: string, + restoreCallback?: (pubkey: string) => Promise ) { - if (!generatedMnemonic) { + if (isEmpty(generatedMnemonic)) { throw new Error('Session always need a mnemonic. Either generated or given by the user'); } - if (!mnemonicLanguage) { + if (isEmpty(mnemonicLanguage)) { throw new Error('We always need a mnemonicLanguage'); } - if (!displayName) { + if (isEmpty(displayName)) { throw new Error('We always need a displayName'); } @@ -124,7 +126,16 @@ export async function registerSingleDevice( await saveRecoveryPhrase(generatedMnemonic); const pubKeyString = toHex(identityKeyPair.pubKey); - await registrationDone(pubKeyString, displayName); + if (isEmpty(pubKeyString)) { + throw new Error("We don't have a pubkey from the recovery password..."); + } + + if (restoreCallback) { + // when restoring an account completing the registration is handled by the RestoreAccount component + await restoreCallback(pubKeyString); + } else { + await registrationDone(pubKeyString, displayName); + } } export async function generateMnemonic() { diff --git a/ts/util/storage.ts b/ts/util/storage.ts index 89916f960..2ff785f93 100644 --- a/ts/util/storage.ts +++ b/ts/util/storage.ts @@ -118,6 +118,8 @@ export async function setSignInByLinking(isLinking: boolean) { await put('is_sign_in_by_linking', isLinking); } +/** if we sign in with an existing recovery password, then we don't need to show any of the onboarding ui once we login + */ export function isSignWithRecoveryPhrase() { const isRecoveryPhraseUsed = get('is_sign_in_recovery_phrase'); if (isRecoveryPhraseUsed === undefined) {