feat: promises in actions is an antipattern

packages/shared/profiles/ProfileAsPromise.ts

@@ -4,12 +4,25 @@ import { ProfileType } from './types'
 import { store } from 'shared/store/isolatedStore'
 import { Avatar } from '@dcl/schemas'
 import future from 'fp-future'
+import defaultLogger from 'shared/logger'
 
 // This method creates a promise that makes sure that a profile was downloaded AND added to renderer's catalog
 export async function ProfileAsPromise(userId: string, version?: number, profileType?: ProfileType): Promise<Avatar> {
-  const fut = future<Avatar>()
-  store.dispatch(profileRequest(userId, fut, profileType, version))
-  return fut
+  const state = {
+    fut: future<Avatar>(),
+    unsubscribe: function() {},
+    listener: function () {
+      const profile = getProfile(store.getState(), userId)
+      if (profile) {
+        defaultLogger.log('Profile resolved')
+        state.fut.resolve(profile)
+        state.unsubscribe()
+      }
+    }
+  }
+  store.dispatch(profileRequest(userId, state.fut, profileType, version))
+  state.unsubscribe = store.subscribe(state.listener)
+  return state.fut
 }
 
 export function getProfileIfExist(userId: string): Avatar | null {

packages/shared/profiles/sagas.ts

@@ -3,7 +3,7 @@ import { call, put, select, takeEvery, fork, take, debounce, apply } from 'redux
 import { hashV1 } from '@dcl/hashing'
 
 import { ethereumConfigurations, RESET_TUTORIAL, ETHEREUM_NETWORK } from 'config'
-import defaultLogger from 'shared/logger'
+import { createLogger } from 'shared/logger'
 import {
   PROFILE_REQUEST,
   SAVE_PROFILE,
@@ -67,6 +67,8 @@ import { IRealmAdapter } from 'shared/realm/types'
 import { unsignedCRC32 } from 'atomicHelpers/crc32'
 import { DeploymentData } from 'dcl-catalyst-client/dist/utils/DeploymentBuilder'
 
+const logger = createLogger('session')
+
 const concatenatedActionTypeUserId = (action: { type: string; payload: { userId: string } }) =>
   action.type + action.payload.userId
 
@@ -128,7 +130,7 @@ function* initialRemoteProfileLoad() {
   // check that the user still has the claimed name, otherwise pick one
   function selectClaimedName() {
     if (names.length) {
-      defaultLogger.info(`Found missing claimed name '${names[0]}' for profile ${userId}, consolidating profile... `)
+      logger.info(`Found missing claimed name '${names[0]}' for profile ${userId}, consolidating profile... `)
       profile = { ...profile, name: names[0], hasClaimedName: true, tutorialStep: 0xfff }
     } else {
       profile = { ...profile, hasClaimedName: false, tutorialStep: 0x0 }
@@ -158,7 +160,7 @@ export function* handleFetchProfile(action: ProfileRequestAction): any {
 
   const roomConnection: RoomConnection | undefined = yield select(getCommsRoom)
 
-  const loadingMyOwnProfile: boolean = yield select(isCurrentUserId, userId)
+  const loadingCurrentUserProfile: boolean = yield select(isCurrentUserId, userId)
 
   {
     // first check if we have a cached copy of the requested Profile
@@ -170,41 +172,57 @@ export function* handleFetchProfile(action: ProfileRequestAction): any {
 
     if (existingProfileWithCorrectVersion) {
       // resolve the future
-      yield call(future.resolve, existingProfile)
+      // :yield call(future.resolve, existingProfile)
       yield put(profileSuccess(existingProfile))
       return
     }
   }
 
   try {
-    const iAmAGuest: boolean = loadingMyOwnProfile && (yield select(getIsGuestLogin))
-    const shouldReadProfileFromLocalStorage = iAmAGuest
-    const shouldFallbackToLocalStorage = !shouldReadProfileFromLocalStorage && loadingMyOwnProfile
-    const shouldFetchViaComms = roomConnection && profileType === ProfileType.LOCAL && !loadingMyOwnProfile
-    const shouldLoadFromCatalyst =
-      shouldFetchViaComms || (loadingMyOwnProfile && !iAmAGuest) || profileType === ProfileType.DEPLOYED
-    const shouldFallbackToRandomProfile = true
+    const currentUserIsGuest: boolean = yield select(getIsGuestLogin)
+    // Only the current user profile can be read from local storage
+    const canReadProfileFromLocalStorage = currentUserIsGuest || loadingCurrentUserProfile
+    // Can't read guests users from comms, can't read if no roomConnection
+    const canLoadFromComms = !(loadingCurrentUserProfile && currentUserIsGuest) && !!roomConnection
+    const shouldFetchFromRemote = !loadingCurrentUserProfile
+
+    // Was a particular ProfileType requested?
+    const shouldFetchViaComms = profileType === ProfileType.LOCAL && !loadingCurrentUserProfile && !!roomConnection
+    const shouldFetchViaCatalyst = profileType === ProfileType.DEPLOYED && !loadingCurrentUserProfile && !!roomConnection
+
+    const canLoadFromCatalyst = shouldFetchFromRemote || (loadingCurrentUserProfile && !currentUserIsGuest)
 
-    const versionNumber = +(version || '1')
+    // Never fall back for current user to a random profile unless we are guest
+    const canFallbackToRandomProfile = !loadingCurrentUserProfile || currentUserIsGuest
 
-    const reasons = [shouldFetchViaComms ? 'comms' : '', shouldLoadFromCatalyst ? 'catalyst' : ''].join('-')
+    const versionNumber = +(version || '-1')
+
+    logger.log(JSON.stringify({
+      currentUserIsGuest, canReadProfileFromLocalStorage, canLoadFromCatalyst,
+      canLoadFromComms, shouldFetchFromRemote, shouldFetchViaComms,
+      shouldFetchViaCatalyst, canFallbackToRandomProfile, userId, version
+    }, null, 2))
 
     const profile: Avatar =
-      // first fetch avatar through comms
+      // Forced fetch through comms
       (shouldFetchViaComms && (yield call(requestProfileToPeers, roomConnection, userId, versionNumber))) ||
-      // then for my profile, try localStorage
-      (shouldReadProfileFromLocalStorage && (yield call(readProfileFromLocalStorage))) ||
-      // and then via catalyst before localstorage because when you change the name from the builder we need to loaded from the catalyst first.
-      (shouldLoadFromCatalyst && (yield call(getRemoteProfile, userId, loadingMyOwnProfile ? 0 : versionNumber))) ||
-      // last resort, localStorage
-      (shouldFallbackToLocalStorage && (yield call(readProfileFromLocalStorage))) ||
-      // lastly, come up with a random profile
-      (shouldFallbackToRandomProfile && (yield call(generateRandomUserProfile, userId, reasons)))
+      // Forced fetch through catalyst
+      (shouldFetchViaCatalyst && (yield call(getRemoteProfile, userId, loadingCurrentUserProfile ? 0 : versionNumber))) ||
+
+      // Faster is to use localstorage
+      (canReadProfileFromLocalStorage && (yield call(readProfileFromLocalStorage))) ||
+      // Otherwise use the catalyst
+      (canLoadFromCatalyst && (yield call(getRemoteProfile, userId, loadingCurrentUserProfile ? 0 : versionNumber))) ||
+      // Last resort, go through comms
+      (canLoadFromComms && (yield call(requestProfileToPeers, roomConnection, userId, versionNumber))) ||
+      // If everything failed, come up with a random profile
+      (canFallbackToRandomProfile && (yield call(generateRandomUserProfile, userId, 'could not read profile')))
+
 
     const avatar: Avatar = yield call(ensureAvatarCompatibilityFormat, profile)
     avatar.userId = userId
 
-    if (loadingMyOwnProfile && shouldReadProfileFromLocalStorage) {
+    if (loadingCurrentUserProfile && !currentUserIsGuest) {
       // for local user, hasConnectedWeb3 == identity.hasConnectedWeb3
       const identity: ExplorerIdentity | undefined = yield select(getCurrentIdentity)
       avatar.hasConnectedWeb3 = identity?.hasConnectedWeb3 || avatar.hasConnectedWeb3
@@ -235,7 +253,7 @@ function* getRemoteProfile(userId: string, version?: number) {
     if (avatar) {
       avatar = ensureAvatarCompatibilityFormat(avatar)
       if (!validateAvatar(avatar)) {
-        defaultLogger.warn(`Remote avatar for user is invalid.`, userId, avatar, validateAvatar.errors)
+        logger.warn(`Remote avatar for user is invalid.`, userId, avatar, validateAvatar.errors)
         trackEvent(REMOTE_AVATAR_IS_INVALID, {
           avatar
         })
@@ -249,7 +267,7 @@ function* getRemoteProfile(userId: string, version?: number) {
     }
   } catch (error: any) {
     if (error.message !== 'Profile not found') {
-      defaultLogger.warn(`Error requesting profile for auth check ${userId}, `, error)
+      logger.warn(`Error requesting profile for auth check ${userId}, `, error)
     }
   }
   return null
@@ -287,7 +305,7 @@ export function profileServerRequest(userId: string, version?: number): Promise<
 
       return res || { avatars: [], timestamp: Date.now() }
     } catch (e: any) {
-      defaultLogger.error(e)
+      logger.error(e)
       if (key) {
         cachedRequests.delete(key)
       }
@@ -351,7 +369,7 @@ function* handleSaveLocalAvatar(saveAvatar: SaveProfileDelta) {
     } as Avatar
 
     if (!validateAvatar(profile)) {
-      defaultLogger.error('error validating schemas', validateAvatar.errors)
+      logger.error('error validating schemas', validateAvatar.errors)
       trackEvent('invalid_schema', {
         schema: 'avatar',
         payload: profile,
@@ -400,7 +418,7 @@ function* handleDeployProfile(deployProfileAction: DeployProfile) {
       message: 'error deploying profile. ' + e.message,
       stack: e.stacktrace
     })
-    defaultLogger.error('Error deploying profile!', e)
+    logger.error('Error deploying profile!', e)
     yield put(deployProfileFailure(userId, profile, e))
   }
 }
@@ -412,7 +430,7 @@ function* readProfileFromLocalStorage() {
   if (profile && profile.userId === identity.address) {
     try {
       return ensureAvatarCompatibilityFormat(profile)
-    } catch {}
+    } catch { }
     return null
   } else {
     return null
@@ -507,7 +525,7 @@ async function makeContentFile(path: string, content: string | Blob | Buffer): P
 }
 
 export async function generateRandomUserProfile(userId: string, reason: string): Promise<Avatar> {
-  defaultLogger.info('Generating random profile for ' + userId + ' ' + reason)
+  logger.info('Generating random profile for ' + userId + ' ' + reason)
 
   const bytes = new TextEncoder().encode(userId)
 

packages/shared/session/sagas.ts

@@ -145,6 +145,7 @@ function* authenticate(action: AuthenticateAction) {
   yield call(ensureMetaConfigurationInitialized)
   yield put(changeLoginState(LoginState.COMPLETED))
 
+  yield call(waitForRoomConnection)
   if (!isGuest) {
     yield call(referUser, identity)
   }