Merge pull request #4229 from EdgeApp/paul/errorHunting

Paul/error hunting
EdgeApp · May 30, 2023 · 974d4a2 · 974d4a2
2 parents 4f6dbcc + b0933a7
commit 974d4a2
Show file tree

Hide file tree

Showing 8 changed files with 144 additions and 105 deletions.
diff --git a/src/components/services/AccountCallbackManager.tsx b/src/components/services/AccountCallbackManager.tsx
@@ -134,24 +134,28 @@ export function AccountCallbackManager(props: Props) {
   })
 
   // Do the expensive work with rate limiting:
-  useAsyncEffect(async () => {
-    setDirty(notDirty)
-
-    // Update wallets:
-    if (dirty.walletList) {
-      // Update all wallets (hammer mode):
-      console.log('Updating wallet list')
-      await dispatch(updateWalletsRequest())
-      await snooze(1000)
-    }
-
-    // Update exchange rates:
-    if (dirty.rates) {
-      console.log('Updating exchange rates')
-      await dispatch(updateExchangeRates())
-      await snooze(1000)
-    }
-  }, [dirty])
+  useAsyncEffect(
+    async () => {
+      setDirty(notDirty)
+
+      // Update wallets:
+      if (dirty.walletList) {
+        // Update all wallets (hammer mode):
+        console.log('Updating wallet list')
+        await dispatch(updateWalletsRequest())
+        await snooze(1000)
+      }
+
+      // Update exchange rates:
+      if (dirty.rates) {
+        console.log('Updating exchange rates')
+        await dispatch(updateExchangeRates())
+        await snooze(1000)
+      }
+    },
+    [dirty],
+    'AccountCallbackManager'
+  )
 
   return null
 }

diff --git a/src/components/services/ActionQueueService.ts b/src/components/services/ActionQueueService.ts
@@ -65,16 +65,20 @@ export const ActionQueueService = () => {
   // Initialization
   //
 
-  useAsyncEffect(async () => {
-    if (account?.dataStore != null) {
-      const store = makeActionQueueStore(account, clientId)
-      const queue = await store.getActionQueueMap()
-      dispatch({
-        type: 'ACTION_QUEUE/LOAD_QUEUE',
-        data: queue
-      })
-    }
-  }, [account, dispatch])
+  useAsyncEffect(
+    async () => {
+      if (account?.dataStore != null) {
+        const store = makeActionQueueStore(account, clientId)
+        const queue = await store.getActionQueueMap()
+        dispatch({
+          type: 'ACTION_QUEUE/LOAD_QUEUE',
+          data: queue
+        })
+      }
+    },
+    [account, dispatch],
+    'ActionQueueService'
+  )
 
   //
   // Runtime Loop

diff --git a/src/components/services/AirshipInstance.tsx b/src/components/services/AirshipInstance.tsx
@@ -8,13 +8,21 @@ import { AirshipToast } from '../common/AirshipToast'
 import { AlertDropdown } from '../navigation/AlertDropdown'
 export const Airship = makeAirship()
 
+export interface ShowErrorWarningOptions {
+  // Report error to external bug tracking tool (ie. Bugsnag)
+  trackError?: boolean
+  tag?: string
+}
 /**
  * Shows an error to the user.
  * Used when some user-requested operation fails.
  */
-export function showError(error: unknown): void {
-  const translatedError = translateError(error)
-  Bugsnag.notify(`showError: ${translatedError}`)
+export function showError(error: unknown, options: ShowErrorWarningOptions = {}): void {
+  const { trackError = true, tag } = options
+  const translatedError = tag ? `Tag: ${tag}. ` + translateError(error) : translateError(error)
+  if (trackError) {
+    Bugsnag.notify(`showError: ${translatedError}`)
+  }
   console.log(redText('Showing error drop-down alert: ' + makeErrorLog(error)))
   Airship.show(bridge => <AlertDropdown bridge={bridge} message={translatedError} />)
 }
@@ -23,9 +31,12 @@ export function showError(error: unknown): void {
  * Shows a warning to the user.
  * Used when some user-requested operation succeeds but with a warning.
  */
-export function showWarning(error: unknown): void {
-  const translatedError = translateError(error)
-  Bugsnag.notify(`showWarning: ${translatedError}`)
+export function showWarning(error: unknown, options: ShowErrorWarningOptions = {}): void {
+  const { trackError = true, tag } = options
+  const translatedError = tag ? `Tag: ${tag}. ` + translateError(error) : translateError(error)
+  if (trackError) {
+    Bugsnag.notify(`showWarning: ${translatedError}`)
+  }
   console.log(yellowText('Showing warning drop-down alert: ' + makeErrorLog(error)))
   Airship.show(bridge => <AlertDropdown bridge={bridge} message={translatedError} warning />)
 }

diff --git a/src/components/services/LoanManagerService.ts b/src/components/services/LoanManagerService.ts
@@ -53,11 +53,15 @@ export const LoanManagerService = (props: Props) => {
   // Initialization
   //
 
-  useAsyncEffect(async () => {
-    if (account.disklet != null) {
-      dispatch(loadLoanAccounts(account))
-    }
-  }, [account, dispatch])
+  useAsyncEffect(
+    async () => {
+      if (account.disklet != null) {
+        dispatch(loadLoanAccounts(account))
+      }
+    },
+    [account, dispatch],
+    'LoanManagerService 1'
+  )
 
   //
   // Cleanup Routine
@@ -95,50 +99,54 @@ export const LoanManagerService = (props: Props) => {
 
   // Cache changes to specific watched properties of the loan accounts to detect
   // deltas
-  useAsyncEffect(async () => {
-    for (const loanAccountId of Object.keys(loanAccountMap)) {
-      await waitForBorrowEngineSync(loanAccountMap[loanAccountId].borrowEngine)
-      const { borrowEngine } = loanAccountMap[loanAccountId]
-      const { debts, collaterals } = borrowEngine
-
-      // Cache accounts only if we can support liquidation price calculations
-      const filteredDebts = debts.filter(debt => !zeroString(debt.nativeAmount))
-      const filteredCollaterals = collaterals.filter(collateral => !zeroString(collateral.nativeAmount))
-      const onlyOneCollateral = filteredCollaterals.length === 1
-      // TODO: Find a less crude way to determine if a token is USD-based
-      const onlyUsdBasedDebts = filteredDebts.every(debt => (debt.tokenId != null ? USD_BASED_TOKEN_IDS.includes(debt.tokenId.toLowerCase()) : false))
-
-      if (onlyOneCollateral && onlyUsdBasedDebts) {
-        // Only trigger push events after the exchange rates are available for
-        // all loan assets on this account
-        const loanAssetTokenIds = [...filteredDebts, ...filteredCollaterals].map(loanAsset => loanAsset.tokenId)
-        if (loanAssetTokenIds.every(tokenId => tokenId == null || !zeroString(exchangeRates[`${getCurrencyCode(borrowEngine, tokenId)}_iso:USD`]))) {
-          if (debts.length > 0) {
-            // If it's the first time the account has been seen, we want to
-            // avoid a notification if it already exceeds the liquidation price
-            // right when the app is booted. Newly created loans can never
-            // exceed the liquidation price.
-            if (cachedLoanAssetsMap[loanAccountId] == null) {
-              cachedLoanAssetsMap[loanAccountId] = JSON.stringify([...debts, ...collaterals])
+  useAsyncEffect(
+    async () => {
+      for (const loanAccountId of Object.keys(loanAccountMap)) {
+        await waitForBorrowEngineSync(loanAccountMap[loanAccountId].borrowEngine)
+        const { borrowEngine } = loanAccountMap[loanAccountId]
+        const { debts, collaterals } = borrowEngine
+
+        // Cache accounts only if we can support liquidation price calculations
+        const filteredDebts = debts.filter(debt => !zeroString(debt.nativeAmount))
+        const filteredCollaterals = collaterals.filter(collateral => !zeroString(collateral.nativeAmount))
+        const onlyOneCollateral = filteredCollaterals.length === 1
+        // TODO: Find a less crude way to determine if a token is USD-based
+        const onlyUsdBasedDebts = filteredDebts.every(debt => (debt.tokenId != null ? USD_BASED_TOKEN_IDS.includes(debt.tokenId.toLowerCase()) : false))
+
+        if (onlyOneCollateral && onlyUsdBasedDebts) {
+          // Only trigger push events after the exchange rates are available for
+          // all loan assets on this account
+          const loanAssetTokenIds = [...filteredDebts, ...filteredCollaterals].map(loanAsset => loanAsset.tokenId)
+          if (loanAssetTokenIds.every(tokenId => tokenId == null || !zeroString(exchangeRates[`${getCurrencyCode(borrowEngine, tokenId)}_iso:USD`]))) {
+            if (debts.length > 0) {
+              // If it's the first time the account has been seen, we want to
+              // avoid a notification if it already exceeds the liquidation price
+              // right when the app is booted. Newly created loans can never
+              // exceed the liquidation price.
+              if (cachedLoanAssetsMap[loanAccountId] == null) {
+                cachedLoanAssetsMap[loanAccountId] = JSON.stringify([...debts, ...collaterals])
+                await uploadLiquidationPushEvents(loanAccountId, false)
+              }
+              // If we already have a cache of this account and the LTV changed,
+              // upload a push notification even if the liquidation price has been
+              // exceeded
+              else if (JSON.stringify([...debts, ...collaterals]) !== cachedLoanAssetsMap[loanAccountId]) {
+                cachedLoanAssetsMap[loanAccountId] = JSON.stringify([...debts, ...collaterals])
+                await uploadLiquidationPushEvents(loanAccountId, true)
+              }
+            } else {
+              // Ensure push event is cleared if account is closed or debts no
+              // longer exist
               await uploadLiquidationPushEvents(loanAccountId, false)
             }
-            // If we already have a cache of this account and the LTV changed,
-            // upload a push notification even if the liquidation price has been
-            // exceeded
-            else if (JSON.stringify([...debts, ...collaterals]) !== cachedLoanAssetsMap[loanAccountId]) {
-              cachedLoanAssetsMap[loanAccountId] = JSON.stringify([...debts, ...collaterals])
-              await uploadLiquidationPushEvents(loanAccountId, true)
-            }
-          } else {
-            // Ensure push event is cleared if account is closed or debts no
-            // longer exist
-            await uploadLiquidationPushEvents(loanAccountId, false)
           }
         }
+        setCachedLoanAssetsMap({ ...cachedLoanAssetsMap })
       }
-      setCachedLoanAssetsMap({ ...cachedLoanAssetsMap })
-    }
-  }, [loanAccountMap, exchangeRates])
+    },
+    [loanAccountMap, exchangeRates],
+    'LoanManagerService 2'
+  )
 
   const uploadLiquidationPushEvents = React.useCallback(
     async (loanAccountId: string, isSkipPriceCheck: boolean) => {

diff --git a/src/components/services/NetworkActivity.ts b/src/components/services/NetworkActivity.ts
@@ -17,7 +17,7 @@ class NetworkActivityComponent extends React.Component<Props> {
     console.log('NetworkActivity - isConnected changed: ', info.isConnected)
     this.props.changeConnectivity(info.isConnected ?? false)
     if (!info.isConnected) {
-      showError(`${lstrings.network_alert_title}`)
+      showError(`${lstrings.network_alert_title}`, { trackError: false })
     }
   }
 

diff --git a/src/components/services/PermissionsManager.tsx b/src/components/services/PermissionsManager.tsx
@@ -20,10 +20,14 @@ export const PermissionsManager = () => {
   const statePermissions = useSelector(state => state.permissions)
   const isAppForeground = useIsAppForeground()
 
-  useAsyncEffect(async () => {
-    if (!isAppForeground) return
-    await dispatch(setNewPermissions(statePermissions))
-  }, [isAppForeground, statePermissions])
+  useAsyncEffect(
+    async () => {
+      if (!isAppForeground) return
+      await dispatch(setNewPermissions(statePermissions))
+    },
+    [isAppForeground, statePermissions],
+    'PermissionsManager'
+  )
 
   return null
 }

diff --git a/src/components/services/Services.tsx b/src/components/services/Services.tsx
@@ -76,27 +76,35 @@ export function Services(props: Props) {
   })
 
   // Methods to call immediately after login:
-  useAsyncEffect(async () => {
-    if (account != null) {
-      maybeShowFioHandleModal(account)
-    }
-  }, [account, maybeShowFioHandleModal])
+  useAsyncEffect(
+    async () => {
+      if (account != null) {
+        maybeShowFioHandleModal(account)
+      }
+    },
+    [account, maybeShowFioHandleModal],
+    'Services 1'
+  )
 
   // Methods to call once all of the currency wallets have been loaded
-  useAsyncEffect(async () => {
-    if (account?.waitForAllWallets == null) return
-    await account.waitForAllWallets()
+  useAsyncEffect(
+    async () => {
+      if (account?.waitForAllWallets == null) return
+      await account.waitForAllWallets()
 
-    dispatch(registerNotificationsV2()).catch(e => {
-      console.warn('registerNotificationsV2 error:', e)
-    })
+      dispatch(registerNotificationsV2()).catch(e => {
+        console.warn('registerNotificationsV2 error:', e)
+      })
 
-    // HACK: The balances object isn't full when the above promise resolves so we need to wait a few seconds before proceeding
-    await snooze(5000)
-    dispatch(checkCompromisedKeys(navigation)).catch(e => {
-      console.warn('checkCompromisedKeys error:', e)
-    })
-  }, [account])
+      // HACK: The balances object isn't full when the above promise resolves so we need to wait a few seconds before proceeding
+      await snooze(5000)
+      dispatch(checkCompromisedKeys(navigation)).catch(e => {
+        console.warn('checkCompromisedKeys error:', e)
+      })
+    },
+    [account],
+    'Services 2'
+  )
 
   // Methods to call periodically
   useRefresher(

diff --git a/src/hooks/useAsyncEffect.ts b/src/hooks/useAsyncEffect.ts
@@ -18,7 +18,7 @@ interface State {
  * Runs an effect when its dependencies change, just like `useEffect`,
  * but awaits the returned promise before starting the next run.
  */
-export function useAsyncEffect(effect: AsyncEffect, deps?: unknown[]): void {
+export function useAsyncEffect(effect: AsyncEffect, deps?: unknown[], tag?: string): void {
   const state = React.useRef<State>({
     closed: false,
     dirty: false,
@@ -32,9 +32,9 @@ export function useAsyncEffect(effect: AsyncEffect, deps?: unknown[]): void {
   React.useEffect(
     () => () => {
       state.current.closed = true
-      wakeup(state.current)
+      wakeup(state.current, tag)
     },
-    []
+    [tag]
   )
 
   // Check for differences:
@@ -59,7 +59,7 @@ function matchDeps(a?: unknown[], b?: unknown[]): boolean {
 /**
  * Does the next thing based on the current state.
  */
-function wakeup(state: State): void {
+function wakeup(state: State, tag?: string): void {
   // We can't do anything if the effect is already running:
   if (state.running) return
 
@@ -82,7 +82,7 @@ function wakeup(state: State): void {
         wakeup(state)
       })
       .catch(error => {
-        showError(error)
+        showError(error, { tag })
         state.running = false
         wakeup(state)
       })