From 335b3bb2d55550b0512324ebce6d00d8d3c1b1cb Mon Sep 17 00:00:00 2001
From: Salim TOUBAL <salim.toubal@outlook.com>
Date: Thu, 6 Feb 2025 22:35:32 +0100
Subject: [PATCH] fix: fix noisy sentry logs (#30163)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
fix noisy sentry logs
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30163?quickstart=1)

## **Related issues**

Fixes: #30162

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/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-extension/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.
---
 .../{142.1.test.ts => 134.1.test.ts}          | 49 ++++++++++++++++---
 app/scripts/migrations/{142.1.ts => 134.1.ts} | 16 ++++--
 app/scripts/migrations/index.js               |  2 +-
 3 files changed, 56 insertions(+), 11 deletions(-)
 rename app/scripts/migrations/{142.1.test.ts => 134.1.test.ts} (86%)
 rename app/scripts/migrations/{142.1.ts => 134.1.ts} (95%)

diff --git a/app/scripts/migrations/142.1.test.ts b/app/scripts/migrations/134.1.test.ts
similarity index 86%
rename from app/scripts/migrations/142.1.test.ts
rename to app/scripts/migrations/134.1.test.ts
index 41b0194f70cd..e83f9df30647 100644
--- a/app/scripts/migrations/142.1.test.ts
+++ b/app/scripts/migrations/134.1.test.ts
@@ -1,5 +1,5 @@
 import { NetworkConfiguration } from '@metamask/network-controller';
-import { migrate, version } from './142.1';
+import { migrate, version } from './134.1';
 
 describe(`Migration ${version}`, () => {
   let originalSentry: typeof global.sentry;
@@ -21,14 +21,14 @@ describe(`Migration ${version}`, () => {
     sentryCaptureExceptionMock.mockClear();
   });
 
-  it('updates the meta version to 142.1 regardless of state content', async () => {
+  it('updates the meta version to 134.1 regardless of state content', async () => {
     const dummyState = {
       meta: { version: 0 },
       data: {},
     };
 
     const migrated = await migrate(dummyState);
-    expect(migrated.meta.version).toBe(142.1);
+    expect(migrated.meta.version).toBe(134.1);
   });
 
   it('returns original state if AccountsController is missing', async () => {
@@ -41,9 +41,8 @@ describe(`Migration ${version}`, () => {
     };
     const result = await migrate(originalState);
 
-    expect(sentryCaptureExceptionMock).toHaveBeenCalled();
     expect(result.data).toEqual(originalState.data);
-    expect(result.meta.version).toBe(142.1);
+    expect(result.meta.version).toBe(134.1);
   });
 
   it('returns original state if AccountsController is not an object', async () => {
@@ -242,7 +241,6 @@ describe(`Migration ${version}`, () => {
     };
 
     const result = await migrate(originalState);
-    expect(sentryCaptureExceptionMock).toHaveBeenCalled();
     expect(result.data).toEqual(originalState.data);
   });
 
@@ -313,6 +311,43 @@ describe(`Migration ${version}`, () => {
     >;
     expect(tokensControllerState.tokens).toEqual(originalTokens);
 
-    expect(result.meta.version).toBe(142.1);
+    expect(result.meta.version).toBe(134.1);
+  });
+
+  it('returns original state and logs error if tokens is not empty but allTokensForChain is not an object', async () => {
+    const originalState = {
+      meta: { version: 0 },
+      data: {
+        AccountsController: {
+          internalAccounts: { selectedAccount: '0x123' },
+        },
+        NetworkController: {
+          selectedNetworkClientId: 'mainnet',
+          networkConfigurationsByChainId: {
+            '0x1': {
+              rpcEndpoints: [{ networkClientId: 'mainnet' }],
+            },
+          },
+        },
+        TokensController: {
+          tokens: [{ address: '0xtokenA' }], // Non-empty tokens array
+          allTokens: {
+            '0x1': null, // allTokensForChain is not an object
+          },
+        },
+      },
+    };
+
+    const result = await migrate(originalState);
+
+    expect(sentryCaptureExceptionMock).toHaveBeenCalledWith(
+      expect.objectContaining({
+        message: expect.stringContaining(
+          `Migration ${version}: tokens is not an empty array, but allTokensForChain is not an object.`,
+        ),
+      }),
+    );
+
+    expect(result.data).toEqual(originalState.data);
   });
 });
diff --git a/app/scripts/migrations/142.1.ts b/app/scripts/migrations/134.1.ts
similarity index 95%
rename from app/scripts/migrations/142.1.ts
rename to app/scripts/migrations/134.1.ts
index b9398fb01cc2..0bcc7aeb19f5 100644
--- a/app/scripts/migrations/142.1.ts
+++ b/app/scripts/migrations/134.1.ts
@@ -7,7 +7,7 @@ type VersionedData = {
   data: Record<string, unknown>;
 };
 
-export const version = 142.1;
+export const version = 134.1;
 
 /**
  * This migration attempts to reset `TokensController.tokens` to the list of tokens
@@ -168,17 +168,27 @@ function transformState(
     return state;
   }
 
+  const { tokens } = tokensControllerState;
   const { allTokens } = tokensControllerState;
   const allTokensForChain = allTokens[currentChainId];
-  if (!isObject(allTokensForChain)) {
+
+  if (
+    Array.isArray(tokens) &&
+    tokens.length > 0 &&
+    !isObject(allTokensForChain)
+  ) {
     global.sentry?.captureException?.(
       new Error(
-        `Migration ${version}: allTokens["${currentChainId}"] is missing or not an object.`,
+        `Migration ${version}: tokens is not an empty array, but allTokensForChain is not an object.`,
       ),
     );
     return state;
   }
 
+  if (!isObject(allTokensForChain)) {
+    return state;
+  }
+
   const accountTokens = allTokensForChain[selectedAccount];
   if (!Array.isArray(accountTokens)) {
     global.sentry?.captureException?.(
diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js
index 19f87e1f3c8f..6df962d4709f 100644
--- a/app/scripts/migrations/index.js
+++ b/app/scripts/migrations/index.js
@@ -158,6 +158,7 @@ const migrations = [
   require('./133.1'),
   require('./133.2'),
   require('./134'),
+  require('./134.1'),
   require('./135'),
   require('./136'),
   require('./137'),
@@ -166,7 +167,6 @@ const migrations = [
   require('./140'),
   require('./141'),
   require('./142'),
-  require('./142.1'),
   require('./143'),
 ];