From 8d659070ef35737be708d01cd9e40ce28dfff3d3 Mon Sep 17 00:00:00 2001 From: Kylan Hurt Date: Thu, 7 Nov 2024 16:22:16 -0500 Subject: [PATCH] fix: Add migration to fix NotificationServicesController bug (#12219) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The purpose of this PR is to fix an error apparently caused by a missing `NotificationServicesController` property on the `state.engine.backgroundState` ## **Related issues** Fixes: #12207 ## **Manual testing steps** Although we didn't pinpoint what was causing the `NotificationServicesController` to be missing, this code fills in the data if it is missing. ## **Screenshots/Recordings** n/a ### **Before** n/a ### **After** n/a ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: sethkfman <10342624+sethkfman@users.noreply.github.com> --- app/store/migrations/060.test.ts | 81 ++++++++++++++++++++++++++++++++ app/store/migrations/060.ts | 29 ++++++++++++ app/store/migrations/index.ts | 2 + app/util/sentry/tags/index.ts | 6 +++ 4 files changed, 118 insertions(+) create mode 100644 app/store/migrations/060.test.ts create mode 100644 app/store/migrations/060.ts diff --git a/app/store/migrations/060.test.ts b/app/store/migrations/060.test.ts new file mode 100644 index 00000000000..723eccce3fc --- /dev/null +++ b/app/store/migrations/060.test.ts @@ -0,0 +1,81 @@ +import migrate, { DEFAULT_NOTIFICATION_SERVICES_CONTROLLER } from './060'; +import { merge } from 'lodash'; +import { captureException } from '@sentry/react-native'; +import initialRootState, { backgroundState } from '../../util/test/initial-root-state'; +import mockedEngine from '../../core/__mocks__/MockedEngine'; + +jest.mock('@sentry/react-native', () => ({ + captureException: jest.fn(), +})); +const mockedCaptureException = jest.mocked(captureException); + +jest.mock('../../core/Engine', () => ({ + init: () => mockedEngine.init(), +})); + +describe('Migration #60 - Insert NotificationServicesController if missing', () => { + beforeEach(() => { + jest.restoreAllMocks(); + jest.resetAllMocks(); + }); + + const invalidStates = [ + { + state: null, + errorMessage: "FATAL ERROR: Migration 60: Invalid state error: 'object'", + scenario: 'state is invalid', + }, + { + state: merge({}, initialRootState, { + engine: null, + }), + errorMessage: + "FATAL ERROR: Migration 60: Invalid engine state error: 'object'", + scenario: 'engine state is invalid', + }, + { + state: merge({}, initialRootState, { + engine: { + backgroundState: null, + }, + }), + errorMessage: + "FATAL ERROR: Migration 60: Invalid engine backgroundState error: 'object'", + scenario: 'backgroundState is invalid', + }, + ]; + + for (const { errorMessage, scenario, state } of invalidStates) { + it(`should capture exception if ${scenario}`, async () => { + const newState = await migrate(state); + + expect(newState).toStrictEqual(state); + expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error)); + expect(mockedCaptureException.mock.calls[0][0].message).toBe( + errorMessage, + ); + }); + } + + it('should insert default NotificationServicesController if missing', async () => { + const oldState = { + ...initialRootState, + engine: { + backgroundState, + }, + }; + + const expectedState = { + ...initialRootState, + engine: { + backgroundState: { + ...backgroundState, + NotificationServicesController: DEFAULT_NOTIFICATION_SERVICES_CONTROLLER + }, + }, + }; + + const migratedState = await migrate(oldState); + expect(migratedState).toStrictEqual(expectedState); + }); +}); diff --git a/app/store/migrations/060.ts b/app/store/migrations/060.ts new file mode 100644 index 00000000000..b142cc04455 --- /dev/null +++ b/app/store/migrations/060.ts @@ -0,0 +1,29 @@ +import { ensureValidState } from './util'; +import { hasProperty, isObject } from '@metamask/utils'; + +export const DEFAULT_NOTIFICATION_SERVICES_CONTROLLER = { + isCheckingAccountsPresence: false, + isFeatureAnnouncementsEnabled: false, + isFetchingMetamaskNotifications: false, + isMetamaskNotificationsFeatureSeen: false, + isNotificationServicesEnabled: false, + isUpdatingMetamaskNotifications: false, + isUpdatingMetamaskNotificationsAccount: [], + metamaskNotificationsList: [], + metamaskNotificationsReadList: [], + subscriptionAccountsSeen: [], +}; + +export default function migrate(state: unknown) { + if (!ensureValidState(state, 60)) { + return state; + } + + if ( + !hasProperty(state.engine.backgroundState, 'NotificationServicesController') || + !isObject(state.engine.backgroundState.NotificationServicesController) + ) { + state.engine.backgroundState.NotificationServicesController = DEFAULT_NOTIFICATION_SERVICES_CONTROLLER; + } + return state; +} diff --git a/app/store/migrations/index.ts b/app/store/migrations/index.ts index 52ac3f83fc8..a1b929c96c7 100644 --- a/app/store/migrations/index.ts +++ b/app/store/migrations/index.ts @@ -60,6 +60,7 @@ import migration56 from './056'; import migration57 from './057'; import migration58 from './058'; import migration59 from './059'; +import migration60 from './060'; type MigrationFunction = (state: unknown) => unknown; type AsyncMigrationFunction = (state: unknown) => Promise; @@ -132,6 +133,7 @@ export const migrationList: MigrationsList = { 57: migration57, 58: migration58, 59: migration59, + 60: migration60 }; // Enable both synchronous and asynchronous migrations diff --git a/app/util/sentry/tags/index.ts b/app/util/sentry/tags/index.ts index 796bb9212fe..636663a71be 100644 --- a/app/util/sentry/tags/index.ts +++ b/app/util/sentry/tags/index.ts @@ -8,6 +8,12 @@ import { selectPendingApprovals } from '../../../selectors/approvalController'; export function getTraceTags(state: RootState) { if (!Object.keys(state?.engine?.backgroundState).length) return; + if (!state?.engine?.backgroundState?.AccountsController) return; + if (!state?.engine?.backgroundState?.NftController) return; + if (!state?.engine?.backgroundState?.NotificationServicesController) return; + if (!state?.engine?.backgroundState?.TokensController) return; + if (!state?.engine?.backgroundState?.TransactionController) return; + if (!state?.engine?.backgroundState?.NotificationServicesController) return; const unlocked = state.user.userLoggedIn; const accountCount = selectInternalAccounts(state).length; const nftCount = selectAllNftsFlat(state).length;