fix(connect-evm): suppress switchChain _failed event on successful addEthereumChain recovery

packages/analytics/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add optional `failure_reason` property to `MMConnectProperties` in `schema.ts`, attached by producers on `mmconnect_wallet_action_failed` and `mmconnect_connection_failed`. Mirrors [`metamask-sdk-analytics-api#31`](https://github.com/consensys-vertical-apps/metamask-sdk-analytics-api/pull/31). ([#290](https://github.com/MetaMask/connect-monorepo/pull/290))
+
 ### Removed
 
 - Removed `MobileSDKConnectV2Payload` / `MobileSDKConnectV2Properties` from `schema.ts` and the `EventV2` oneOf; the `mobile/sdk-connect-v2` namespace has no emitters after [`metamask-mobile#27864`](https://github.com/MetaMask/metamask-mobile/pull/27864) and [`metamask-mobile#28322`](https://github.com/MetaMask/metamask-mobile/pull/28322), and the V2 endpoint is being updated to reject the namespace in [`metamask-sdk-analytics-api#29`](https://github.com/consensys-vertical-apps/metamask-sdk-analytics-api/pull/29). Internal types only — no change to the public API.

packages/analytics/src/schema.ts

@@ -559,6 +559,15 @@ export type components = {
       dapp_requested_chains?: string[];
       /** @description Array of CAIP-2 chain IDs that the user has permissioned */
       user_permissioned_chains?: string[];
+      /**
+       * @description Short tag describing why a failed event fired (e.g.
+       * `transport_timeout`, `wallet_internal_error`, `wallet_unauthorized`).
+       * Only set on `mmconnect_connection_failed` and
+       * `mmconnect_wallet_action_failed`. Open string for now — once we have
+       * enough data we may convert this to a closed enum. Mirrors the field
+       * in `api.spec.yml` of the analytics-api repo.
+       */
+      failure_reason?: string;
     };
   };
   responses: never;

packages/connect-evm/CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Remove `@metamask/chain-agnostic-permission` dependency. The two helpers used from it (`getEthAccounts`, `getPermittedEthChainIds`) and the `parseScopeString` utility are now implemented locally on top of `@metamask/utils` primitives. This drops the transitive `@metamask/controller-utils` / `lodash` / `bn.js` / `eth-ens-namehash` / `fast-deep-equal` / `@metamask/ethjs-unit` chain from the `connect-evm` bundle.
 
+### Fixed
+
+- Stopped emitting `mmconnect_wallet_action_failed` for the `wallet_switchEthereumChain` attempt on the `"Unrecognized chain ID" → wallet_addEthereumChain` recovery path. The shim now only emits the failed event when there's no recovery to attempt (no `chainConfiguration` provided) or when the error is unrelated to a missing chain. A successful recovery used to produce four Mixpanel events for what is logically one chain change (`switch _requested`, `switch _failed`, `add _requested`, `add _succeeded`); it now produces three (`switch _requested`, `add _requested`, `add _succeeded`). If the recovery's own `wallet_addEthereumChain` call also fails, that failure is still tracked by `#addEthereumChain`'s own `_failed` event, so the overall flow still surfaces exactly one `_failed` per logical chain change. ([#294](https://github.com/MetaMask/connect-monorepo/pull/294))
+
 ## [1.2.0]
 
 ### Added

packages/connect-evm/src/connect.test.ts

@@ -1,4 +1,7 @@
 /* eslint-disable @typescript-eslint/no-shadow -- Vitest globals */
+
+/* eslint-disable @typescript-eslint/naming-convention -- analytics event names are snake_case by schema convention */
+import { analytics } from '@metamask/analytics';
 import type { SessionData, MultichainCore } from '@metamask/connect-multichain';
 import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest';
 
@@ -59,7 +62,6 @@ function createMockCore(): MockCore {
   const storageSet = vi.fn().mockResolvedValue(undefined);
 
   const mockCore = {
-    // eslint-disable-next-line @typescript-eslint/naming-convention -- mock mirrors real class _status
     _status: _status as ConnectEvmStatus,
     get status(): ConnectEvmStatus {
       return this._status;
@@ -97,7 +99,13 @@ function createMockCore(): MockCore {
         get: storageGet,
         set: storageSet,
       },
+      getAnonId: vi.fn().mockResolvedValue('test-anon-id'),
+    },
+    options: {
+      dapp: { url: 'https://example.com' },
+      versions: { 'connect-evm': '0.0.0-test' },
     },
+    transportType: 'mwp',
   };
 
   mockCore._status = _status;
@@ -154,7 +162,7 @@ describe('MetamaskConnectEVM', () => {
 
       await client.getProvider().request({
         method: 'wallet_requestPermissions',
-        // eslint-disable-next-line @typescript-eslint/naming-convention
+
         params: [{ eth_accounts: {} }],
       });
 
@@ -757,6 +765,7 @@ describe('MetamaskConnectEVM', () => {
   describe('switchChain', () => {
     let mockCore: MockCore;
     let client: Awaited<ReturnType<typeof MetamaskConnectEVM.create>>;
+    let trackSpy: ReturnType<typeof vi.spyOn>;
 
     const polygonChainConfiguration = {
       chainId: '0x89',
@@ -765,6 +774,11 @@ describe('MetamaskConnectEVM', () => {
       rpcUrls: ['https://polygon-rpc.com'],
     };
 
+    const trackedEventNames = (): string[] =>
+      (trackSpy.mock.calls as [string, unknown][]).map(
+        ([eventName]) => eventName,
+      );
+
     beforeEach(async () => {
       mockCore = createMockCore();
       mockCore.storage.adapter.get.mockResolvedValue(JSON.stringify('0x1'));
@@ -787,6 +801,12 @@ describe('MetamaskConnectEVM', () => {
       // Ignore the eth_accounts call made during the connect flow above so
       // each test can assert against a clean call log.
       mockCore.transport.sendEip1193Message.mockClear();
+
+      // Spy on analytics.track after the connect-time events have already
+      // fired so test assertions only see calls made during the test body.
+      trackSpy = vi.spyOn(analytics, 'track').mockImplementation(() => {
+        // intentionally noop — we only want the spy's call record
+      });
     });
 
     it('falls back to wallet_addEthereumChain when wallet_switchEthereumChain fails with "Unrecognized chain ID" and chainConfiguration is provided', async () => {
@@ -815,9 +835,21 @@ describe('MetamaskConnectEVM', () => {
         'wallet_switchEthereumChain',
         'wallet_addEthereumChain',
       ]);
+
+      // The recovery succeeded — Mixpanel should not see a `_failed` event
+      // for the swallowed switch attempt. From the user's perspective this
+      // is one successful chain change.
+      expect(trackedEventNames()).toEqual([
+        'mmconnect_wallet_action_requested', // switch
+        'mmconnect_wallet_action_requested', // add
+        'mmconnect_wallet_action_succeeded', // add
+      ]);
+      expect(trackedEventNames()).not.toContain(
+        'mmconnect_wallet_action_failed',
+      );
     });
 
-    it('rethrows the original "Unrecognized chain ID" error (preserving code 4902) when no chainConfiguration is provided', async () => {
+    it('still emits `_failed` for the switch when recovery is impossible (no chainConfiguration provided)', async () => {
       mockCore.transport.sendEip1193Message.mockImplementation(
         async (request: { method: string }) => {
           if (request.method === 'wallet_switchEthereumChain') {
@@ -843,6 +875,13 @@ describe('MetamaskConnectEVM', () => {
       );
       expect(calls).toEqual(['wallet_switchEthereumChain']);
       expect(calls).not.toContain('wallet_addEthereumChain');
+
+      // No recovery is going to happen, so the switch failure must surface
+      // to Mixpanel — otherwise the failure is invisible.
+      expect(trackedEventNames()).toEqual([
+        'mmconnect_wallet_action_requested',
+        'mmconnect_wallet_action_failed',
+      ]);
     });
 
     it('rethrows non "Unrecognized chain ID" errors without falling back, even when chainConfiguration is provided', async () => {
@@ -867,6 +906,60 @@ describe('MetamaskConnectEVM', () => {
       );
       expect(calls).toEqual(['wallet_switchEthereumChain']);
       expect(calls).not.toContain('wallet_addEthereumChain');
+
+      // The catch's recovery branch only fires for "Unrecognized chain ID"
+      // errors; everything else must still emit `_failed` (or `_rejected`
+      // post the rejection classifier — irrelevant here, both are tracked
+      // events that must fire).
+      const eventNames = trackedEventNames();
+      expect(eventNames).toContain('mmconnect_wallet_action_requested');
+      expect(
+        eventNames.some(
+          (name) =>
+            name === 'mmconnect_wallet_action_failed' ||
+            name === 'mmconnect_wallet_action_rejected',
+        ),
+      ).toBe(true);
+    });
+
+    it('emits `_failed` for the add when recovery itself fails (suppression only applies on successful recovery)', async () => {
+      mockCore.transport.sendEip1193Message.mockImplementation(
+        async (request: { method: string }) => {
+          if (request.method === 'wallet_switchEthereumChain') {
+            const error = new Error('Unrecognized chain ID 0x89') as Error & {
+              code: number;
+            };
+            error.code = 4902;
+            throw error;
+          }
+          if (request.method === 'wallet_addEthereumChain') {
+            throw new Error('Wallet refused to add the chain');
+          }
+          return { result: null, id: 1, jsonrpc: '2.0' as const };
+        },
+      );
+
+      await expect(
+        client.switchChain({
+          chainId: '0x89',
+          chainConfiguration: polygonChainConfiguration,
+        }),
+      ).rejects.toThrow('Wallet refused to add the chain');
+
+      // The switch attempt's `_failed` is still suppressed (the user's
+      // intent was a chain change, not specifically a switch). The
+      // overall failure surfaces through the add's own `_failed` event.
+      // Net effect: one `_failed` per logical chain change, regardless of
+      // which RPC method the wallet rejected.
+      const eventNames = trackedEventNames();
+      expect(eventNames).toEqual([
+        'mmconnect_wallet_action_requested', // switch
+        'mmconnect_wallet_action_requested', // add
+        'mmconnect_wallet_action_failed', // add
+      ]);
+      expect(
+        eventNames.filter((name) => name === 'mmconnect_wallet_action_failed'),
+      ).toHaveLength(1);
     });
   });
 
@@ -897,7 +990,7 @@ describe('MetamaskConnectEVM', () => {
       // Call wallet_requestPermissions — should force a new request
       await client.getProvider().request({
         method: 'wallet_requestPermissions',
-        // eslint-disable-next-line @typescript-eslint/naming-convention
+
         params: [{ eth_accounts: {} }],
       });
 
@@ -937,7 +1030,7 @@ describe('MetamaskConnectEVM', () => {
 
       await client.getProvider().request({
         method: 'wallet_requestPermissions',
-        // eslint-disable-next-line @typescript-eslint/naming-convention
+
         params: [{ eth_accounts: {} }],
       });
 

packages/connect-evm/src/connect.ts

@@ -581,13 +581,22 @@ export class MetamaskConnectEVM {
       }
       return Promise.resolve();
     } catch (error) {
-      await this.#trackWalletActionFailed(method, scope, params, error);
+      // "Unrecognized chain ID" + a chainConfiguration is the recovery path:
+      // the wallet doesn't know this chain, so we silently fall through to
+      // `wallet_addEthereumChain`. From the dapp/user's perspective this is
+      // still one logical chain change with one outcome, so we let the add's
+      // own `_succeeded` / `_failed` event represent the whole flow rather
+      // than emitting a `_failed` here for an implementation detail the
+      // user never sees. Without this suppression a successful recovery
+      // produces four events in Mixpanel (switch _requested, switch _failed,
+      // add _requested, add _succeeded) instead of the three it should.
       const isChainMissingInWallet = (error as Error).message.includes(
         'Unrecognized chain ID',
       );
       if (isChainMissingInWallet && chainConfiguration) {
         return this.#addEthereumChain(chainConfiguration);
       }
+      await this.#trackWalletActionFailed(method, scope, params, error);
       throw error;
     }
   }

packages/connect-multichain/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Attach a `failure_reason` tag to `mmconnect_wallet_action_failed` and `mmconnect_connection_failed` events via a new `classifyFailureReason` helper, distinguishing transport timeouts, transport disconnects, EIP-1193 wallet errors (`4100 wallet_unauthorized`, `4200 wallet_method_unsupported`, `4902 unrecognised_chain`), and JSON-RPC wallet errors (`-32601`, `-32602`, `-32603`, plus the `-32000…-32099` server-error range), with an `unknown` fallback. Schema-side: [`metamask-sdk-analytics-api#31`](https://github.com/consensys-vertical-apps/metamask-sdk-analytics-api/pull/31). ([#290](https://github.com/MetaMask/connect-monorepo/pull/290))
+
 ## [0.13.0]
 
 ### Uncategorized

packages/connect-multichain/src/domain/utils/index.ts

@@ -8,6 +8,8 @@ export function getVersion(): string {
 }
 
 export {
+  classifyFailureReason,
   getWalletActionAnalyticsProperties,
   isRejectionError,
 } from '../../multichain/utils/analytics';
+export type { FailureReason } from '../../multichain/utils/analytics';

packages/connect-multichain/src/multichain/index.ts

@@ -29,6 +29,7 @@ import {
   TransportType,
 } from '../domain';
 import {
+  classifyFailureReason,
   getBaseAnalyticsProperties,
   isRejectionError,
 } from './utils/analytics';
@@ -757,6 +758,7 @@ export class MetaMaskConnectMultichain extends MultichainCore {
             analytics.track('mmconnect_connection_failed', {
               ...baseProps,
               transport_type: transportType,
+              failure_reason: classifyFailureReason(error),
             });
           }
         } catch {

packages/connect-multichain/src/multichain/rpc/requestRouter.test.ts

@@ -1,5 +1,7 @@
 /* eslint-disable id-length -- vitest alias */
 /* eslint-disable no-empty-function -- Empty mock functions */
+/* eslint-disable @typescript-eslint/unbound-method -- referencing the mocked `analytics.track` is intentional in spy assertions */
+/* eslint-disable @typescript-eslint/naming-convention -- analytics event properties are snake_case by schema convention */
 import { analytics } from '@metamask/analytics';
 import * as t from 'vitest';
 
@@ -226,4 +228,47 @@ t.describe('RequestRouter', () => {
       },
     );
   });
+
+  t.describe('failure_reason classification on wallet actions', () => {
+    t.it(
+      'attaches `failure_reason: wallet_internal_error` when the wallet returns code -32603',
+      async () => {
+        mockTransport.request.mockResolvedValue({
+          error: { code: -32603, message: 'Internal error' },
+        });
+
+        await t
+          .expect(requestRouter.invokeMethod(baseOptions))
+          .rejects.toBeInstanceOf(RPCInvokeMethodErr);
+
+        t.expect(analytics.track).toHaveBeenCalledWith(
+          'mmconnect_wallet_action_failed',
+          t.expect.objectContaining({
+            failure_reason: 'wallet_internal_error',
+            method: 'eth_sendTransaction',
+          }),
+        );
+      },
+    );
+
+    t.it(
+      'attaches `failure_reason: transport_timeout` when transport throws a timeout',
+      async () => {
+        const timeoutErr = new Error('Transport request timed out');
+        timeoutErr.name = 'TransportTimeoutError';
+        mockTransport.request.mockRejectedValue(timeoutErr);
+
+        await t
+          .expect(requestRouter.invokeMethod(baseOptions))
+          .rejects.toThrow();
+
+        t.expect(analytics.track).toHaveBeenCalledWith(
+          'mmconnect_wallet_action_failed',
+          t.expect.objectContaining({
+            failure_reason: 'transport_timeout',
+          }),
+        );
+      },
+    );
+  });
 });

packages/connect-multichain/src/multichain/rpc/requestRouter.ts

@@ -22,6 +22,7 @@ import {
 } from '../../domain';
 import { openDeeplink } from '../utils';
 import {
+  classifyFailureReason,
   getWalletActionAnalyticsProperties,
   isRejectionError,
 } from '../utils/analytics';
@@ -138,7 +139,7 @@ export class RequestRouter {
       if (isRejection) {
         await this.#trackWalletActionRejected(options);
       } else {
-        await this.#trackWalletActionFailed(options);
+        await this.#trackWalletActionFailed(options, error);
       }
       if (error instanceof RPCInvokeMethodErr) {
         throw error;
@@ -188,14 +189,21 @@ export class RequestRouter {
   /**
    * Tracks wallet action failed event.
    *
-   * @param options
+   * @param options - The invoke method options.
+   * @param error - The error that caused the failure (used to classify the
+   * `failure_reason` property on the event).
    */
-  async #trackWalletActionFailed(options: InvokeMethodOptions): Promise<void> {
+  async #trackWalletActionFailed(
+    options: InvokeMethodOptions,
+    error: unknown,
+  ): Promise<void> {
     const props = await getWalletActionAnalyticsProperties(
       this.config,
       this.config.storage,
       options,
       this.transportType,
+      // eslint-disable-next-line @typescript-eslint/naming-convention -- analytics property is snake_case by schema convention
+      { failure_reason: classifyFailureReason(error) },
     );
     analytics.track('mmconnect_wallet_action_failed', props);
   }