refactor: improve error handling of extension tab api interactions

input-output-hk · Feb 21, 2025 · b4a5351 · b4a5351
1 parent 958032a
commit b4a5351
Show file tree

Hide file tree

Showing 20 changed files with 255 additions and 105 deletions.
diff --git a/apps/browser-extension-wallet/src/components/Layout/MainLayout.tsx b/apps/browser-extension-wallet/src/components/Layout/MainLayout.tsx
@@ -52,7 +52,7 @@ export const MainLayout = ({
     getAboutExtensionData();
   }, [getAboutExtensionData]);
 
-  const onUpdateAknowledge = useCallback(async () => {
+  const onUpdateAcknowledge = useCallback(async () => {
     const data = { version, acknowledged: true, reason };
     await storage.local.set({
       [ABOUT_EXTENSION_KEY]: data
@@ -72,7 +72,7 @@ export const MainLayout = ({
       </div>
       <Announcement
         visible={showAnnouncement && version && !acknowledged}
-        onConfirm={onUpdateAknowledge}
+        onConfirm={onUpdateAcknowledge}
         version={version}
         reason={reason}
       />

diff --git a/apps/browser-extension-wallet/src/features/dapp/components/ConfirmData.tsx b/apps/browser-extension-wallet/src/features/dapp/components/ConfirmData.tsx
@@ -56,16 +56,27 @@ export const DappConfirmData = (): React.ReactElement => {
     dataToSign: string;
   }>();
 
-  const cancelTransaction = useCallback(async () => {
-    await req.reject('User rejected to sign');
-    window.close();
-  }, [req]);
+  const cancelTransaction = useCallback(
+    async (reason = 'User rejected to sign') => {
+      await req.reject(reason);
+      window.close();
+    },
+    [req]
+  );
 
   useOnUnload(cancelTransaction);
 
   useEffect(() => {
     const subscription = signingCoordinator.signDataRequest$.pipe(take(1)).subscribe(async (r) => {
-      setDappInfo(await senderToDappInfo(r.signContext.sender));
+      try {
+        setDappInfo(await senderToDappInfo(r.signContext.sender));
+      } catch (error) {
+        logger.error(error);
+        void cancelTransaction('Could not get DApp info');
+        redirectToSignFailure();
+        return;
+      }
+
       setSignDataRequest(r);
     });
 
@@ -86,7 +97,7 @@ export const DappConfirmData = (): React.ReactElement => {
       subscription.unsubscribe();
       api.shutdown();
     };
-  }, [setSignDataRequest]);
+  }, [cancelTransaction, redirectToSignFailure, setSignDataRequest]);
 
   useEffect(() => {
     if (!req) return;

diff --git a/...-extension-wallet/src/features/dapp/components/confirm-transaction/ConfirmTransaction.tsx b/...-extension-wallet/src/features/dapp/components/confirm-transaction/ConfirmTransaction.tsx
@@ -19,6 +19,8 @@ import { runtime } from 'webextension-polyfill';
 import { Skeleton } from 'antd';
 import { DappTransactionContainer } from './DappTransactionContainer';
 import { useTxWitnessRequest } from '@providers/TxWitnessRequestProvider';
+import { useRedirection } from '@hooks';
+import { dAppRoutePaths } from '@routes';
 
 export const ConfirmTransaction = (): React.ReactElement => {
   const { t } = useTranslation();
@@ -28,6 +30,7 @@ export const ConfirmTransaction = (): React.ReactElement => {
     signTxRequest: { request: req, set: setSignTxRequest }
   } = useViewsFlowContext();
   const { walletType, isHardwareWallet, walletInfo, inMemoryWallet } = useWalletStore();
+  const redirectToDappTxSignFailure = useRedirection(dAppRoutePaths.dappTxSignFailure);
   const analytics = useAnalyticsContext();
   const [confirmTransactionError] = useState(false);
   const disallowSignTx = useDisallowSignTx(req);
@@ -49,11 +52,26 @@ export const ConfirmTransaction = (): React.ReactElement => {
 
   const txWitnessRequest = useTxWitnessRequest();
 
+  const cancelTransaction = useCallback(() => {
+    disallowSignTx(true);
+  }, [disallowSignTx]);
+
+  useOnUnload(cancelTransaction);
+
   useEffect(() => {
     (async () => {
-      if (!txWitnessRequest) return (): (() => void) => void 0;
+      const emptyFn = (): void => void 0;
+      if (!txWitnessRequest) return emptyFn;
+
+      try {
+        setDappInfo(await senderToDappInfo(txWitnessRequest.signContext.sender));
+      } catch (error) {
+        logger.error(error);
+        void disallowSignTx(true, 'Could not get DApp info');
+        redirectToDappTxSignFailure();
+        return emptyFn;
+      }
 
-      setDappInfo(await senderToDappInfo(txWitnessRequest.signContext.sender));
       setSignTxRequest(txWitnessRequest);
 
       const api = exposeApi<Pick<UserPromptService, 'readyToSignTx'>>(
@@ -73,7 +91,7 @@ export const ConfirmTransaction = (): React.ReactElement => {
         api.shutdown();
       };
     })();
-  }, [setSignTxRequest, setDappInfo, txWitnessRequest]);
+  }, [setSignTxRequest, setDappInfo, txWitnessRequest, redirectToDappTxSignFailure, disallowSignTx]);
 
   const onCancelTransaction = () => {
     analytics.sendEventToPostHog(PostHogAction.SendTransactionSummaryCancelClick, {
@@ -82,12 +100,6 @@ export const ConfirmTransaction = (): React.ReactElement => {
     disallowSignTx(true);
   };
 
-  const cancelTransaction = useCallback(() => {
-    disallowSignTx(true);
-  }, [disallowSignTx]);
-
-  useOnUnload(cancelTransaction);
-
   return (
     <Layout layoutClassname={cn(confirmTransactionError && styles.layoutError)} pageClassname={styles.spaceBetween}>
       {req && walletInfo && inMemoryWallet ? <DappTransactionContainer /> : <Skeleton loading />}

diff --git a/apps/browser-extension-wallet/src/features/dapp/components/confirm-transaction/hooks.ts b/apps/browser-extension-wallet/src/features/dapp/components/confirm-transaction/hooks.ts
@@ -151,7 +151,8 @@ export const useCreateMintedAssetList = ({
 
 export const useDisallowSignTx = (
   req: TransactionWitnessRequest<Wallet.WalletMetadata, Wallet.AccountMetadata>
-): ((close?: boolean) => Promise<void>) => useCallback(async (close) => await disallowSignTx(req, close), [req]);
+): ((close?: boolean, reason?: string) => Promise<void>) =>
+  useCallback(async (close, reason) => await disallowSignTx(req, close, reason), [req]);
 
 export const useAllowSignTx = (
   req: TransactionWitnessRequest<Wallet.WalletMetadata, Wallet.AccountMetadata>

diff --git a/apps/browser-extension-wallet/src/features/dapp/components/confirm-transaction/utils.ts b/apps/browser-extension-wallet/src/features/dapp/components/confirm-transaction/utils.ts
@@ -27,10 +27,11 @@ export const readyToSign = (): void => {
 
 export const disallowSignTx = async (
   req: TransactionWitnessRequest<Wallet.WalletMetadata, Wallet.AccountMetadata>,
-  close = false
+  close = false,
+  reason = 'User declined to sign'
 ): Promise<void> => {
   try {
-    await req?.reject('User declined to sign');
+    await req?.reject(reason);
   } finally {
     close && window.close();
   }

diff --git a/...ami-migration/migration-tool/cross-extension-messaging/lace-migration-client.extension.ts b/...ami-migration/migration-tool/cross-extension-messaging/lace-migration-client.extension.ts
@@ -7,6 +7,7 @@ import { NAMI_EXTENSION_ID } from './lace/environment';
 import { createLaceMigrationOpenListener } from './lace/create-lace-migration-open-listener';
 import { LACE_EXTENSION_ID } from './nami/environment';
 import { logger } from '@lace/common';
+import { brandPotentialExtApiError } from '@utils/brand-potential-ext-api-error';
 
 type CheckMigrationStatus = () => Promise<MigrationState>;
 
@@ -42,6 +43,14 @@ export const handleNamiRequests = (): void => {
   runtime.onMessageExternal.addListener(createLaceMigrationPingListener(NAMI_EXTENSION_ID));
   logger.debug('[NAMI MIGRATION] createLaceMigrationOpenListener');
   runtime.onMessageExternal.addListener(
-    createLaceMigrationOpenListener(NAMI_EXTENSION_ID, LACE_EXTENSION_ID, tabs.create)
+    createLaceMigrationOpenListener(NAMI_EXTENSION_ID, LACE_EXTENSION_ID, ({ url }) =>
+      brandPotentialExtApiError(
+        tabs.create({ url }),
+        `[NAMI MIGRATION] laceMigrationOpenListener failed to create tab with url ${url}`,
+        {
+          reThrow: false
+        }
+      )
+    )
   );
 };
diff --git a/...tion/migration-tool/cross-extension-messaging/lace/create-lace-migration-open-listener.ts b/...tion/migration-tool/cross-extension-messaging/lace/create-lace-migration-open-listener.ts
@@ -8,7 +8,11 @@ export const createLaceMigrationOpenListener =
     logger.debug('[NAMI MIGRATION] createLaceMigrationOpenListener', message, sender);
     if (message === NamiMessages.open && sender.id === namiExtensionId) {
       // First close all open lace tabs
-      await closeAllLaceOrNamiTabs();
+      try {
+        await closeAllLaceOrNamiTabs();
+      } catch (error) {
+        logger.error('[NAMI MIGRATION] createLaceMigrationOpenListener: failed to close all windows', error);
+      }
       createTab({ url: `chrome-extension://${laceExtensionId}/app.html` });
     }
   };
diff --git a/apps/browser-extension-wallet/src/lib/scripts/background/cip30.ts b/apps/browser-extension-wallet/src/lib/scripts/background/cip30.ts
@@ -47,7 +47,7 @@ export const confirmationCallback: walletCip30.CallbackConfirmation = {
         return cancelOnTabClose(tab);
       } catch (error) {
         logger.error(error);
-        return Promise.reject(new ApiError(APIErrorCode.InternalError, 'Unable to sign transaction'));
+        throw new ApiError(APIErrorCode.InternalError, 'Unable to sign transaction');
       }
     },
     DEBOUNCE_THROTTLE,
@@ -62,8 +62,7 @@ export const confirmationCallback: walletCip30.CallbackConfirmation = {
         return cancelOnTabClose(tab);
       } catch (error) {
         logger.error(error);
-        // eslint-disable-next-line unicorn/no-useless-undefined
-        return Promise.reject(new ApiError(APIErrorCode.InternalError, 'Unable to sign data'));
+        throw new ApiError(APIErrorCode.InternalError, 'Unable to sign data');
       }
     },
     DEBOUNCE_THROTTLE,

diff --git a/apps/browser-extension-wallet/src/lib/scripts/background/requestAccess.ts b/apps/browser-extension-wallet/src/lib/scripts/background/requestAccess.ts
@@ -10,15 +10,31 @@ import { AUTHORIZED_DAPPS_KEY } from '../types';
 import { Wallet } from '@lace/cardano';
 import { BehaviorSubject } from 'rxjs';
 import { senderToDappInfo } from '@src/utils/senderToDappInfo';
+import { logger } from '@lace/common';
 
 const DEBOUNCE_THROTTLE = 500;
 
 export const dappInfo$ = new BehaviorSubject<Wallet.DappInfo>(undefined);
 
 export const requestAccess: RequestAccess = async (sender: Runtime.MessageSender) => {
-  const { logo, name, url } = await senderToDappInfo(sender);
+  let dappInfo: Wallet.DappInfo;
+  try {
+    dappInfo = await senderToDappInfo(sender);
+  } catch (error) {
+    logger.error('Failed to get info of a DApp requesting access', error);
+    return false;
+  }
+
+  const { logo, name, url } = dappInfo;
   dappInfo$.next({ logo, name, url });
-  await ensureUiIsOpenAndLoaded('#/dapp/connect');
+
+  try {
+    await ensureUiIsOpenAndLoaded('#/dapp/connect');
+  } catch (error) {
+    logger.error('Failed to ensure DApp connection UI is loaded', error);
+    return false;
+  }
+
   const isAllowed = await userPromptService.allowOrigin(url);
   if (isAllowed === 'deny') return Promise.reject();
   if (isAllowed === 'allow') {
@@ -31,14 +47,16 @@ export const requestAccess: RequestAccess = async (sender: Runtime.MessageSender
       authorizedDappsList.next([{ logo, name, url }]);
     }
   } else {
-    tabs.onRemoved.addListener((t) => {
-      if (t === sender.tab.id) {
+    const onRemovedHandler = (tabId: number) => {
+      if (tabId === sender.tab.id) {
         authenticator.revokeAccess(sender);
-        tabs.onRemoved.removeListener(this);
+        tabs.onRemoved.removeListener(onRemovedHandler);
       }
-    });
+    };
+    tabs.onRemoved.addListener(onRemovedHandler);
   }
-  return Promise.resolve(true);
+
+  return true;
 };
 
 export const requestAccessDebounced = pDebounce(requestAccess, DEBOUNCE_THROTTLE, { before: true });
diff --git a/apps/browser-extension-wallet/src/lib/scripts/background/services/utilityServices.ts b/apps/browser-extension-wallet/src/lib/scripts/background/services/utilityServices.ts
@@ -29,6 +29,7 @@ import { laceFeaturesApiProperties, LACE_FEATURES_CHANNEL } from '../injectUtil'
 import { getErrorMessage } from '@src/utils/get-error-message';
 import { logger } from '@lace/common';
 import { POPUP_WINDOW_NAMI_TITLE } from '@utils/constants';
+import { brandPotentialExtApiError } from '@utils/brand-potential-ext-api-error';
 
 export const requestMessage$ = new Subject<Message>();
 export const backendFailures$ = new BehaviorSubject(0);
@@ -103,17 +104,24 @@ const handleOpenBrowser = async (data: OpenBrowserData) => {
       break;
   }
   const params = data.urlSearchParams ? `?${data.urlSearchParams}` : '';
-  await tabs.create({ url: `app.html#${path}${params}` }).catch((error) => logger.error(error));
+  const url = `app.html#${path}${params}`;
+  await brandPotentialExtApiError(tabs.create({ url }), `Failed to open expanded view with url: ${url}`).catch(
+    (error) => logger.error(error)
+  );
 };
 
 const handleOpenNamiBrowser = async (data: OpenNamiBrowserData) => {
-  await tabs.create({ url: `popup.html#${data.path}` }).catch((error) => logger.error(error));
+  const url = `popup.html#${data.path}`;
+  await brandPotentialExtApiError(tabs.create({ url }), `Failed to open nami mode extended with url: ${url}`).catch(
+    (error) => logger.error(error)
+  );
 };
 
 const enrichWithTabsDataIfMissing = (browserWindows: Windows.Window[]) => {
   const promises = browserWindows.map(async (w) => ({
     ...w,
-    tabs: w.tabs || (await tabs.query({ windowId: w.id }))
+    tabs:
+      w.tabs || (await brandPotentialExtApiError(tabs.query({ windowId: w.id }), 'Failed to query tabs of a window'))
   }));
   return Promise.all(promises);
 };
@@ -129,7 +137,8 @@ const doesWindowHaveOtherTabs = (browserWindow: WindowWithTabsNotOptional) =>
 
 const closeAllTabsAndOpenPopup = async () => {
   try {
-    const allWindows = await enrichWithTabsDataIfMissing(await windows.getAll());
+    const allWindowsRaw = await brandPotentialExtApiError(windows.getAll(), 'Failed to query all browser windows');
+    const allWindows = await enrichWithTabsDataIfMissing(allWindowsRaw);
     if (allWindows.length === 0) return;
 
     const windowsWith3rdPartyTabs = allWindows.filter((w) => doesWindowHaveOtherTabs(w));
@@ -141,14 +150,16 @@ const closeAllTabsAndOpenPopup = async () => {
     const noSingleWindowWith3rdPartyTabsOpen = !nextFocusedWindow;
     if (noSingleWindowWith3rdPartyTabsOpen) {
       nextFocusedWindow = allWindows[0];
-      await tabs.create({ active: true, windowId: nextFocusedWindow.id });
+      await brandPotentialExtApiError(
+        tabs.create({ active: true, windowId: nextFocusedWindow.id }),
+        'Failed to open empty tab to prevent window from closing'
+      );
     }
 
-    await windows.update(nextFocusedWindow.id, { focused: true });
+    await brandPotentialExtApiError(windows.update(nextFocusedWindow.id, { focused: true }), 'Failed to focus window');
     await closeAllLaceOrNamiTabs();
-    await action.openPopup();
+    await brandPotentialExtApiError(action.openPopup(), 'Failed to open popup');
   } catch (error) {
-    // unable to programatically open the popup again
     logger.error(error);
   }
 };

diff --git a/apps/browser-extension-wallet/src/lib/scripts/background/session/is-lace-tab-active.ts b/apps/browser-extension-wallet/src/lib/scripts/background/session/is-lace-tab-active.ts
@@ -1,6 +1,7 @@
 import { LACE_EXTENSION_ID } from '@src/features/nami-migration/migration-tool/cross-extension-messaging/nami/environment';
 import { distinctUntilChanged, from, fromEventPattern, map, merge, share, startWith, switchMap } from 'rxjs';
 import { Tabs, tabs, windows } from 'webextension-polyfill';
+import { brandPotentialExtApiError } from '@utils/brand-potential-ext-api-error';
 
 type WindowId = number;
 
@@ -21,10 +22,13 @@ const tabActivated$ = fromEventPattern<Tabs.OnActivatedActiveInfoType>(
 export const isLaceTabActive$ = merge(windowRemoved$, tabUpdated$, tabActivated$).pipe(
   switchMap(() =>
     from(
-      tabs.query({
-        active: true,
-        url: `chrome-extension://${LACE_EXTENSION_ID}/*`
-      })
+      brandPotentialExtApiError(
+        tabs.query({
+          active: true,
+          url: `chrome-extension://${LACE_EXTENSION_ID}/*`
+        }),
+        'Failed to query for currently active lace tab'
+      )
     )
   ),
   map((activeLaceTabs) => activeLaceTabs.length > 0),