Merge branch 'main' into extract-create-service-policy-1

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@metamask/core-monorepo",
-  "version": "281.0.0",
+  "version": "282.0.0",
   "private": true,
   "description": "Monorepo for packages shared between MetaMask clients",
   "repository": {

packages/notification-services-controller/package.json

@@ -113,7 +113,7 @@
     "@lavamoat/preinstall-always-fail": "^2.1.0",
     "@metamask/auto-changelog": "^3.4.4",
     "@metamask/keyring-controller": "^19.0.3",
-    "@metamask/profile-sync-controller": "^4.0.0",
+    "@metamask/profile-sync-controller": "^4.0.1",
     "@types/jest": "^27.4.1",
     "@types/readable-stream": "^2.3.0",
     "contentful": "^10.15.0",

packages/profile-sync-controller/CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [4.0.1]
+
+### Added
+
+- Add optional sentry context parameter to erroneous situation callbacks ([#5139](https://github.com/MetaMask/core/pull/5139))
+
 ## [4.0.0]
 
 ### Changed
@@ -410,7 +416,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Initial release
 
-[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/[email protected]
+[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/[email protected]
+[4.0.1]: https://github.com/MetaMask/core/compare/@metamask/[email protected]...@metamask/[email protected]
 [4.0.0]: https://github.com/MetaMask/core/compare/@metamask/[email protected]...@metamask/[email protected]
 [3.3.0]: https://github.com/MetaMask/core/compare/@metamask/[email protected]...@metamask/[email protected]
 [3.2.0]: https://github.com/MetaMask/core/compare/@metamask/[email protected]...@metamask/[email protected]

packages/profile-sync-controller/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@metamask/profile-sync-controller",
-  "version": "4.0.0",
+  "version": "4.0.1",
   "description": "The profile sync helps developers synchronize data across multiple clients and devices in a privacy-preserving way. All data saved in the user storage database is encrypted client-side to preserve privacy. The user storage provides a modular design, giving developers the flexibility to construct and manage their storage spaces in a way that best suits their needs",
   "keywords": [
     "MetaMask",

packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts

@@ -173,9 +173,8 @@ describe('user-storage/user-storage-controller - performGetStorageAllFeatureEntr
       getMetaMetricsState: () => true,
     });
 
-    const result = await controller.performGetStorageAllFeatureEntries(
-      'notifications',
-    );
+    const result =
+      await controller.performGetStorageAllFeatureEntries('notifications');
     mockAPI.done();
     expect(result).toStrictEqual([MOCK_STORAGE_DATA]);
   });
@@ -893,7 +892,7 @@ describe('user-storage/user-storage-controller - syncInternalAccountsWithUserSto
       ) => {
         onAccountAdded?.();
         onAccountNameUpdated?.();
-        onAccountSyncErroneousSituation?.('error message');
+        onAccountSyncErroneousSituation?.('error message', {});
         getMessenger();
         getUserStorageControllerInstance();
         return undefined;

packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts

@@ -161,6 +161,7 @@ type ControllerConfig = {
     onAccountSyncErroneousSituation?: (
       profileId: string,
       situationMessage: string,
+      sentryContext?: Record<string, unknown>,
     ) => void;
   };
 
@@ -864,10 +865,11 @@ export default class UserStorageController extends BaseController<
           this.#config?.accountSyncing?.onAccountAdded?.(profileId),
         onAccountNameUpdated: () =>
           this.#config?.accountSyncing?.onAccountNameUpdated?.(profileId),
-        onAccountSyncErroneousSituation: (situationMessage) =>
+        onAccountSyncErroneousSituation: (situationMessage, sentryContext) =>
           this.#config?.accountSyncing?.onAccountSyncErroneousSituation?.(
             profileId,
             situationMessage,
+            sentryContext,
           ),
       },
       {

packages/profile-sync-controller/src/controllers/user-storage/account-syncing/controller-integration.test.ts

@@ -298,28 +298,32 @@ describe('user-storage/account-syncing/controller-integration - syncInternalAcco
 
   describe('handles corrupted user storage gracefully', () => {
     const arrangeMocksForBogusAccounts = async () => {
+      const accountsList =
+        MOCK_INTERNAL_ACCOUNTS.ONE_DEFAULT_NAME as InternalAccount[];
       const { messengerMocks, config, options } = await arrangeMocks({
         messengerMockOptions: {
           accounts: {
-            accountsList:
-              MOCK_INTERNAL_ACCOUNTS.ONE_DEFAULT_NAME as InternalAccount[],
+            accountsList,
           },
         },
       });
 
+      const userStorageList =
+        MOCK_USER_STORAGE_ACCOUNTS.TWO_DEFAULT_NAMES_WITH_ONE_BOGUS;
+
       return {
         config,
         options,
         messengerMocks,
+        accountsList,
+        userStorageList,
         mockAPI: {
           mockEndpointGetUserStorage:
             await mockEndpointGetUserStorageAllFeatureEntries(
               USER_STORAGE_FEATURE_NAMES.accounts,
               {
                 status: 200,
-                body: await createMockUserStorageEntries(
-                  MOCK_USER_STORAGE_ACCOUNTS.TWO_DEFAULT_NAMES_WITH_ONE_BOGUS,
-                ),
+                body: await createMockUserStorageEntries(userStorageList),
               },
             ),
           mockEndpointBatchDeleteUserStorage:
@@ -363,20 +367,86 @@ describe('user-storage/account-syncing/controller-integration - syncInternalAcco
       expect(mockAPI.mockEndpointBatchDeleteUserStorage.isDone()).toBe(true);
     });
 
-    it('fires the onAccountSyncErroneousSituation callback in erroneous situations', async () => {
-      const onAccountSyncErroneousSituation = jest.fn();
+    describe('Fires the onAccountSyncErroneousSituation callback on erroneous situations', () => {
+      it('And logs if the final state is incorrect', async () => {
+        const onAccountSyncErroneousSituation = jest.fn();
 
-      const { config, options } = await arrangeMocksForBogusAccounts();
+        const { config, options, userStorageList, accountsList } =
+          await arrangeMocksForBogusAccounts();
 
-      await AccountSyncingControllerIntegrationModule.syncInternalAccountsWithUserStorage(
-        {
-          ...config,
-          onAccountSyncErroneousSituation,
-        },
-        options,
-      );
+        await AccountSyncingControllerIntegrationModule.syncInternalAccountsWithUserStorage(
+          {
+            ...config,
+            onAccountSyncErroneousSituation,
+          },
+          options,
+        );
 
-      expect(onAccountSyncErroneousSituation).toHaveBeenCalledTimes(1);
+        expect(onAccountSyncErroneousSituation).toHaveBeenCalledTimes(2);
+        expect(onAccountSyncErroneousSituation.mock.calls).toEqual([
+          [
+            'An account was present in the user storage accounts list but was not found in the internal accounts list after the sync',
+            {
+              internalAccountsList: accountsList,
+              internalAccountsToBeSavedToUserStorage: [],
+              refreshedInternalAccountsList: accountsList,
+              userStorageAccountsList: userStorageList,
+              userStorageAccountsToBeDeleted: [userStorageList[1]],
+            },
+          ],
+          [
+            'Erroneous situations were found during the sync, and final state does not match the expected state',
+            {
+              finalInternalAccountsList: accountsList,
+              finalUserStorageAccountsList: null,
+            },
+          ],
+        ]);
+      });
+
+      it('And logs if the final state is correct', async () => {
+        const onAccountSyncErroneousSituation = jest.fn();
+
+        const { config, options, userStorageList, accountsList } =
+          await arrangeMocksForBogusAccounts();
+
+        await mockEndpointGetUserStorageAllFeatureEntries(
+          USER_STORAGE_FEATURE_NAMES.accounts,
+          {
+            status: 200,
+            body: await createMockUserStorageEntries([userStorageList[0]]),
+          },
+        );
+
+        await AccountSyncingControllerIntegrationModule.syncInternalAccountsWithUserStorage(
+          {
+            ...config,
+            onAccountSyncErroneousSituation,
+          },
+          options,
+        );
+
+        expect(onAccountSyncErroneousSituation).toHaveBeenCalledTimes(2);
+        expect(onAccountSyncErroneousSituation.mock.calls).toEqual([
+          [
+            'An account was present in the user storage accounts list but was not found in the internal accounts list after the sync',
+            {
+              internalAccountsList: accountsList,
+              internalAccountsToBeSavedToUserStorage: [],
+              refreshedInternalAccountsList: accountsList,
+              userStorageAccountsList: userStorageList,
+              userStorageAccountsToBeDeleted: [userStorageList[1]],
+            },
+          ],
+          [
+            'Erroneous situations were found during the sync, but final state matches the expected state',
+            {
+              finalInternalAccountsList: accountsList,
+              finalUserStorageAccountsList: [userStorageList[0]],
+            },
+          ],
+        ]);
+      });
     });
   });
 

packages/profile-sync-controller/src/controllers/user-storage/account-syncing/controller-integration.ts

@@ -96,7 +96,10 @@ type SyncInternalAccountsWithUserStorageConfig = AccountSyncingConfig & {
   maxNumberOfAccountsToAdd?: number;
   onAccountAdded?: () => void;
   onAccountNameUpdated?: () => void;
-  onAccountSyncErroneousSituation?: (errorMessage: string) => void;
+  onAccountSyncErroneousSituation?: (
+    errorMessage: string,
+    sentryContext?: Record<string, unknown>,
+  ) => void;
 };
 
 /**
@@ -141,6 +144,9 @@ export async function syncInternalAccountsWithUserStorage(
       );
       return;
     }
+    // Keep a record if erroneous situations are found during the sync
+    // This is done so we can send the context to Sentry in case of an erroneous situation
+    let erroneousSituationsFound = false;
 
     // Prepare an array of internal accounts to be saved to the user storage
     const internalAccountsToBeSavedToUserStorage: InternalAccount[] = [];
@@ -191,8 +197,18 @@ export async function syncInternalAccountsWithUserStorage(
       if (!userStorageAccount) {
         // If the account was just added in the previous step, skip saving it, it's likely to be a bogus account
         if (newlyAddedAccounts.includes(internalAccount)) {
+          erroneousSituationsFound = true;
           onAccountSyncErroneousSituation?.(
             'An account was added to the internal accounts list but was not present in the user storage accounts list',
+            {
+              internalAccount,
+              userStorageAccount,
+              newlyAddedAccounts,
+              userStorageAccountsList,
+              internalAccountsList,
+              refreshedInternalAccountsList,
+              internalAccountsToBeSavedToUserStorage,
+            },
           );
           continue;
         }
@@ -295,11 +311,64 @@ export async function syncInternalAccountsWithUserStorage(
         USER_STORAGE_FEATURE_NAMES.accounts,
         userStorageAccountsToBeDeleted.map((account) => account.a),
       );
+      erroneousSituationsFound = true;
       onAccountSyncErroneousSituation?.(
         'An account was present in the user storage accounts list but was not found in the internal accounts list after the sync',
+        {
+          userStorageAccountsToBeDeleted,
+          internalAccountsList,
+          refreshedInternalAccountsList,
+          internalAccountsToBeSavedToUserStorage,
+          userStorageAccountsList,
+        },
       );
     }
 
+    if (erroneousSituationsFound) {
+      const [finalUserStorageAccountsList, finalInternalAccountsList] =
+        await Promise.all([
+          getUserStorageAccountsList(options),
+          getInternalAccountsList(options),
+        ]);
+
+      const doesEveryAccountInInternalAccountsListExistInUserStorageAccountsList =
+        finalInternalAccountsList.every((account) =>
+          finalUserStorageAccountsList?.some(
+            (userStorageAccount) => userStorageAccount.a === account.address,
+          ),
+        );
+
+      // istanbul ignore next
+      const doesEveryAccountInUserStorageAccountsListExistInInternalAccountsList =
+        (finalUserStorageAccountsList?.length || 0) > maxNumberOfAccountsToAdd
+          ? true
+          : finalUserStorageAccountsList?.every((account) =>
+              finalInternalAccountsList.some(
+                (internalAccount) => internalAccount.address === account.a,
+              ),
+            );
+
+      const doFinalListsMatch =
+        doesEveryAccountInInternalAccountsListExistInUserStorageAccountsList &&
+        doesEveryAccountInUserStorageAccountsListExistInInternalAccountsList;
+
+      const context = {
+        finalUserStorageAccountsList,
+        finalInternalAccountsList,
+      };
+      if (doFinalListsMatch) {
+        onAccountSyncErroneousSituation?.(
+          'Erroneous situations were found during the sync, but final state matches the expected state',
+          context,
+        );
+      } else {
+        onAccountSyncErroneousSituation?.(
+          'Erroneous situations were found during the sync, and final state does not match the expected state',
+          context,
+        );
+      }
+    }
+
     // We do this here and not in the finally statement because we want to make sure that
     // the accounts are saved / updated / deleted at least once before we set this flag
     await getUserStorageControllerInstance().setHasAccountSyncingSyncedAtLeastOnce(

yarn.lock

@@ -3415,7 +3415,7 @@ __metadata:
     "@metamask/base-controller": "npm:^7.1.1"
     "@metamask/controller-utils": "npm:^11.4.5"
     "@metamask/keyring-controller": "npm:^19.0.3"
-    "@metamask/profile-sync-controller": "npm:^4.0.0"
+    "@metamask/profile-sync-controller": "npm:^4.0.1"
     "@metamask/utils": "npm:^11.0.1"
     "@types/jest": "npm:^27.4.1"
     "@types/readable-stream": "npm:^2.3.0"
@@ -3596,7 +3596,7 @@ __metadata:
   languageName: unknown
   linkType: soft
 
-"@metamask/profile-sync-controller@npm:^4.0.0, @metamask/profile-sync-controller@workspace:packages/profile-sync-controller":
+"@metamask/profile-sync-controller@npm:^4.0.1, @metamask/profile-sync-controller@workspace:packages/profile-sync-controller":
   version: 0.0.0-use.local
   resolution: "@metamask/profile-sync-controller@workspace:packages/profile-sync-controller"
   dependencies: