From 55ee0b30c0607da774808c660423a6896cfbde45 Mon Sep 17 00:00:00 2001 From: Alec WM Date: Thu, 29 Aug 2024 08:38:00 +1000 Subject: [PATCH] fix: improved general report reliability (#1590) --- .../src/domains/app/store.app.ts | 4 +- .../extension-core/src/libs/GeneralReport.ts | 142 ++++++++++++------ 2 files changed, 100 insertions(+), 46 deletions(-) diff --git a/packages/extension-core/src/domains/app/store.app.ts b/packages/extension-core/src/domains/app/store.app.ts index 85859103c..637ceb76d 100644 --- a/packages/extension-core/src/domains/app/store.app.ts +++ b/packages/extension-core/src/domains/app/store.app.ts @@ -1,6 +1,7 @@ import { DEBUG, IS_FIREFOX } from "extension-shared" import { gt } from "semver" +import { GeneralReport } from "../../libs/GeneralReport" import { migratePasswordV2ToV1 } from "../../libs/migrations/legacyMigrations" import { StorageProvider } from "../../libs/Store" import { StakingSupportedChain } from "../staking/types" @@ -20,7 +21,8 @@ export type AppStoreData = { hideBraveWarning: boolean hasBraveWarningBeenShown: boolean analyticsRequestShown: boolean - analyticsReportSent?: number + analyticsReportCreatedAt?: number + analyticsReport?: GeneralReport hideBackupWarningUntil?: number hasSpiritKey: boolean hideStakingBanner: StakingSupportedChain[] diff --git a/packages/extension-core/src/libs/GeneralReport.ts b/packages/extension-core/src/libs/GeneralReport.ts index c04d21e1c..39dd814be 100644 --- a/packages/extension-core/src/libs/GeneralReport.ts +++ b/packages/extension-core/src/libs/GeneralReport.ts @@ -1,4 +1,5 @@ import keyring from "@polkadot/ui-keyring" +import { sleep } from "@talismn/util" import { DEBUG } from "extension-shared" import groupBy from "lodash/groupBy" import { type Properties } from "posthog-js" @@ -9,7 +10,7 @@ import { appStore } from "../domains/app/store.app" import { settingsStore } from "../domains/app/store.settings" import { balancePool } from "../domains/balances/pool" import { Balances } from "../domains/balances/types" -import { getNftCollectionFloorUsd } from "../domains/nfts" +import { getNftCollectionFloorUsd, subscribeNfts } from "../domains/nfts" import { nftsStore$ } from "../domains/nfts/store" import { chaindataProvider } from "../rpcs/chaindata" import { hasGhostsOfThePast } from "../util/hasGhostsOfThePast" @@ -17,34 +18,65 @@ import { privacyRoundCurrency } from "../util/privacyRoundCurrency" const REPORTING_PERIOD = 24 * 3600 * 1000 // 24 hours +export type GeneralReport = Awaited> + +/** + * This global variable makes sure that we only build one report at a time. + * If the background script is restarted, this flag will be reset to `false`. + */ +let isBuildingReport = false + // // This should get sent at most once per 24 hours, whenever any other events get sent // export async function withGeneralReport(properties?: Properties) { - const analyticsReportSent = await appStore.get("analyticsReportSent") - - // on first run, note down the timestamp but don't make a report - if (typeof analyticsReportSent !== "number") { - await appStore.set({ analyticsReportSent: Date.now() }) - return properties + // If a report has been created but not yet submitted, + // this function will attach it to the pending event's properties + const includeExistingReportInProperties = async () => { + const analyticsReport = await appStore.get("analyticsReport") + if (!analyticsReport) return + + await appStore.set({ analyticsReport: undefined }) + properties = { ...properties, $set: { ...(properties?.$set ?? {}), ...analyticsReport } } } - // on subsequent runs, do nothing if the diff between the recorded timestamp and now is less than REPORTING_PERIOD - if (Date.now() - analyticsReportSent < REPORTING_PERIOD) return properties - - // and when the diff is greater, make a report - try { - const generalReport = await getGeneralReport() + // If we've not created a report before, or if it has been REPORTING_PERIOD ms since we last created a report, + // this function will spawn an async task to create a new report in the background. + const spawnTaskToCreateNewReport = async () => { + const analyticsReportCreatedAt = await appStore.get("analyticsReportCreatedAt") + + // if the wallet has already created a report, do nothing when the time since the last report is less than REPORTING_PERIOD + const hasCreatedReport = typeof analyticsReportCreatedAt === "number" + const timeSinceReportCreated = hasCreatedReport ? Date.now() - analyticsReportCreatedAt : 0 + if (hasCreatedReport && timeSinceReportCreated < REPORTING_PERIOD) return + + // if we're already creating a report (in response to an event which happened before this one) + // then don't try to build another one at the same time + if (isBuildingReport) return + isBuildingReport = true + + // spawn async task (don't wait for it to complete before continuing) + ;(async () => { + try { + const analyticsReport = await getGeneralReport() + + // don't include general report if user is onboarding/has analytics turned off/other short-circuit conditions + if (analyticsReport === undefined) return + + await appStore.set({ analyticsReportCreatedAt: Date.now(), analyticsReport }) + } catch (cause) { + console.warn("Failed to build general report", { cause }) // eslint-disable-line no-console + } finally { + // set this flag back to false so we don't block the next report + isBuildingReport = false + } + })() + } - // don't include general report if user is onboarding/has analytics turned off/other short-circuit conditions - if (generalReport === undefined) return properties + await includeExistingReportInProperties() + await spawnTaskToCreateNewReport() - await appStore.set({ analyticsReportSent: Date.now() }) - return { ...properties, $set: { ...(properties?.$set ?? {}), ...generalReport } } - } catch (cause) { - console.warn("Failed to build general report", { cause }) // eslint-disable-line no-console - return properties - } + return properties } async function getGeneralReport() { @@ -68,30 +100,47 @@ async function getGeneralReport() { const watchedAccounts = accounts.filter(({ meta }) => meta.origin === AccountType.Watched) const watchedAccountsCount = watchedAccounts.length - const poolStatus = await Promise.race([ - balancePool.status, - new Promise<"initialising">((resolve) => setTimeout(() => resolve("initialising"), 2_000)), - ]) - // NOTE: There are two not-ready states of the balances pool, which are both called `initialising`. - // 1. The pool is still getting chainata ready so that it can set up balances subscriptions. - // 2. The pool is ready and has subscriptions, but some balances are still being refreshed from the chains. - // - // if balancePool isn't initialised (chaindata loaded from db), balances stats may be incorrect (initialising state 1) - if (balancePool.hasInitialised.isPending()) return - // if balancePool isn't initialised (all balance queries set to `live`), balances stats may be incorrect (initialising state 2) - if (poolStatus === "initialising") return - - // if the balancePool has cached balances, we probably have an incomplete state of the user's balances - // (i.e. we're still fetching them after a browser restart) - // - // 30 seconds after the pool is initialised, all of the cached balances are transformed into stale balances - // - // in the future, we should expose an observable on the pool which can indicate whether or not the pool is hydrated, - // until then, this is a good-enough workaround - const balanceJsons = Object.values(balancePool.balances).filter((balance) => - ownedAddresses.includes(balance.address) - ) - if (balanceJsons.some((b) => b.status === "cache")) return + let disconnect!: () => void + try { + // create token balances / nft subscriptions, and wait for the pool to settle + // this ensures that we have up-to-date information for the report + const onDisconnected = new Promise((resolve) => { + let hasDisconnected = false + disconnect = () => { + if (hasDisconnected) return + hasDisconnected = true + resolve() + } + }) + + let balancesLive = false + let nftsLive = false + + // token balances + const subscriptionId = "ANALYTICS-GENERAL-REPORT" + balancePool.subscribe(subscriptionId, onDisconnected, (response) => { + if (response.status !== "live") return + balancesLive = true + if (!nftsLive) return + disconnect() + }) + // nfts + const unsubNfts = subscribeNfts((data) => { + if (data.status !== "loaded") return + nftsLive = true + if (!balancesLive) return + disconnect() + }) + onDisconnected.then(() => unsubNfts()) + // timeout (don't wait forever for all token balances and nfts to be live) + await sleep(30_000).then(disconnect) + + // wait for live token balances & nfts, or timeout to complete + await onDisconnected + } finally { + // if anything throws, make sure we shut down all the subscriptions we opened + disconnect() + } // account type breakdown const accountBreakdown: Record, number> = { @@ -126,6 +175,9 @@ async function getGeneralReport() { ), ]) + const balanceJsons = Object.values(balancePool.balances).filter((balance) => + ownedAddresses.includes(balance.address) + ) /* eslint-disable-next-line no-var */ var balances = new Balances(balanceJsons, { chains, evmNetworks, tokens, tokenRates }) } catch (cause) {