From 24975c4e1cfae2b38232f0356a670c98448aeecc Mon Sep 17 00:00:00 2001 From: Michael Lustig Date: Thu, 19 Dec 2024 10:33:21 -0500 Subject: [PATCH 1/8] bugfix: fix notification deeplink to group chat --- features/notifications/utils/onInteractWithNotification.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/features/notifications/utils/onInteractWithNotification.ts b/features/notifications/utils/onInteractWithNotification.ts index daf4f2cd7..dca4ee5ee 100644 --- a/features/notifications/utils/onInteractWithNotification.ts +++ b/features/notifications/utils/onInteractWithNotification.ts @@ -55,12 +55,14 @@ export const onInteractWithNotification = async ( | undefined; if (conversationTopic) { - const account = + // todo(lustig): zod verification of external payloads such as those from + // notifications, deep links, etc + const account: string = notificationData["account"] || useAccountsStore.getState().currentAccount; // Fetch the conversation list to ensure we have the latest conversation list // before navigating to the conversation - await fetchPersistedConversationListQuery(account); + await fetchPersistedConversationListQuery({ account }); useAccountsStore.getState().setCurrentAccount(account, false); navigateToTopic(conversationTopic as ConversationTopic); From 8f6ba7ca02dc1b046609fa4ddac7ae4c104e3525 Mon Sep 17 00:00:00 2001 From: Michael Lustig Date: Thu, 19 Dec 2024 10:55:45 -0500 Subject: [PATCH 2/8] attempt to wait for db to reconnect --- .../utils/onInteractWithNotification.ts | 27 ++++++++++++++++++- utils/xmtpRN/sync.ts | 1 + 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/features/notifications/utils/onInteractWithNotification.ts b/features/notifications/utils/onInteractWithNotification.ts index dca4ee5ee..45e634bd2 100644 --- a/features/notifications/utils/onInteractWithNotification.ts +++ b/features/notifications/utils/onInteractWithNotification.ts @@ -9,6 +9,9 @@ import { useAccountsStore } from "@data/store/accountsStore"; import type { ConversationId, ConversationTopic } from "@xmtp/react-native-sdk"; import { fetchPersistedConversationListQuery } from "@/queries/useConversationListQuery"; import { resetNotifications } from "./resetNotifications"; +import logger from "@/utils/logger"; +import { getXmtpClient } from "@/utils/xmtpRN/sync"; +import { ConverseXmtpClientType } from "@/utils/xmtpRN/client"; export const onInteractWithNotification = async ( event: Notifications.NotificationResponse @@ -62,7 +65,29 @@ export const onInteractWithNotification = async ( // Fetch the conversation list to ensure we have the latest conversation list // before navigating to the conversation - await fetchPersistedConversationListQuery({ account }); + try { + await fetchPersistedConversationListQuery({ account }); + } catch (e) { + logger.error( + `[onInteractWithNotification] Error fetching conversation list from network` + ); + if ( + `${e}`.includes("storage error: Pool needs to reconnect before use") + ) { + logger.info( + `[onInteractWithNotification] Reconnecting XMTP client for account ${account}` + ); + const client = (await getXmtpClient(account)) as ConverseXmtpClientType; + await client.reconnectLocalDatabase(); + logger.info( + `[onInteractWithNotification] XMTP client reconnected for account ${account}` + ); + logger.info( + `[onInteractWithNotification] Fetching conversation list from network for account ${account}` + ); + await fetchPersistedConversationListQuery({ account }); + } + } useAccountsStore.getState().setCurrentAccount(account, false); navigateToTopic(conversationTopic as ConversationTopic); diff --git a/utils/xmtpRN/sync.ts b/utils/xmtpRN/sync.ts index 936d644ae..8d339efc4 100644 --- a/utils/xmtpRN/sync.ts +++ b/utils/xmtpRN/sync.ts @@ -26,6 +26,7 @@ const instantiatingClientForAccount: { export const getXmtpClient = async ( account: string + // todo(any): why is this Union necessary? Why can't we just use ConverseXmtpClientType? ): Promise => { const lowerCaseAccount = account.toLowerCase(); if (account && xmtpClientByAccount[lowerCaseAccount]) { From cfa629993da1c0c143097cf32e325d7f559ab8f0 Mon Sep 17 00:00:00 2001 From: Michael Lustig Date: Thu, 19 Dec 2024 12:09:48 -0500 Subject: [PATCH 3/8] wait for xmtp client hydration; cleanup nav --- data/store/appStore.ts | 27 ++++++++++++ .../utils/onInteractWithNotification.ts | 41 +++-------------- screens/Main.tsx | 18 +++++--- utils/navigation.ts | 44 ++++++++++++++----- 4 files changed, 75 insertions(+), 55 deletions(-) diff --git a/data/store/appStore.ts b/data/store/appStore.ts index 52035a28c..82210f3df 100644 --- a/data/store/appStore.ts +++ b/data/store/appStore.ts @@ -2,6 +2,7 @@ import { create } from "zustand"; import { persist, createJSONStorage } from "zustand/middleware"; import { zustandMMKVStorage } from "../../utils/mmkv"; +import logger from "@/utils/logger"; // A app-wide store to store settings that don't depend on // an account like if notifications are accepted @@ -85,3 +86,29 @@ export const useAppStore = create()( } ) ); + +/** + * Utility function to wait for the app store to be hydrated. + * This can be used, for example, to ensure that the xmtp clients have been instantiated and the + * conversation list is fetched before navigating to a conversation. + * + * As of December 19, 2024, when we say Hydration, we mean that the following are true: + * 1) XMTP client for all accounts added to device have been instantiated and cached + * 2) Conversation list for all accounts added to device have been fetched from the network and cached + * 3) Inbox ID for all accounts added to device have been fetched from the network and cached + * + * You can observe that logic in the HydrationStateHandler, and that will likely be moved once + * we refactor accounts to be InboxID based in upcoming refactors. + */ +export const waitForXmtpClientHydration = async () => { + logger.debug( + `[waitForAppStoreHydration] Waiting for app store hydration to complete` + ); + while (!useAppStore.getState().hydrationDone) { + logger.debug( + `[waitForAppStoreHydration] App store hydration not complete, waiting 100ms...` + ); + await new Promise((resolve) => setTimeout(resolve, 100)); + } + logger.debug(`[waitForAppStoreHydration] App store hydration complete`); +}; diff --git a/features/notifications/utils/onInteractWithNotification.ts b/features/notifications/utils/onInteractWithNotification.ts index 45e634bd2..569169175 100644 --- a/features/notifications/utils/onInteractWithNotification.ts +++ b/features/notifications/utils/onInteractWithNotification.ts @@ -1,21 +1,16 @@ import * as Notifications from "expo-notifications"; -import { - navigate, - navigateToTopic, - setTopicToNavigateTo, -} from "@utils/navigation"; +import { navigate, navigateToTopic } from "@utils/navigation"; import { getTopicFromV3Id } from "@utils/groupUtils/groupId"; import { useAccountsStore } from "@data/store/accountsStore"; import type { ConversationId, ConversationTopic } from "@xmtp/react-native-sdk"; -import { fetchPersistedConversationListQuery } from "@/queries/useConversationListQuery"; import { resetNotifications } from "./resetNotifications"; -import logger from "@/utils/logger"; -import { getXmtpClient } from "@/utils/xmtpRN/sync"; -import { ConverseXmtpClientType } from "@/utils/xmtpRN/client"; +import { waitForXmtpClientHydration } from "@/data/store/appStore"; +import logger from "@utils/logger"; export const onInteractWithNotification = async ( event: Notifications.NotificationResponse ) => { + logger.debug("[onInteractWithNotification]"); let notificationData = event.notification.request.content.data; // Android returns the data in the body as a string if ( @@ -58,40 +53,14 @@ export const onInteractWithNotification = async ( | undefined; if (conversationTopic) { + await waitForXmtpClientHydration(); // todo(lustig): zod verification of external payloads such as those from // notifications, deep links, etc const account: string = notificationData["account"] || useAccountsStore.getState().currentAccount; - - // Fetch the conversation list to ensure we have the latest conversation list - // before navigating to the conversation - try { - await fetchPersistedConversationListQuery({ account }); - } catch (e) { - logger.error( - `[onInteractWithNotification] Error fetching conversation list from network` - ); - if ( - `${e}`.includes("storage error: Pool needs to reconnect before use") - ) { - logger.info( - `[onInteractWithNotification] Reconnecting XMTP client for account ${account}` - ); - const client = (await getXmtpClient(account)) as ConverseXmtpClientType; - await client.reconnectLocalDatabase(); - logger.info( - `[onInteractWithNotification] XMTP client reconnected for account ${account}` - ); - logger.info( - `[onInteractWithNotification] Fetching conversation list from network for account ${account}` - ); - await fetchPersistedConversationListQuery({ account }); - } - } useAccountsStore.getState().setCurrentAccount(account, false); navigateToTopic(conversationTopic as ConversationTopic); - setTopicToNavigateTo(undefined); resetNotifications(); } }; diff --git a/screens/Main.tsx b/screens/Main.tsx index 81389ebac..2ec9f7d95 100644 --- a/screens/Main.tsx +++ b/screens/Main.tsx @@ -21,7 +21,11 @@ import { useThemeProvider } from "../theme/useAppTheme"; import { useAddressBookStateHandler } from "../utils/addressBook"; import { useAutoConnectExternalWallet } from "../utils/evm/external"; import { usePrivyAccessToken } from "../utils/evm/privy"; -import { converseNavigations } from "../utils/navigation"; +import { + converseNavigations, + converseNavigatorRef, + setConverseNavigatorRef, +} from "../utils/navigation"; import { ConversationScreenConfig } from "../features/conversation/conversation.nav"; import { GroupScreenConfig } from "./Navigation/GroupNav"; import { @@ -39,6 +43,7 @@ import { getConverseStateFromPath, } from "./Navigation/navHelpers"; import { JoinGroupScreenConfig } from "@/features/GroupInvites/joinGroup/JoinGroupNavigation"; +import logger from "@/utils/logger"; const prefix = Linking.createURL("/"); @@ -84,9 +89,10 @@ export default function Main() { theme={navigationTheme} linking={linking} ref={(r) => { - if (r) { - converseNavigations["main"] = r; - } + logger.info( + `[Main] Setting navigation ref to ${r ? "not null" : "null"}` + ); + setConverseNavigatorRef(r); }} onUnhandledAction={() => { // Since we're handling multiple navigators, @@ -105,9 +111,7 @@ export default function Main() { const NavigationContent = () => { const authStatus = useAuthStatus(); - const { splashScreenHidden } = useAppStore( - useSelect(["notificationsPermissionStatus", "splashScreenHidden"]) - ); + const { splashScreenHidden } = useAppStore(useSelect(["splashScreenHidden"])); // Uncomment to test design system components // return ( diff --git a/utils/navigation.ts b/utils/navigation.ts index b4892804c..b411709a6 100644 --- a/utils/navigation.ts +++ b/utils/navigation.ts @@ -3,32 +3,52 @@ import { Linking as RNLinking } from "react-native"; import logger from "./logger"; import config from "../config"; -import { currentAccount, getChatStore } from "../data/store/accountsStore"; import { NavigationParamList } from "../screens/Navigation/Navigation"; -import type { ConversationWithCodecsType } from "./xmtpRN/client"; import type { ConversationTopic } from "@xmtp/react-native-sdk"; +import { NavigationContainerRef } from "@react-navigation/native"; -export const converseNavigations: { [navigationName: string]: any } = {}; +let converseNavigatorRef: NavigationContainerRef | null = + null; + +export const setConverseNavigatorRef = ( + ref: NavigationContainerRef | null +) => { + if (converseNavigatorRef) { + logger.error("[Navigation] Conversation navigator ref already set"); + return; + } + if (!ref) { + logger.error("[Navigation] Conversation navigator ref is null"); + return; + } + converseNavigatorRef = ref; +}; export const navigate = async ( screen: T, params?: NavigationParamList[T] ) => { + if (!converseNavigatorRef) { + logger.error("[Navigation] Conversation navigator not found"); + return; + } + + if (!converseNavigatorRef.isReady()) { + logger.error( + "[Navigation] Conversation navigator is not ready (wait for appStore#hydrated to be true using waitForAppStoreHydration)" + ); + return; + } + logger.debug( `[Navigation] Navigating to ${screen} ${ params ? JSON.stringify(params) : "" }` ); - // Navigate to a screen in all navigators - // like Linking.openURL but without redirect on web - Object.values(converseNavigations).forEach((navigation) => { - navigation.navigate(screen, params); - }); -}; -export let topicToNavigateTo: string | undefined = undefined; -export const setTopicToNavigateTo = (topic: string | undefined) => { - topicToNavigateTo = topic; + // todo(any): figure out proper typing here + // @ts-ignore + converseNavigatorRef.navigate(screen, params); }; export const navigateToTopic = async (topic: ConversationTopic) => { From 0a0fa5b6a0387b1791b72f95df4d7ccb360070e9 Mon Sep 17 00:00:00 2001 From: Michael Lustig Date: Thu, 19 Dec 2024 12:15:15 -0500 Subject: [PATCH 4/8] Update onInteractWithNotification.ts --- features/notifications/utils/onInteractWithNotification.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/notifications/utils/onInteractWithNotification.ts b/features/notifications/utils/onInteractWithNotification.ts index 569169175..288ddabc1 100644 --- a/features/notifications/utils/onInteractWithNotification.ts +++ b/features/notifications/utils/onInteractWithNotification.ts @@ -11,6 +11,8 @@ export const onInteractWithNotification = async ( event: Notifications.NotificationResponse ) => { logger.debug("[onInteractWithNotification]"); + // todo(lustig): zod verification of external payloads such as those from + // notifications, deep links, etc let notificationData = event.notification.request.content.data; // Android returns the data in the body as a string if ( @@ -54,8 +56,6 @@ export const onInteractWithNotification = async ( if (conversationTopic) { await waitForXmtpClientHydration(); - // todo(lustig): zod verification of external payloads such as those from - // notifications, deep links, etc const account: string = notificationData["account"] || useAccountsStore.getState().currentAccount; useAccountsStore.getState().setCurrentAccount(account, false); From b02e0fb080c0213196ded8e40d381092225758d8 Mon Sep 17 00:00:00 2001 From: Michael Lustig Date: Thu, 19 Dec 2024 12:32:17 -0500 Subject: [PATCH 5/8] typescript --- dependencies/Environment/Flavors/determineFlavor.utils.ts | 2 ++ screens/Main.tsx | 6 +----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/dependencies/Environment/Flavors/determineFlavor.utils.ts b/dependencies/Environment/Flavors/determineFlavor.utils.ts index 7c33358db..b3931efc0 100644 --- a/dependencies/Environment/Flavors/determineFlavor.utils.ts +++ b/dependencies/Environment/Flavors/determineFlavor.utils.ts @@ -1,6 +1,8 @@ import { DependencyFlavor, DependencyFlavors } from "./flavors.type"; export function determineDependencyFlavor(): DependencyFlavor { + // todo(lustig): remove this once we have a better way to determine the flavor + // @ts-ignore if (typeof jest !== "undefined" || process.env.JEST_WORKER_ID !== undefined) { return DependencyFlavors.jest; } diff --git a/screens/Main.tsx b/screens/Main.tsx index 2ec9f7d95..d3aa9ce2e 100644 --- a/screens/Main.tsx +++ b/screens/Main.tsx @@ -21,11 +21,7 @@ import { useThemeProvider } from "../theme/useAppTheme"; import { useAddressBookStateHandler } from "../utils/addressBook"; import { useAutoConnectExternalWallet } from "../utils/evm/external"; import { usePrivyAccessToken } from "../utils/evm/privy"; -import { - converseNavigations, - converseNavigatorRef, - setConverseNavigatorRef, -} from "../utils/navigation"; +import { setConverseNavigatorRef } from "../utils/navigation"; import { ConversationScreenConfig } from "../features/conversation/conversation.nav"; import { GroupScreenConfig } from "./Navigation/GroupNav"; import { From e6dc4495aa11c8c685401aedb20bedceade58609 Mon Sep 17 00:00:00 2001 From: Michael Lustig Date: Thu, 19 Dec 2024 13:44:11 -0500 Subject: [PATCH 6/8] better waiting logic based on subscribe instead of polling --- data/store/appStore.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/data/store/appStore.ts b/data/store/appStore.ts index 82210f3df..0154535fa 100644 --- a/data/store/appStore.ts +++ b/data/store/appStore.ts @@ -100,15 +100,19 @@ export const useAppStore = create()( * You can observe that logic in the HydrationStateHandler, and that will likely be moved once * we refactor accounts to be InboxID based in upcoming refactors. */ -export const waitForXmtpClientHydration = async () => { - logger.debug( - `[waitForAppStoreHydration] Waiting for app store hydration to complete` - ); - while (!useAppStore.getState().hydrationDone) { - logger.debug( - `[waitForAppStoreHydration] App store hydration not complete, waiting 100ms...` - ); - await new Promise((resolve) => setTimeout(resolve, 100)); - } - logger.debug(`[waitForAppStoreHydration] App store hydration complete`); +export const waitForXmtpClientHydration = (): Promise => { + return new Promise((resolve) => { + const { hydrationDone } = useAppStore.getState(); + if (hydrationDone) { + resolve(); + return; + } + + const unsubscribe = useAppStore.subscribe((state, prevState) => { + if (state.hydrationDone && !prevState.hydrationDone) { + resolve(); + unsubscribe(); + } + }); + }); }; From 2ebf4f097dcf44b676d32478e6376f769d2431a1 Mon Sep 17 00:00:00 2001 From: Michael Lustig Date: Thu, 19 Dec 2024 13:49:00 -0500 Subject: [PATCH 7/8] use store subscription rather than poll; add timeout (defaults to 1 second) --- data/store/appStore.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/data/store/appStore.ts b/data/store/appStore.ts index 0154535fa..6b36ecba7 100644 --- a/data/store/appStore.ts +++ b/data/store/appStore.ts @@ -92,7 +92,9 @@ export const useAppStore = create()( * This can be used, for example, to ensure that the xmtp clients have been instantiated and the * conversation list is fetched before navigating to a conversation. * - * As of December 19, 2024, when we say Hydration, we mean that the following are true: + * @param timeout - The maximum time to wait for hydration in milliseconds. Defaults to 1000ms (1 second). + * + * As of December 19, 2024, when we say XMTP client hydration, we mean that the following are true: * 1) XMTP client for all accounts added to device have been instantiated and cached * 2) Conversation list for all accounts added to device have been fetched from the network and cached * 3) Inbox ID for all accounts added to device have been fetched from the network and cached @@ -100,8 +102,10 @@ export const useAppStore = create()( * You can observe that logic in the HydrationStateHandler, and that will likely be moved once * we refactor accounts to be InboxID based in upcoming refactors. */ -export const waitForXmtpClientHydration = (): Promise => { - return new Promise((resolve) => { +export const waitForXmtpClientHydration = ( + timeout: number = 1000 +): Promise => { + const hydrationPromise = new Promise((resolve) => { const { hydrationDone } = useAppStore.getState(); if (hydrationDone) { resolve(); @@ -115,4 +119,12 @@ export const waitForXmtpClientHydration = (): Promise => { } }); }); + + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => { + reject(new Error("Xmtp client hydration timed out")); + }, timeout); + }); + + return Promise.race([hydrationPromise, timeoutPromise]); }; From 807aff57857f447d4b0a13cd2e4b12a83a7c62be Mon Sep 17 00:00:00 2001 From: Michael Lustig Date: Thu, 19 Dec 2024 15:40:16 -0500 Subject: [PATCH 8/8] refacgtor waitForXmtpClientHydration to use subscription to store; wait instead of sleep --- data/store/appStore.ts | 39 ++++++++++++------- .../joinGroup/JoinGroup.client.ts | 6 +-- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/data/store/appStore.ts b/data/store/appStore.ts index 6b36ecba7..f4d3045a7 100644 --- a/data/store/appStore.ts +++ b/data/store/appStore.ts @@ -3,6 +3,7 @@ import { persist, createJSONStorage } from "zustand/middleware"; import { zustandMMKVStorage } from "../../utils/mmkv"; import logger from "@/utils/logger"; +import { wait } from "@/utils/wait"; // A app-wide store to store settings that don't depend on // an account like if notifications are accepted @@ -92,8 +93,6 @@ export const useAppStore = create()( * This can be used, for example, to ensure that the xmtp clients have been instantiated and the * conversation list is fetched before navigating to a conversation. * - * @param timeout - The maximum time to wait for hydration in milliseconds. Defaults to 1000ms (1 second). - * * As of December 19, 2024, when we say XMTP client hydration, we mean that the following are true: * 1) XMTP client for all accounts added to device have been instantiated and cached * 2) Conversation list for all accounts added to device have been fetched from the network and cached @@ -102,29 +101,43 @@ export const useAppStore = create()( * You can observe that logic in the HydrationStateHandler, and that will likely be moved once * we refactor accounts to be InboxID based in upcoming refactors. */ -export const waitForXmtpClientHydration = ( - timeout: number = 1000 -): Promise => { +export const waitForXmtpClientHydration = (): Promise => { const hydrationPromise = new Promise((resolve) => { const { hydrationDone } = useAppStore.getState(); if (hydrationDone) { + logger.debug( + "[waitForXmtpClientHydrationWithTimeout] Already hydrated, resolving" + ); resolve(); return; } - const unsubscribe = useAppStore.subscribe((state, prevState) => { + logger.debug( + "[waitForXmtpClientHydrationWithTimeout] Not hydrated, subscribing" + ); + const unsubscribe = useAppStore.subscribe(async (state, prevState) => { + logger.debug( + `[waitForXmtpClientHydrationWithTimeout] Hydration state changed: ${prevState.hydrationDone} -> ${state.hydrationDone}` + ); if (state.hydrationDone && !prevState.hydrationDone) { + logger.debug( + `[waitForXmtpClientHydrationWithTimeout] waiting a split second before resolving to allow next render` + ); + + // Wait for the next render to complete + // note(lustig): this is a hack to ensure that the next render has completed + // as this is used to navigate to a conversation currently. + // We'll revisit this and make something that doesn't suck as much later. + await wait(1); + + logger.debug( + `[waitForXmtpClientHydrationWithTimeout] resolving promise` + ); resolve(); unsubscribe(); } }); }); - const timeoutPromise = new Promise((_, reject) => { - setTimeout(() => { - reject(new Error("Xmtp client hydration timed out")); - }, timeout); - }); - - return Promise.race([hydrationPromise, timeoutPromise]); + return hydrationPromise; }; diff --git a/features/GroupInvites/joinGroup/JoinGroup.client.ts b/features/GroupInvites/joinGroup/JoinGroup.client.ts index cc452477f..f2a467826 100644 --- a/features/GroupInvites/joinGroup/JoinGroup.client.ts +++ b/features/GroupInvites/joinGroup/JoinGroup.client.ts @@ -24,11 +24,11 @@ import { JoinGroupResult } from "./joinGroup.types"; import { ConversationListQueryData } from "@/queries/useConversationListQuery"; import { entify } from "@/queries/entify"; import { GroupWithCodecsType } from "@/utils/xmtpRN/client"; +import { wait } from "@/utils/wait"; const GROUP_JOIN_REQUEST_POLL_MAX_ATTEMPTS = 10; const GROUP_JOIN_REQUEST_POLL_INTERVAL_MS = 1000; -const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); /** * TODOs: * @@ -187,7 +187,7 @@ export class JoinGroupClient { logger.debug( `[liveAttemptToJoinGroup] Waiting ${GROUP_JOIN_REQUEST_POLL_INTERVAL_MS}ms before next poll` ); - await sleep(GROUP_JOIN_REQUEST_POLL_INTERVAL_MS); + await wait(GROUP_JOIN_REQUEST_POLL_INTERVAL_MS); } logger.warn( @@ -522,7 +522,7 @@ export class JoinGroupClient { account: string, groupInviteId: string ): Promise => { - await sleep(5000); + await wait(5000); return { type: "group-join-request.timed-out", } as const;