From 24d6cee82c36f9c2d1f21b52e25b3a757cbd88bc 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 The purpose of this PR is to fix an error apparently caused by a missing `NotificationServicesController` property on the `state.engine.backgroundState` Fixes: #12207 Although we didn't pinpoint what was causing the `NotificationServicesController` to be missing, this code fills in the data if it is missing. n/a n/a n/a - [ ] 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. - [ ] 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/058.test.ts | 34 ++++-------- app/store/migrations/058.ts | 41 +++++++------- app/store/migrations/059.test.ts | 93 ++++++++++++++++++++++++++++++++ app/store/migrations/059.ts | 24 +++++++++ app/store/migrations/index.ts | 2 + app/util/sentry/tags/index.ts | 6 +++ 6 files changed, 159 insertions(+), 41 deletions(-) create mode 100644 app/store/migrations/059.test.ts create mode 100644 app/store/migrations/059.ts diff --git a/app/store/migrations/058.test.ts b/app/store/migrations/058.test.ts index aaab8e61668..4ff8980d627 100644 --- a/app/store/migrations/058.test.ts +++ b/app/store/migrations/058.test.ts @@ -1,7 +1,7 @@ -import migrate from './058'; +import migrate, { DEFAULT_NOTIFICATION_SERVICES_CONTROLLER } from './058'; import { merge } from 'lodash'; import { captureException } from '@sentry/react-native'; -import initialRootState from '../../util/test/initial-root-state'; +import initialRootState, { backgroundState } from '../../util/test/initial-root-state'; import mockedEngine from '../../core/__mocks__/MockedEngine'; jest.mock('@sentry/react-native', () => ({ @@ -13,7 +13,7 @@ jest.mock('../../core/Engine', () => ({ init: () => mockedEngine.init(), })); -describe('Migration #58 - Update default search engine from DDG to Google', () => { +describe('Migration #58 - Insert NotificationServicesController if missing', () => { beforeEach(() => { jest.restoreAllMocks(); jest.resetAllMocks(); @@ -43,17 +43,6 @@ describe('Migration #58 - Update default search engine from DDG to Google', () = "FATAL ERROR: Migration 58: Invalid engine backgroundState error: 'object'", scenario: 'backgroundState is invalid', }, - { - state: merge({}, initialRootState, { - engine: { - backgroundState: {}, - }, - settings: null, - }), - errorMessage: - "FATAL ERROR: Migration 58: Invalid Settings state error: 'object'", - scenario: 'Settings object is invalid', - }, ]; for (const { errorMessage, scenario, state } of invalidStates) { @@ -68,23 +57,22 @@ describe('Migration #58 - Update default search engine from DDG to Google', () = }); } - it('should update the search engine to Google', async () => { + it('should insert default NotificationServicesController if missing', async () => { const oldState = { + ...initialRootState, engine: { - backgroundState: {}, + backgroundState, }, - settings: { - searchEngine: 'DuckDuckGo', - } }; const expectedState = { + ...initialRootState, engine: { - backgroundState: {}, + backgroundState: { + ...backgroundState, + NotificationServicesController: DEFAULT_NOTIFICATION_SERVICES_CONTROLLER + }, }, - settings: { - searchEngine: 'Google', - } }; const migratedState = await migrate(oldState); diff --git a/app/store/migrations/058.ts b/app/store/migrations/058.ts index b7708f0fcd6..289f0b89b1b 100644 --- a/app/store/migrations/058.ts +++ b/app/store/migrations/058.ts @@ -1,24 +1,29 @@ -import { captureException } from '@sentry/react-native'; -import { isObject } from '@metamask/utils'; 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, 58)) { - // Increment the migration number as appropriate - return state; - } + if (!ensureValidState(state, 58)) { + return state; + } - if (!isObject(state.settings)) { - captureException( - new Error( - `FATAL ERROR: Migration 58: Invalid Settings state error: '${typeof state.settings}'`, - ), - ); + if ( + !hasProperty(state.engine.backgroundState, 'NotificationServicesController') || + !isObject(state.engine.backgroundState.NotificationServicesController) + ) { + state.engine.backgroundState.NotificationServicesController = DEFAULT_NOTIFICATION_SERVICES_CONTROLLER; + } return state; - } - - state.settings.searchEngine = 'Google'; - - // Return the modified state - return state; } diff --git a/app/store/migrations/059.test.ts b/app/store/migrations/059.test.ts new file mode 100644 index 00000000000..de9bf2ab938 --- /dev/null +++ b/app/store/migrations/059.test.ts @@ -0,0 +1,93 @@ +import migrate from './059'; +import { merge } from 'lodash'; +import { captureException } from '@sentry/react-native'; +import initialRootState 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 #58 - Update default search engine from DDG to Google', () => { + beforeEach(() => { + jest.restoreAllMocks(); + jest.resetAllMocks(); + }); + + const invalidStates = [ + { + state: null, + errorMessage: "FATAL ERROR: Migration 58: Invalid state error: 'object'", + scenario: 'state is invalid', + }, + { + state: merge({}, initialRootState, { + engine: null, + }), + errorMessage: + "FATAL ERROR: Migration 58: Invalid engine state error: 'object'", + scenario: 'engine state is invalid', + }, + { + state: merge({}, initialRootState, { + engine: { + backgroundState: null, + }, + }), + errorMessage: + "FATAL ERROR: Migration 58: Invalid engine backgroundState error: 'object'", + scenario: 'backgroundState is invalid', + }, + { + state: merge({}, initialRootState, { + engine: { + backgroundState: {}, + }, + settings: null, + }), + errorMessage: + "FATAL ERROR: Migration 58: Invalid Settings state error: 'object'", + scenario: 'Settings object 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 update the search engine to Google', async () => { + const oldState = { + engine: { + backgroundState: {}, + }, + settings: { + searchEngine: 'DuckDuckGo', + } + }; + + const expectedState = { + engine: { + backgroundState: {}, + }, + settings: { + searchEngine: 'Google', + } + }; + + const migratedState = await migrate(oldState); + expect(migratedState).toStrictEqual(expectedState); + }); +}); diff --git a/app/store/migrations/059.ts b/app/store/migrations/059.ts new file mode 100644 index 00000000000..d4218b8fa82 --- /dev/null +++ b/app/store/migrations/059.ts @@ -0,0 +1,24 @@ +import { captureException } from '@sentry/react-native'; +import { isObject } from '@metamask/utils'; +import { ensureValidState } from './util'; + +export default function migrate(state: unknown) { + if (!ensureValidState(state, 59)) { + // Increment the migration number as appropriate + return state; + } + + if (!isObject(state.settings)) { + captureException( + new Error( + `FATAL ERROR: Migration 59: Invalid Settings state error: '${typeof state.settings}'`, + ), + ); + return state; + } + + state.settings.searchEngine = 'Google'; + + // Return the modified state + return state; +} diff --git a/app/store/migrations/index.ts b/app/store/migrations/index.ts index 91c8ba0a5d5..52ac3f83fc8 100644 --- a/app/store/migrations/index.ts +++ b/app/store/migrations/index.ts @@ -59,6 +59,7 @@ import migration55 from './055'; import migration56 from './056'; import migration57 from './057'; import migration58 from './058'; +import migration59 from './059'; type MigrationFunction = (state: unknown) => unknown; type AsyncMigrationFunction = (state: unknown) => Promise; @@ -130,6 +131,7 @@ export const migrationList: MigrationsList = { 56: migration56, 57: migration57, 58: migration58, + 59: migration59, }; // 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;