From baf18c1ebab3908a8b736a15dbd6979c9bc3700e Mon Sep 17 00:00:00 2001 From: Alex Donesky Date: Mon, 11 May 2026 12:37:17 -0500 Subject: [PATCH 01/10] feat(analytics): attach `failure_reason` to `_failed` events + fix `isRejectionError` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bundled changes from the WAPI-1253 spike that share scaffolding (both reuse `getUnwrappedErrorDetails`): 1. **Attach `failure_reason` to `_failed` events.** Both `mmconnect_wallet_action_failed` and `mmconnect_connection_failed` now carry a producer-side tag describing why they fired — `transport_timeout`, `transport_disconnect`, `wallet_method_unsupported`, `wallet_invalid_params`, `wallet_internal_error`, `wallet_custom_error`, `no_active_session`, `unrecognised_chain`, `rpc_node_http_error`, `rpc_node_request_error`, `rpc_node_response_error`, or `unknown`. The schema-side change lives in: consensys-vertical-apps/metamask-sdk-analytics-api#31 Note: the `rpc_node_*` branches exist in the classifier but no producer currently emits them — read-only RPC failures still throw without tracking. A follow-up PR may wire them up. 2. **Unwrap `RPCInvokeMethodErr` in `isRejectionError`.** The router re-throws wallet errors wrapped in `RPCInvokeMethodErr`, whose outer `code` is the SDK's static `53`. Without unwrapping, wallet-side `code: 4001` rejections fell through to the brittle message-substring fallback, and any rejection whose message didn't happen to contain "reject"/"denied"/"cancel" was misclassified as a failure. Also tightened the "user" message heuristic so unrelated phrases like "user operation reverted" (Account Abstraction) no longer count as rejections. Tests: - New `analytics.test.ts` covering `isRejectionError` (including the `RPCInvokeMethodErr` unwrap and the tightened "user" heuristic) and every branch of `classifyFailureReason`. - New `requestRouter.test.ts` assertions for `failure_reason` on wallet internal errors and transport timeouts, plus the regression test that wallet-side 4001 with a non-matching message now lands in `_rejected`. Schema mirror: hand-edited `packages/analytics/src/schema.ts` to add the new optional property. Regenerating via `openapi-typescript@7` would have produced a 1k-line diff (the file was generated by an older version that emitted `type` aliases instead of `interface`s); hand-edit keeps the diff tight and matches the existing style. Related: WAPI-1253. Sibling PRs from the same spike: - #291 — dedupe EVM shim tracking. - #292 — `isRejectionError` unwrap fix on its own (this PR carries the same fix as a dependency; whichever lands second is a no-op). --- packages/analytics/CHANGELOG.md | 4 + packages/analytics/src/schema.ts | 8 + packages/connect-multichain/CHANGELOG.md | 9 + .../src/domain/utils/index.ts | 2 + .../src/multichain/index.ts | 2 + .../src/multichain/rpc/requestRouter.test.ts | 70 ++++++ .../src/multichain/rpc/requestRouter.ts | 14 +- .../src/multichain/utils/analytics.test.ts | 203 ++++++++++++++++++ .../src/multichain/utils/analytics.ts | 191 +++++++++++++++- 9 files changed, 493 insertions(+), 10 deletions(-) create mode 100644 packages/connect-multichain/src/multichain/utils/analytics.test.ts diff --git a/packages/analytics/CHANGELOG.md b/packages/analytics/CHANGELOG.md index 3c49524d..fc7a5d03 100644 --- a/packages/analytics/CHANGELOG.md +++ b/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`, mirroring [`metamask-sdk-analytics-api#31`](https://github.com/consensys-vertical-apps/metamask-sdk-analytics-api/pull/31). Producers attach this on `mmconnect_wallet_action_failed` and `mmconnect_connection_failed` to distinguish transport timeouts, wallet-side JSON-RPC errors, RPC node failures, etc. + ### 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. diff --git a/packages/analytics/src/schema.ts b/packages/analytics/src/schema.ts index 166d3c71..ce6cbac1 100644 --- a/packages/analytics/src/schema.ts +++ b/packages/analytics/src/schema.ts @@ -559,6 +559,14 @@ 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`, `rpc_node_error`). 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; diff --git a/packages/connect-multichain/CHANGELOG.md b/packages/connect-multichain/CHANGELOG.md index 154c2c20..20e5dae3 100644 --- a/packages/connect-multichain/CHANGELOG.md +++ b/packages/connect-multichain/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Attach a `failure_reason` property to `mmconnect_wallet_action_failed` and `mmconnect_connection_failed` analytics events. The new `classifyFailureReason` helper tags transport timeouts, transport disconnects, wallet JSON-RPC errors (method unsupported / invalid params / internal), provider custom errors, "no active session", "unrecognised chain", and read-only RPC node failures (HTTP / request / response), with an `unknown` fallback. The schema-side change lives in [`metamask-sdk-analytics-api#31`](https://github.com/consensys-vertical-apps/metamask-sdk-analytics-api/pull/31). + +### Fixed + +- `isRejectionError` now unwraps `RPCInvokeMethodErr` so a wallet-side `code: 4001` survives the SDK's transport-boundary wrapping and is correctly classified as a user rejection. Previously the outer `code: 53` masked the inner code, and rejections were only caught via the fragile message-substring fallback. +- Tightened the `isRejectionError` message heuristics so generic "user" mentions (e.g. "user operation reverted") are no longer treated as user rejections — only explicit phrases like "user rejected" / "user denied" / "user cancelled". + ## [0.13.0] ### Uncategorized diff --git a/packages/connect-multichain/src/domain/utils/index.ts b/packages/connect-multichain/src/domain/utils/index.ts index c3c5ff80..99d76a9a 100644 --- a/packages/connect-multichain/src/domain/utils/index.ts +++ b/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'; diff --git a/packages/connect-multichain/src/multichain/index.ts b/packages/connect-multichain/src/multichain/index.ts index c2619a39..cf2e51fe 100644 --- a/packages/connect-multichain/src/multichain/index.ts +++ b/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 { diff --git a/packages/connect-multichain/src/multichain/rpc/requestRouter.test.ts b/packages/connect-multichain/src/multichain/rpc/requestRouter.test.ts index ab598a94..590ce498 100644 --- a/packages/connect-multichain/src/multichain/rpc/requestRouter.test.ts +++ b/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,72 @@ 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', + }), + ); + }, + ); + + t.it( + 'classifies wallet-side rejection code 4001 as `_rejected` even when the message would not match the heuristics on its own', + async () => { + // Wallet sends code: 4001 with a custom message that does not contain + // "reject"/"denied"/"cancel"/"user rejected". Before the unwrap fix, + // this would land in `_failed`; after the fix it should be `_rejected`. + mockTransport.request.mockResolvedValue({ + error: { code: 4001, message: 'Some non-standard message' }, + }); + + await t + .expect(requestRouter.invokeMethod(baseOptions)) + .rejects.toBeInstanceOf(RPCInvokeMethodErr); + + t.expect(analytics.track).toHaveBeenCalledWith( + 'mmconnect_wallet_action_rejected', + t.expect.anything(), + ); + t.expect(analytics.track).not.toHaveBeenCalledWith( + 'mmconnect_wallet_action_failed', + t.expect.anything(), + ); + }, + ); + }); }); diff --git a/packages/connect-multichain/src/multichain/rpc/requestRouter.ts b/packages/connect-multichain/src/multichain/rpc/requestRouter.ts index cdf3e9f7..576bdc7d 100644 --- a/packages/connect-multichain/src/multichain/rpc/requestRouter.ts +++ b/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 { + async #trackWalletActionFailed( + options: InvokeMethodOptions, + error: unknown, + ): Promise { 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); } diff --git a/packages/connect-multichain/src/multichain/utils/analytics.test.ts b/packages/connect-multichain/src/multichain/utils/analytics.test.ts new file mode 100644 index 00000000..b59da3d4 --- /dev/null +++ b/packages/connect-multichain/src/multichain/utils/analytics.test.ts @@ -0,0 +1,203 @@ +/* eslint-disable id-length -- vitest alias */ +import * as t from 'vitest'; + +import { + classifyFailureReason, + getWalletActionAnalyticsProperties, + isRejectionError, +} from './analytics'; +import { + type InvokeMethodOptions, + type MultichainOptions, + RPCHttpErr, + RPCInvokeMethodErr, + RPCReadonlyRequestErr, + RPCReadonlyResponseErr, + type Scope, + type StoreClient, + TransportType, +} from '../../domain'; + +t.describe('isRejectionError', () => { + t.it('returns false for non-object errors', () => { + t.expect(isRejectionError(null)).toBe(false); + t.expect(isRejectionError(undefined)).toBe(false); + t.expect(isRejectionError('User rejected')).toBe(false); + t.expect(isRejectionError(42)).toBe(false); + }); + + t.it('returns true for EIP-1193 user-rejected code 4001', () => { + t.expect(isRejectionError({ code: 4001, message: 'anything' })).toBe(true); + }); + + t.it('returns true for code 4100 (unauthorized)', () => { + t.expect(isRejectionError({ code: 4100, message: 'denied by user' })).toBe( + true, + ); + }); + + t.it('returns true for messages mentioning explicit user action', () => { + t.expect(isRejectionError({ message: 'User rejected the request' })).toBe( + true, + ); + t.expect(isRejectionError({ message: 'User denied signature' })).toBe(true); + t.expect(isRejectionError({ message: 'User cancelled' })).toBe(true); + t.expect(isRejectionError({ message: 'Request rejected' })).toBe(true); + }); + + t.it( + 'does NOT treat unrelated "user" mentions as rejections (avoids false positives)', + () => { + // Account Abstraction error messages often contain "user operation" + t.expect(isRejectionError({ message: 'user operation reverted' })).toBe( + false, + ); + t.expect(isRejectionError({ message: 'user agent not supported' })).toBe( + false, + ); + }, + ); + + t.it( + 'unwraps RPCInvokeMethodErr to detect wallet-side rejection codes', + () => { + // Outer error has code: 53 (the SDK's static internal code), but the + // inner wallet code is 4001 — the classifier must look at rpcCode. + const wrapped = new RPCInvokeMethodErr( + 'RPC Request failed with code 4001: Some random message that does not match heuristics', + 4001, + 'Some random message that does not match heuristics', + ); + t.expect(isRejectionError(wrapped)).toBe(true); + }, + ); +}); + +t.describe('classifyFailureReason', () => { + t.it('classifies transport timeout from TransportTimeoutError name', () => { + const error = new Error('Transport request timed out'); + error.name = 'TransportTimeoutError'; + t.expect(classifyFailureReason(error)).toBe('transport_timeout'); + }); + + t.it( + 'classifies transport timeout from DefaultTransport plain message', + () => { + // DefaultTransport throws `new Error('Request timeout')` (not a + // TransportTimeoutError) on its own setTimeout path — we still want to + // classify this as a timeout. + t.expect(classifyFailureReason(new Error('Request timeout'))).toBe( + 'transport_timeout', + ); + }, + ); + + t.it('classifies transport disconnect from TransportError name', () => { + const error = new Error('Chrome port not connected'); + error.name = 'TransportError'; + t.expect(classifyFailureReason(error)).toBe('transport_disconnect'); + }); + + t.it('classifies the SDK "No active session found" sentinel', () => { + t.expect(classifyFailureReason(new Error('No active session found'))).toBe( + 'no_active_session', + ); + }); + + t.it('classifies the "Unrecognized chain" message', () => { + t.expect( + classifyFailureReason(new Error('Unrecognized chain ID "0xfa"')), + ).toBe('unrecognised_chain'); + }); + + t.it('classifies wallet JSON-RPC method-not-found', () => { + const error = new RPCInvokeMethodErr('inner', -32601, 'Method not found'); + t.expect(classifyFailureReason(error)).toBe('wallet_method_unsupported'); + }); + + t.it('classifies wallet JSON-RPC invalid params', () => { + const error = new RPCInvokeMethodErr('inner', -32602, 'Invalid params'); + t.expect(classifyFailureReason(error)).toBe('wallet_invalid_params'); + }); + + t.it('classifies wallet JSON-RPC internal error', () => { + const error = new RPCInvokeMethodErr('inner', -32603, 'Internal error'); + t.expect(classifyFailureReason(error)).toBe('wallet_internal_error'); + }); + + t.it('classifies provider-defined custom error code', () => { + const error = new RPCInvokeMethodErr('inner', 4900, 'Disconnected'); + t.expect(classifyFailureReason(error)).toBe('wallet_custom_error'); + }); + + t.it('classifies read-only RPC HTTP errors', () => { + t.expect( + classifyFailureReason( + new RPCHttpErr('https://example.com', 'eth_blockNumber', 503), + ), + ).toBe('rpc_node_http_error'); + }); + + t.it( + 'classifies read-only RPC request errors (fetch timeouts, aborts)', + () => { + t.expect( + classifyFailureReason( + new RPCReadonlyRequestErr('Request timeout after 30000ms'), + ), + ).toBe('rpc_node_request_error'); + }, + ); + + t.it('classifies read-only RPC response errors (malformed JSON)', () => { + t.expect( + classifyFailureReason(new RPCReadonlyResponseErr('Unexpected token <')), + ).toBe('rpc_node_response_error'); + }); + + t.it('falls back to "unknown" for unrecognised errors', () => { + t.expect( + classifyFailureReason(new Error('Something exploded somewhere')), + ).toBe('unknown'); + t.expect(classifyFailureReason(null)).toBe('unknown'); + t.expect(classifyFailureReason('a string')).toBe('unknown'); + }); +}); + +t.describe('getWalletActionAnalyticsProperties', () => { + const mockOptions: MultichainOptions = { + dapp: { name: 'Test', url: 'https://test.com' }, + versions: { 'connect-multichain': '1.2.3' }, + } as unknown as MultichainOptions; + + const mockStorage = { + getAnonId: async () => Promise.resolve('anon-id-123'), + } as unknown as StoreClient; + + const invokeOptions: InvokeMethodOptions = { + scope: 'eip155:1' as Scope, + request: { method: 'personal_sign', params: [] }, + }; + + t.it('does not attach failure_reason by default', async () => { + const props = await getWalletActionAnalyticsProperties( + mockOptions, + mockStorage, + invokeOptions, + TransportType.Browser, + ); + t.expect(props).not.toHaveProperty('failure_reason'); + }); + + t.it('attaches failure_reason when passed via extra', async () => { + const props = await getWalletActionAnalyticsProperties( + mockOptions, + mockStorage, + invokeOptions, + TransportType.Browser, + // eslint-disable-next-line @typescript-eslint/naming-convention -- analytics property is snake_case by schema convention + { failure_reason: 'transport_timeout' }, + ); + t.expect(props.failure_reason).toBe('transport_timeout'); + }); +}); diff --git a/packages/connect-multichain/src/multichain/utils/analytics.ts b/packages/connect-multichain/src/multichain/utils/analytics.ts index 9fcd4be5..ffd3e42c 100644 --- a/packages/connect-multichain/src/multichain/utils/analytics.ts +++ b/packages/connect-multichain/src/multichain/utils/analytics.ts @@ -9,11 +9,77 @@ import type { StoreClient, TransportType, } from '../../domain'; -import { getPlatformType } from '../../domain'; +import { + getPlatformType, + RPCHttpErr, + RPCInvokeMethodErr, + RPCReadonlyRequestErr, + RPCReadonlyResponseErr, +} from '../../domain'; + +/** + * Tag describing the cause of a failed wallet action / connection. Surfaced + * as the `failure_reason` property on `mmconnect_wallet_action_failed` and + * `mmconnect_connection_failed` events so we can distinguish e.g. a transport + * timeout from a wallet-side internal error in Mixpanel. + * + * Intentionally a string union (not a const enum) so callers stay free to + * pass through a new bucket; the schema-side property is an open string for + * the same reason. + */ +export type FailureReason = + | 'transport_timeout' + | 'transport_disconnect' + | 'wallet_method_unsupported' + | 'wallet_invalid_params' + | 'wallet_internal_error' + | 'wallet_custom_error' + | 'session_expired' + | 'no_active_session' + | 'unrecognised_chain' + | 'rpc_node_http_error' + | 'rpc_node_request_error' + | 'rpc_node_response_error' + | 'unknown'; + +/** + * Pulls the most informative `code` / `message` pair out of an error, + * unwrapping `RPCInvokeMethodErr` so the wallet-side code (e.g. 4001) is + * visible to classifiers instead of being hidden behind the SDK's static + * `code: 53`. Falls back to the outer error if there is no inner wallet code. + * + * @param error - The error object to inspect + * @returns The most relevant `{ code, message }` pair we can extract + */ +function getUnwrappedErrorDetails(error: unknown): { + code: number | undefined; + message: string; +} { + if (typeof error !== 'object' || error === null) { + return { code: undefined, message: '' }; + } + + if (error instanceof RPCInvokeMethodErr) { + return { + code: error.rpcCode ?? error.code, + message: error.rpcMessage ?? error.message ?? '', + }; + } + + const errorObj = error as { code?: number; message?: string }; + return { + code: errorObj.code, + message: errorObj.message ?? '', + }; +} /** * Checks if an error represents a user rejection. * + * Unwraps `RPCInvokeMethodErr` so the wallet's `code: 4001` survives the + * SDK's transport-boundary wrapping (the outer error otherwise reports + * `code: 53`, which would never match the heuristics here). + * * @param error - The error object to check * @returns True if the error indicates a user rejection, false otherwise */ @@ -22,20 +88,124 @@ export function isRejectionError(error: unknown): boolean { return false; } - const errorObj = error as { code?: number; message?: string }; - const errorCode = errorObj.code; - const errorMessage = errorObj.message?.toLowerCase() ?? ''; + const { code, message } = getUnwrappedErrorDetails(error); + const errorMessage = message.toLowerCase(); return ( - errorCode === 4001 || // User rejected request (common EIP-1193 code) - errorCode === 4100 || // Unauthorized (common rejection code) + code === 4001 || // User rejected request (common EIP-1193 code) + code === 4100 || // Unauthorized (common rejection code) errorMessage.includes('reject') || errorMessage.includes('denied') || errorMessage.includes('cancel') || - errorMessage.includes('user') + // Bare "user" match is intentionally narrow to avoid false positives on + // messages like "user operation reverted" (Account Abstraction). Only + // treats it as a rejection if it sounds like the user actively declined. + errorMessage.includes('user rejected') || + errorMessage.includes('user denied') || + errorMessage.includes('user cancelled') || + errorMessage.includes('user canceled') ); } +/** + * Classifies a failed wallet action / connection error into a short tag for + * the `failure_reason` analytics property. Caller is expected to have already + * established that the error is *not* a user rejection (use `isRejectionError` + * for that branching). + * + * The taxonomy is deliberately producer-side-only — the schema accepts any + * string — so we can add buckets here without an API migration. Once the + * distribution stabilises we may convert the schema field to a closed enum. + * + * @param error - The error to classify + * @returns A short, snake_case tag describing why the operation failed + */ +export function classifyFailureReason(error: unknown): FailureReason { + // Read-only RPC client failures — these come from the configured RPC + // endpoint (e.g. Infura), not from the wallet, and are checked first so + // they aren't swallowed by the generic message heuristics below. + if (error instanceof RPCHttpErr) { + return 'rpc_node_http_error'; + } + if (error instanceof RPCReadonlyRequestErr) { + return 'rpc_node_request_error'; + } + if (error instanceof RPCReadonlyResponseErr) { + return 'rpc_node_response_error'; + } + + if (typeof error !== 'object' || error === null) { + return 'unknown'; + } + + const errorObj = error as { name?: string; message?: string }; + const errorName = errorObj.name ?? ''; + const errorMessageRaw = errorObj.message ?? ''; + const errorMessage = errorMessageRaw.toLowerCase(); + + // Transport-layer errors. Two shapes exist: + // - `TransportTimeoutError` from `@metamask/multichain-api-client` (used by + // MWP and the warmup paths of the default extension transport). It's a + // subclass of `TransportError` so we match on the name field rather than + // importing the symbol (the type lives in a runtime dependency that the + // analytics utils shouldn't pull in directly). + // - A plain `new Error('Request timeout')` thrown by `DefaultTransport`'s + // own setTimeout. Indistinguishable from other errors without the message. + if ( + errorName === 'TransportTimeoutError' || + errorMessageRaw === 'Request timeout' || + errorMessage.includes('timed out') || + errorMessage.includes('timeout') + ) { + return 'transport_timeout'; + } + if ( + errorName === 'TransportError' || + errorMessage.includes('not connected') || + errorMessage.includes('disconnect') + ) { + return 'transport_disconnect'; + } + + // SDK-thrown sentinel. + if (errorMessageRaw === 'No active session found') { + return 'no_active_session'; + } + + if (errorMessage.includes('unrecognized chain')) { + return 'unrecognised_chain'; + } + + if (errorMessage.includes('session') && errorMessage.includes('expired')) { + return 'session_expired'; + } + + // Inspect the wallet-side JSON-RPC code if there is one. Unwraps + // `RPCInvokeMethodErr` so the wallet's actual error code is visible. + const { code } = getUnwrappedErrorDetails(error); + if (typeof code === 'number') { + if (code === -32601) { + return 'wallet_method_unsupported'; + } + if (code === -32602) { + return 'wallet_invalid_params'; + } + if (code === -32603) { + return 'wallet_internal_error'; + } + // Standard JSON-RPC server error range. + if (code <= -32000 && code >= -32099) { + return 'wallet_internal_error'; + } + // Provider-defined error range (EIP-1193 + EIP-1474 custom codes). + if (code >= 1000 && code <= 4999) { + return 'wallet_custom_error'; + } + } + + return 'unknown'; +} + /** * Gets base analytics properties that are common across all events. * @@ -71,6 +241,10 @@ export async function getBaseAnalyticsProperties( * @param storage - Storage client for getting anonymous ID * @param invokeOptions - The invoke method options containing method and scope * @param transportType - The transport type to use for the analytics event + * @param extra - Optional event-specific properties. Today only used to + * attach a `failure_reason` tag to `mmconnect_wallet_action_failed` events. + * @param extra.failure_reason - A short tag describing why the operation + * failed; see `classifyFailureReason` and the `FailureReason` union. * @returns Wallet action analytics properties */ export async function getWalletActionAnalyticsProperties( @@ -78,6 +252,7 @@ export async function getWalletActionAnalyticsProperties( storage: StoreClient, invokeOptions: InvokeMethodOptions, transportType: TransportType, + extra?: { failure_reason?: FailureReason }, ): Promise<{ mmconnect_versions: Record; dapp_id: string; @@ -85,6 +260,7 @@ export async function getWalletActionAnalyticsProperties( caip_chain_id: string; anon_id: string; transport_type: TransportType; + failure_reason?: FailureReason; }> { const dappId = getDappId(options.dapp); const anonId = await storage.getAnonId(); @@ -96,5 +272,6 @@ export async function getWalletActionAnalyticsProperties( caip_chain_id: invokeOptions.scope, anon_id: anonId, transport_type: transportType, + ...(extra?.failure_reason ? { failure_reason: extra.failure_reason } : {}), }; } From 104fbb1075ff78c3bf29a155f80f3a3d60efaef9 Mon Sep 17 00:00:00 2001 From: Alex Donesky Date: Mon, 11 May 2026 13:49:20 -0500 Subject: [PATCH 02/10] fix(analytics): refine failure_reason classifier + permanent playground bench MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to manual testing of the failure_reason classifier. Two behaviour fixes in `classifyFailureReason` / `isRejectionError`, plus moves the test-bench bits into a permanent home in the playground. Classifier: - Drop `code === 4100` from `isRejectionError`. EIP-1193 `4100 Unauthorized` is what MetaMask returns when a method is not in the granted CAIP-25 scope — the multichain permission layer rejects it before any method handler runs (see core/packages/ multichain-api-middleware/src/handlers/wallet-invokeMethod.ts). That's a permission/support signal, not a user-driven rejection. Misclassifying it inflated `_rejected` and hid genuine permission issues from `_failed`. After this change, dapp calls for methods outside the granted scope correctly surface as `_failed` with `failure_reason: wallet_unauthorized`. - Add `wallet_unauthorized` to the `FailureReason` union and route `4100` to it. Distinct from `wallet_method_unsupported` because the wallet never gets to see whether the method exists. - Add explicit branches for `4200 → wallet_method_unsupported` (wallet handler exists but explicitly refuses) and `4902 → unrecognised_chain` (`wallet_switchEthereumChain` to an unknown chain) so we don't lose signal to the catch-all `wallet_custom_error` range. Playground: - Rename `FailureReasonTestBench` → `AnalyticsTestBench` and wrap in a `
` so it's a permanent, collapsible section. No longer gated on `isConnected` — it always renders so testers can also exercise the `no_active_session` branch. - Use the first connected EVM account from `session.sessionScopes` for the `personal_sign` rejection sanity-check. Calling `personal_sign(msg, 0x0000…)` made the wallet bail with `-32602` before showing the prompt, defeating the rejection check. - Move the echo server from the temp `wapi-1253-test-rig/` dir into `playground/browser-playground/scripts/analytics-echo-server.mjs` and wire it as `yarn analytics:echo`. - Document the full manual-testing workflow in the playground README (echo server + `METAMASK_ANALYTICS_ENDPOINT` env var + CSP gotcha + the multichain scope behaviour that explains why so many buckets funnel into `wallet_unauthorized`). --- packages/connect-multichain/CHANGELOG.md | 3 +- .../src/multichain/utils/analytics.test.ts | 46 +- .../src/multichain/utils/analytics.ts | 30 +- playground/browser-playground/README.md | 62 +++ playground/browser-playground/craco.config.js | 3 + playground/browser-playground/package.json | 1 + .../browser-playground/public/index.html | 2 +- .../browser-playground/scripts/README.md | 26 + .../scripts/analytics-echo-server.mjs | 130 +++++ playground/browser-playground/src/App.tsx | 7 + .../src/components/AnalyticsTestBench.tsx | 506 ++++++++++++++++++ 11 files changed, 807 insertions(+), 9 deletions(-) create mode 100644 playground/browser-playground/scripts/analytics-echo-server.mjs create mode 100644 playground/browser-playground/src/components/AnalyticsTestBench.tsx diff --git a/packages/connect-multichain/CHANGELOG.md b/packages/connect-multichain/CHANGELOG.md index 20e5dae3..76501968 100644 --- a/packages/connect-multichain/CHANGELOG.md +++ b/packages/connect-multichain/CHANGELOG.md @@ -9,12 +9,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Attach a `failure_reason` property to `mmconnect_wallet_action_failed` and `mmconnect_connection_failed` analytics events. The new `classifyFailureReason` helper tags transport timeouts, transport disconnects, wallet JSON-RPC errors (method unsupported / invalid params / internal), provider custom errors, "no active session", "unrecognised chain", and read-only RPC node failures (HTTP / request / response), with an `unknown` fallback. The schema-side change lives in [`metamask-sdk-analytics-api#31`](https://github.com/consensys-vertical-apps/metamask-sdk-analytics-api/pull/31). +- Attach a `failure_reason` property to `mmconnect_wallet_action_failed` and `mmconnect_connection_failed` analytics events. The new `classifyFailureReason` helper tags transport timeouts, transport disconnects, wallet JSON-RPC errors (method unsupported / invalid params / internal), provider custom errors (`4100 wallet_unauthorized`, `4200 wallet_method_unsupported`, `4902 unrecognised_chain`, generic `wallet_custom_error`), "no active session", "unrecognised chain", and read-only RPC node failures (HTTP / request / response), with an `unknown` fallback. The schema-side change lives in [`metamask-sdk-analytics-api#31`](https://github.com/consensys-vertical-apps/metamask-sdk-analytics-api/pull/31). ### Fixed - `isRejectionError` now unwraps `RPCInvokeMethodErr` so a wallet-side `code: 4001` survives the SDK's transport-boundary wrapping and is correctly classified as a user rejection. Previously the outer `code: 53` masked the inner code, and rejections were only caught via the fragile message-substring fallback. - Tightened the `isRejectionError` message heuristics so generic "user" mentions (e.g. "user operation reverted") are no longer treated as user rejections — only explicit phrases like "user rejected" / "user denied" / "user cancelled". +- `isRejectionError` no longer treats `code: 4100 Unauthorized` as a user rejection. `4100` is returned by the CAIP-25 permission layer when a method isn't in the granted scope (it fires before the method handler runs) — a permission/support signal, not a user decision. These cases now correctly surface as `mmconnect_wallet_action_failed` with `failure_reason: wallet_unauthorized`. ## [0.13.0] diff --git a/packages/connect-multichain/src/multichain/utils/analytics.test.ts b/packages/connect-multichain/src/multichain/utils/analytics.test.ts index b59da3d4..f479558c 100644 --- a/packages/connect-multichain/src/multichain/utils/analytics.test.ts +++ b/packages/connect-multichain/src/multichain/utils/analytics.test.ts @@ -30,11 +30,18 @@ t.describe('isRejectionError', () => { t.expect(isRejectionError({ code: 4001, message: 'anything' })).toBe(true); }); - t.it('returns true for code 4100 (unauthorized)', () => { - t.expect(isRejectionError({ code: 4100, message: 'denied by user' })).toBe( - true, - ); - }); + t.it( + 'does NOT treat 4100 (unauthorized) as a rejection — that signals a permission/method-support issue, not user intent', + () => { + t.expect( + isRejectionError({ + code: 4100, + message: + 'The requested account and/or method has not been authorized by the user.', + }), + ).toBe(false); + }, + ); t.it('returns true for messages mentioning explicit user action', () => { t.expect(isRejectionError({ message: 'User rejected the request' })).toBe( @@ -125,6 +132,35 @@ t.describe('classifyFailureReason', () => { t.expect(classifyFailureReason(error)).toBe('wallet_internal_error'); }); + t.it( + 'classifies wallet 4100 unauthorized (e.g. CAIP-25 scope did not grant the method)', + () => { + const error = new RPCInvokeMethodErr( + 'inner', + 4100, + 'The requested account and/or method has not been authorized by the user.', + ); + t.expect(classifyFailureReason(error)).toBe('wallet_unauthorized'); + }, + ); + + t.it('classifies wallet 4200 unsupported method', () => { + const error = new RPCInvokeMethodErr('inner', 4200, 'Unsupported method'); + t.expect(classifyFailureReason(error)).toBe('wallet_method_unsupported'); + }); + + t.it( + 'classifies wallet 4902 unrecognised chain (mobile switchEthereumChain)', + () => { + const error = new RPCInvokeMethodErr( + 'inner', + 4902, + 'Unrecognized chain ID "0xfa". Try adding the chain using wallet_addEthereumChain first.', + ); + t.expect(classifyFailureReason(error)).toBe('unrecognised_chain'); + }, + ); + t.it('classifies provider-defined custom error code', () => { const error = new RPCInvokeMethodErr('inner', 4900, 'Disconnected'); t.expect(classifyFailureReason(error)).toBe('wallet_custom_error'); diff --git a/packages/connect-multichain/src/multichain/utils/analytics.ts b/packages/connect-multichain/src/multichain/utils/analytics.ts index ffd3e42c..7f13bfb9 100644 --- a/packages/connect-multichain/src/multichain/utils/analytics.ts +++ b/packages/connect-multichain/src/multichain/utils/analytics.ts @@ -33,6 +33,7 @@ export type FailureReason = | 'wallet_method_unsupported' | 'wallet_invalid_params' | 'wallet_internal_error' + | 'wallet_unauthorized' | 'wallet_custom_error' | 'session_expired' | 'no_active_session' @@ -91,9 +92,14 @@ export function isRejectionError(error: unknown): boolean { const { code, message } = getUnwrappedErrorDetails(error); const errorMessage = message.toLowerCase(); + // EIP-1193 `4001 User Rejected Request` is the canonical rejection code. + // Note: we deliberately do NOT match `4100 Unauthorized` here — that's + // returned for permission denials (e.g. CAIP-25 scope didn't include the + // requested method) and is not a user-driven rejection. Misclassifying it + // as a rejection hides genuine permission/method-support issues. See + // `classifyFailureReason`'s `wallet_unauthorized` bucket. return ( - code === 4001 || // User rejected request (common EIP-1193 code) - code === 4100 || // Unauthorized (common rejection code) + code === 4001 || errorMessage.includes('reject') || errorMessage.includes('denied') || errorMessage.includes('cancel') || @@ -184,6 +190,7 @@ export function classifyFailureReason(error: unknown): FailureReason { // `RPCInvokeMethodErr` so the wallet's actual error code is visible. const { code } = getUnwrappedErrorDetails(error); if (typeof code === 'number') { + // JSON-RPC 2.0 + EIP-1474 standard codes. if (code === -32601) { return 'wallet_method_unsupported'; } @@ -197,6 +204,25 @@ export function classifyFailureReason(error: unknown): FailureReason { if (code <= -32000 && code >= -32099) { return 'wallet_internal_error'; } + // EIP-1193 named provider codes — handled individually before the + // catch-all `wallet_custom_error` range below so we don't lose signal. + if (code === 4100) { + // Unauthorized — most commonly fires when a method isn't in the + // CAIP-25 scope's granted methods list (the multichain permission + // layer rejects it before the method handler runs). Distinct from + // a user rejection (4001) and worth tracking separately. + return 'wallet_unauthorized'; + } + if (code === 4200) { + // Unsupported method — wallet handler exists but explicitly refuses. + return 'wallet_method_unsupported'; + } + if (code === 4902) { + // Unrecognized chain ID — `wallet_switchEthereumChain` to a chain the + // wallet hasn't been told about. Same signal as the message heuristic + // above, but reaches us cleanly via code rather than substring. + return 'unrecognised_chain'; + } // Provider-defined error range (EIP-1193 + EIP-1474 custom codes). if (code >= 1000 && code <= 4999) { return 'wallet_custom_error'; diff --git a/playground/browser-playground/README.md b/playground/browser-playground/README.md index aa511353..b6f3d79d 100644 --- a/playground/browser-playground/README.md +++ b/playground/browser-playground/README.md @@ -73,6 +73,66 @@ Toggle between multichain and legacy EVM modes to test backwards compatibility w Test the wagmi integration for React applications with persistent sessions and multichain support. +## Manually testing analytics events + +The playground emits analytics events via `@metamask/analytics` to whatever endpoint `METAMASK_ANALYTICS_ENDPOINT` resolves to (defaults to the production sink). For local verification — confirming a new property landed, a classifier picked the right bucket, a new event fired at all — you can point the playground at a local echo server and use the in-page **Analytics test bench** section to drive each code path. + +### One-time setup + +The pieces below are already wired up in this repo; this is just so you know what's involved if anything looks off: + +- **`scripts/analytics-echo-server.mjs`** — a tiny Node HTTP server that accepts `POST /v2/events` and pretty-prints every event with `event_name`, `failure_reason`, `method`, and `transport` highlighted. +- **`craco.config.js`** — `DefinePlugin` is patched to forward `process.env.METAMASK_ANALYTICS_ENDPOINT` into the browser bundle. CRA's default behaviour only exposes `REACT_APP_*` vars, so without this the override would silently do nothing. +- **`public/index.html`** — the `Content-Security-Policy` meta tag's `connect-src` allowlist includes `http://localhost:*` and `http://127.0.0.1:*` so the browser is allowed to reach a local sink. Without this the browser drops the request and the Network tab shows nothing — the only hint is a CSP refusal in the console. +- **`src/components/AnalyticsTestBench.tsx`** — the collapsible "Analytics test bench" section in the playground UI, with one button per `classifyFailureReason` branch. + +### Step-by-step + +1. **Start the echo server** in one terminal: + + ```bash + yarn analytics:echo + # → analytics echo server listening on http://localhost:8787 + ``` + +2. **Start the playground** in another terminal with the endpoint override: + + ```bash + METAMASK_ANALYTICS_ENDPOINT="http://localhost:8787/" yarn start + ``` + + (You can also put `METAMASK_ANALYTICS_ENDPOINT=http://localhost:8787/` in your `.env` — it goes through the same `DefinePlugin` path.) + +3. **Open** `http://localhost:3000` (or whatever port CRA picked), connect via the Multichain card. + +4. **Expand "Analytics test bench"** at the top of the page. Each button drives a request shape designed to land in a specific `failure_reason` bucket on `mmconnect_wallet_action_failed`. Watch the echo-server terminal — events arrive within a couple hundred ms. + + The bench keeps an in-page "Recent triggers" log showing the raw `name` / `code` / `msg` the wallet returned, so you can cross-reference what the wallet sent vs which bucket the classifier picked. + +### Triggering buckets that need manual setup + +Two buckets aren't deterministically reachable from a button: + +- **`transport_timeout`** — toggle DevTools → Network → "Offline", then click any wallet-bound trigger, then wait ~30s for the SDK timeout. +- **`transport_disconnect`** — click a wallet-bound trigger, then disable/quit the MetaMask extension before approving. + +Both buttons in the bench just print these instructions in an alert. + +### Gotcha: multichain scope rules + +On a CAIP-25 multichain session, the wallet's permission layer rejects any method not in the granted scope with EIP-1193 `4100 Unauthorized` **before** the method handler runs ([source](https://github.com/MetaMask/core/blob/main/packages/multichain-api-middleware/src/handlers/wallet-invokeMethod.ts)). So buttons like "bogus method" and "switch chain to 0xfa" all land in `wallet_unauthorized`, even though they'd produce different codes (`-32601` / `4902`) if the wallet got to run its handlers. The bench labels reflect this — see the in-page "Heads up" note. + +### Verifying the endpoint override took effect + +If events aren't arriving at the echo server, the env var probably didn't make it into the bundle. Quick check: + +```bash +curl -s http://localhost:3000/static/js/bundle.js | grep -o 'localhost:8787[^"]*' | head -1 +# → localhost:8787/ +``` + +If that prints nothing, the bundle is still pointing at the production endpoint — restart `yarn start` with the env var set in the same shell. + ## Project Structure ``` @@ -80,6 +140,7 @@ browser-playground/ ├── src/ │ ├── App.tsx # Main application component │ ├── components/ +│ │ ├── AnalyticsTestBench.tsx # Collapsible bench for driving each failure_reason branch │ │ ├── DynamicInputs.tsx # Checkbox selection UI │ │ ├── FeaturedNetworks.tsx # Network selection component │ │ ├── LegacyEVMCard.tsx # Legacy EVM connector card @@ -95,6 +156,7 @@ browser-playground/ │ ├── config.ts # Wagmi configuration │ └── metamask-connector.ts # Auto-generated connector ├── scripts/ +│ ├── analytics-echo-server.mjs # Local POST /v2/events sink for manual analytics testing │ ├── copy-wagmi-connector.js # Copies wagmi connector from integrations/ │ └── README.md # Script documentation └── public/ diff --git a/playground/browser-playground/craco.config.js b/playground/browser-playground/craco.config.js index d5d524e5..2c461762 100644 --- a/playground/browser-playground/craco.config.js +++ b/playground/browser-playground/craco.config.js @@ -53,6 +53,9 @@ module.exports = { 'process.env.INFURA_API_KEY': JSON.stringify( process.env.INFURA_API_KEY, ), + 'process.env.METAMASK_ANALYTICS_ENDPOINT': JSON.stringify( + process.env.METAMASK_ANALYTICS_ENDPOINT, + ), }), ); diff --git a/playground/browser-playground/package.json b/playground/browser-playground/package.json index e3baf9e0..c4d433d8 100644 --- a/playground/browser-playground/package.json +++ b/playground/browser-playground/package.json @@ -24,6 +24,7 @@ "build/" ], "scripts": { + "analytics:echo": "node scripts/analytics-echo-server.mjs", "build": "yarn copy-wagmi-connector && DISABLE_ESLINT_PLUGIN=true craco build", "build:docs": "typedoc", "changelog:format": "../../scripts/format-changelog.sh @metamask/browser-playground", diff --git a/playground/browser-playground/public/index.html b/playground/browser-playground/public/index.html index c72cabc1..cec55f64 100644 --- a/playground/browser-playground/public/index.html +++ b/playground/browser-playground/public/index.html @@ -14,7 +14,7 @@ script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; - connect-src 'self' wss://mm-sdk-relay.api.cx.metamask.io https://mm-sdk-analytics.api.cx.metamask.io https://*.infura.io; + connect-src 'self' wss://mm-sdk-relay.api.cx.metamask.io https://mm-sdk-analytics.api.cx.metamask.io https://*.infura.io http://localhost:* http://127.0.0.1:*; frame-src https://fwd.metamask.io; font-src 'self';" /> diff --git a/playground/browser-playground/scripts/README.md b/playground/browser-playground/scripts/README.md index fa5578c6..da5f4166 100644 --- a/playground/browser-playground/scripts/README.md +++ b/playground/browser-playground/scripts/README.md @@ -1,5 +1,31 @@ # Browser Playground Scripts +## analytics-echo-server.mjs + +### Purpose + +A tiny local HTTP server (`POST /v2/events`, default port `8787`) that stands in for `https://mm-sdk-analytics.api.cx.metamask.io` during manual testing. Pretty-prints every event it receives, with `event_name`, `failure_reason`, `method`, and `transport` highlighted, so you can verify the playground is emitting the analytics events you expect. + +Pair it with the **Analytics test bench** section in the playground UI (`src/components/AnalyticsTestBench.tsx`) — see [the playground README](../README.md#manually-testing-analytics-events) for the full walkthrough. + +### Usage + +```bash +yarn analytics:echo # listens on :8787 +PORT=9090 yarn analytics:echo # custom port +``` + +Then start the playground with the analytics endpoint pointed at it: + +```bash +METAMASK_ANALYTICS_ENDPOINT="http://localhost:8787/" yarn start +``` + +### Important Notes + +- The env var injection is wired through `craco.config.js`'s `DefinePlugin`; CRA only exposes `REACT_APP_*` vars to the bundle by default, so a plain `process.env.METAMASK_ANALYTICS_ENDPOINT` would be `undefined` at runtime without that patch. +- `public/index.html`'s `Content-Security-Policy` meta tag includes `http://localhost:*` and `http://127.0.0.1:*` in `connect-src` so local sinks are reachable — without this the browser silently blocks the request and the Network tab shows nothing. + ## copy-wagmi-connector.js ### Purpose diff --git a/playground/browser-playground/scripts/analytics-echo-server.mjs b/playground/browser-playground/scripts/analytics-echo-server.mjs new file mode 100644 index 00000000..2a9d6df2 --- /dev/null +++ b/playground/browser-playground/scripts/analytics-echo-server.mjs @@ -0,0 +1,130 @@ +#!/usr/bin/env node +/** + * Local analytics echo server for the browser playground. + * + * Stands in for `https://mm-sdk-analytics.api.cx.metamask.io` when you want + * to manually inspect the analytics events the playground produces. Accepts + * `POST /v2/events` from `@metamask/analytics`, pretty-prints each event + * with `event_name`, `failure_reason`, `method`, and `transport` highlighted, + * and replies `200 {}` so the SDK's Sender batch loop is happy. + * + * See `playground/browser-playground/README.md` → "Manually testing + * analytics events" for the full setup. + * + * Usage: + * yarn analytics:echo # listens on :8787 + * PORT=9090 yarn analytics:echo # custom port + * node scripts/analytics-echo-server.mjs # standalone, no yarn + */ + +import http from 'node:http'; + +const PORT = Number(process.env.PORT ?? 8787); + +const COLOR = { + reset: '\x1b[0m', + dim: '\x1b[2m', + bold: '\x1b[1m', + cyan: '\x1b[36m', + green: '\x1b[32m', + yellow: '\x1b[33m', + red: '\x1b[31m', + magenta: '\x1b[35m', + blue: '\x1b[34m', +}; + +const colorForEvent = (name) => { + if (!name) return COLOR.dim; + if (name.endsWith('_failed')) return COLOR.red; + if (name.endsWith('_rejected')) return COLOR.yellow; + if (name.endsWith('_succeeded') || name === 'mmconnect_connected') { + return COLOR.green; + } + if (name.endsWith('_requested')) return COLOR.cyan; + return COLOR.magenta; +}; + +let eventCounter = 0; + +const printEvent = (event) => { + eventCounter += 1; + const name = event?.event_name ?? ''; + const props = event?.properties ?? {}; + const failureReason = props.failure_reason; + const method = props.method; + const transport = props.transport_type; + + const head = + `${COLOR.dim}#${eventCounter}${COLOR.reset} ` + + `${colorForEvent(name)}${COLOR.bold}${name}${COLOR.reset}`; + + const tags = []; + if (failureReason) { + tags.push( + `${COLOR.bold}${COLOR.red}failure_reason=${failureReason}${COLOR.reset}`, + ); + } + if (method) tags.push(`${COLOR.cyan}method=${method}${COLOR.reset}`); + if (transport) tags.push(`${COLOR.blue}transport=${transport}${COLOR.reset}`); + + console.log(`\n${head}${tags.length ? ' ' + tags.join(' ') : ''}`); + console.log(`${COLOR.dim}${JSON.stringify(props, null, 2)}${COLOR.reset}`); +}; + +const server = http.createServer((req, res) => { + if (req.method === 'OPTIONS') { + res.writeHead(204, { + 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Methods': 'POST, OPTIONS', + 'Access-Control-Allow-Headers': 'Content-Type', + }); + res.end(); + return; + } + + if (req.method !== 'POST') { + res.writeHead(404, { 'Access-Control-Allow-Origin': '*' }); + res.end('not found'); + return; + } + + const chunks = []; + req.on('data', (chunk) => chunks.push(chunk)); + req.on('end', () => { + const raw = Buffer.concat(chunks).toString('utf8'); + let parsed; + try { + parsed = JSON.parse(raw); + } catch (err) { + console.error(`\n${COLOR.red}Failed to parse body${COLOR.reset}`, err); + console.error(raw); + res.writeHead(200, { 'Access-Control-Allow-Origin': '*' }); + res.end('{}'); + return; + } + + const events = Array.isArray(parsed) ? parsed : [parsed]; + console.log( + `\n${COLOR.dim}━━━ ${new Date().toISOString()} POST ${req.url} (${events.length} event${events.length === 1 ? '' : 's'}) ━━━${COLOR.reset}`, + ); + for (const ev of events) printEvent(ev); + + res.writeHead(200, { + 'Content-Type': 'application/json', + 'Access-Control-Allow-Origin': '*', + }); + res.end('{}'); + }); +}); + +server.listen(PORT, () => { + console.log( + `${COLOR.bold}${COLOR.green}analytics echo server listening on http://localhost:${PORT}${COLOR.reset}`, + ); + console.log( + `${COLOR.dim} expecting POST /v2/events from @metamask/analytics${COLOR.reset}`, + ); + console.log( + `${COLOR.dim} start playground with: METAMASK_ANALYTICS_ENDPOINT=http://localhost:${PORT}/ yarn start${COLOR.reset}\n`, + ); +}); diff --git a/playground/browser-playground/src/App.tsx b/playground/browser-playground/src/App.tsx index 759f2919..b2f2b399 100644 --- a/playground/browser-playground/src/App.tsx +++ b/playground/browser-playground/src/App.tsx @@ -15,6 +15,7 @@ import { ScopeCard } from './components/ScopeCard'; import { LegacyEVMCard } from './components/LegacyEVMCard'; import { WagmiCard } from './components/WagmiCard'; import { SolanaWalletCard } from './components/SolanaWalletCard'; +import { AnalyticsTestBench } from './components/AnalyticsTestBench'; import { useSolanaSDK } from './sdk/SolanaProvider'; import { Buffer } from 'buffer'; @@ -403,6 +404,12 @@ function App() { )} + +
{ + entryId += 1; + return entryId; +}; + +export function AnalyticsTestBench({ + connectedScopes, +}: { + connectedScopes: Scope[]; +}) { + const { invokeMethod, session } = useSDK(); + const [results, setResults] = useState([]); + + // Pick the first connected EVM scope; fall back to mainnet so the buttons + // still render. Each button can override this if it needs a specific scope + // (e.g. unrecognised_chain wants a chain ID the wallet won't know). + const defaultScope: Scope = + (connectedScopes.find((s) => s.startsWith('eip155:')) as Scope) ?? + ('eip155:1' as Scope); + + // Grab the first EVM account from the session for triggers that need a + // real signer (e.g. `personal_sign`). If we hand the wallet the zero + // address, it returns -32602 before showing any prompt — which defeats + // the rejection sanity-check entirely. + const firstEvmAddress = useMemo<`0x${string}` | undefined>(() => { + const scopes = session?.sessionScopes ?? {}; + for (const [scope, value] of Object.entries(scopes)) { + if (!scope.startsWith('eip155:') && scope !== 'wallet') continue; + const accounts = (value as { accounts?: string[] })?.accounts ?? []; + for (const caipAccount of accounts) { + // `eip155:1:0xabc...` → take the trailing address portion + const addr = caipAccount.split(':').pop(); + if (addr?.startsWith('0x')) return addr as `0x${string}`; + } + } + return undefined; + }, [session]); + + const runTrigger = useCallback( + async ( + label: string, + expected: ExpectedBucket | string, + trigger: () => Promise, + ) => { + const id = nextId(); + setResults((prev) => [ + { id, label, expected, status: 'pending' }, + ...prev, + ]); + try { + await trigger(); + setResults((prev) => + prev.map((r): ResultEntry => + r.id === id ? { ...r, status: 'no-throw' } : r, + ), + ); + } catch (error) { + const e = error as { name?: string; message?: string; code?: unknown }; + const errorPatch: Partial = { status: 'threw' }; + if (e.name !== undefined) errorPatch.errorName = e.name; + if (e.message !== undefined) errorPatch.errorMessage = e.message; + if (typeof e.code === 'number' || typeof e.code === 'string') { + errorPatch.errorCode = e.code; + } + setResults((prev) => + prev.map((r): ResultEntry => + r.id === id ? { ...r, ...errorPatch } : r, + ), + ); + // Keep console verbose for cross-referencing with the echo server. + console.warn(`[analytics bench] "${label}":`, error); + } + }, + [], + ); + + /* -------------------------------------------------------------------- */ + /* Trigger definitions. Each one targets a specific classifier branch. */ + /* -------------------------------------------------------------------- */ + + const triggerBogusMethod = () => + runTrigger( + 'wallet_unauthorized (method not in scope)', + 'wallet_unauthorized', + () => + // Bogus method name — in a CAIP-25 multichain session, the wallet's + // permission layer rejects this with `4100 Unauthorized` BEFORE any + // method handler runs (see `wallet-invokeMethod.ts` in @metamask/core). + // So the practical "method unsupported" signal on multichain is 4100, + // not -32601. + invokeMethod({ + scope: defaultScope, + request: { + method: 'foo_bar_doesnt_exist', + params: [], + }, + }), + ); + + const triggerInvalidParams = () => + runTrigger( + 'wallet_invalid_params (−32602)', + 'wallet_invalid_params', + () => + invokeMethod({ + scope: defaultScope, + request: { + method: 'eth_sendTransaction', + params: [ + { + to: 'not-a-hex-address', + value: 'not-hex-either', + } as any, + ], + }, + }), + ); + + const triggerSwitchUnknownChain = () => + runTrigger( + 'wallet_unauthorized (switchEthereumChain not in scope)', + 'wallet_unauthorized', + () => + // CAIP-25 scopes don't typically include wallet_switchEthereumChain + // in the granted methods, so this hits the same 4100 path as the + // bogus method above. To actually reach `4902 unrecognised_chain` + // (or `-32603 wallet_internal_error`), the wallet would need to grant + // the method and then run its handler — not generally reachable from + // the playground. + invokeMethod({ + scope: defaultScope, + request: { + method: 'wallet_switchEthereumChain', + params: [{ chainId: '0xdeadbe' }], + }, + }), + ); + + const triggerSwitchFantom = () => + runTrigger( + 'wallet_unauthorized (switchEthereumChain not in scope)', + 'wallet_unauthorized', + () => + // Same caveat as above — on a typical multichain session this lands + // in 4100 unauthorized, not 4902 unrecognised_chain. Kept as a button + // so we can see if any wallet build behaves differently. + invokeMethod({ + scope: defaultScope, + request: { + method: 'wallet_switchEthereumChain', + params: [{ chainId: '0xfa' }], // Fantom — usually unknown + }, + }), + ); + + const triggerSignTypedDataMalformed = () => + runTrigger( + 'wallet_invalid_params (malformed signTypedData)', + 'wallet_invalid_params', + () => + // Wallets typically validate the typed-data payload before showing + // the confirmation UI; bad input produces -32602 or -32603. The + // exact code varies between builds. + invokeMethod({ + scope: defaultScope, + request: { + method: 'eth_signTypedData_v4', + params: ['0x0000000000000000000000000000000000000000', '{}'], + }, + }), + ); + + const triggerNoActiveSession = () => + runTrigger( + 'no_active_session (SDK sentinel)', + 'no_active_session', + () => + // Only meaningful if you call it BEFORE connecting. The button is + // shown unconditionally — if you're connected it will throw a + // different error and land in `unknown`. + invokeMethod({ + scope: defaultScope, + request: { + method: 'personal_sign', + params: ['0x68656c6c6f', '0x0000000000000000000000000000000000000000'], + }, + }), + ); + + const triggerUnknown = () => + runTrigger( + 'unknown (fallback) — empty method', + 'unknown', + () => + invokeMethod({ + scope: defaultScope, + request: { + method: '', + params: [], + }, + }), + ); + + const triggerRejection = () => { + if (!firstEvmAddress) { + alert( + 'No EVM account in the current session — connect one first so personal_sign has a real signer to prompt for.', + ); + return; + } + return runTrigger( + '(rejection — should fire _rejected, NOT _failed)', + 'n/a — _rejected', + () => + // personal_sign params: [message_hex, signer_address]. Must be a real + // connected account or the wallet rejects with -32602 *before* it + // ever shows the prompt — which would defeat the sanity-check. + invokeMethod({ + scope: defaultScope, + request: { + method: 'personal_sign', + params: ['0x68656c6c6f', firstEvmAddress], + }, + }), + ); + }; + + /* -------------------------------------------------------------------- */ + /* Buttons that need manual repro */ + /* -------------------------------------------------------------------- */ + const triggerTransportTimeout = () => { + alert( + [ + 'transport_timeout requires a stall longer than the transport timeout.', + '', + 'Easiest repro:', + ' 1. Open DevTools → Network → set throttling to "Offline".', + ' 2. Click any wallet-bound trigger above (e.g. "wallet_invalid_params").', + ' 3. Wait ~30s for the SDK timeout to fire.', + '', + 'You should see mmconnect_wallet_action_failed with failure_reason=transport_timeout.', + ].join('\n'), + ); + }; + + const triggerTransportDisconnect = () => { + alert( + [ + 'transport_disconnect requires the wallet to drop the connection mid-request.', + '', + 'Easiest repro:', + ' 1. Click any wallet-bound trigger above.', + ' 2. Immediately disable or quit the MetaMask extension before approving.', + '', + 'You should see mmconnect_wallet_action_failed with failure_reason=transport_disconnect.', + ].join('\n'), + ); + }; + + const buttonClass = + 'text-left bg-slate-800 hover:bg-slate-700 text-white px-3 py-2 rounded text-sm transition-colors'; + const manualButtonClass = + 'text-left bg-amber-700 hover:bg-amber-600 text-white px-3 py-2 rounded text-sm transition-colors'; + + return ( +
+
+ +
+

+ Analytics test bench +

+

+ Drive each failure_reason classifier branch + from the dapp. Pair with the local analytics echo server (see + playground README). +

+
+ + ▶ + + + +
+

+ Each button drives a request shape designed to land in a specific{' '} + failure_reason{' '} + bucket on{' '} + + mmconnect_wallet_action_failed + + . Watch the analytics echo server terminal to see the tagged event. + Connect first — most of these need an active session. +

+

+ Heads up: on multichain (CAIP-25) sessions, the wallet + rejects any method not in the granted scope with{' '} + 4100 Unauthorized{' '} + BEFORE the method handler runs. That means buttons like "bogus + method" and "switch chain to 0xfa" both reach the same code path — + they all land in{' '} + wallet_unauthorized{' '} + because the wallet never gets to see the actual method. To reach{' '} + + wallet_method_unsupported + {' '} + (real -32601) or unrecognised_chain (real{' '} + 4902), the method has to be in scope first — that's a + separate experiment. +

+ +
+ + + + + + + + + + +
+ +
+ Not currently tracked (classifier supports the + buckets but no producer emits them):{' '} + rpc_node_http_error + ,{' '} + rpc_node_request_error + ,{' '} + rpc_node_response_error + . These come from handleWithRpcNode which still throws + without calling analytics.track. +
+ + {results.length > 0 && ( +
+

+ Recent triggers +

+
+ {results.map((r) => ( +
+
+ {r.label} + + expected: {r.expected} + +
+ {r.status === 'pending' && ( +
…waiting
+ )} + {r.status === 'no-throw' && ( +
+ no error thrown — _failed event will NOT have fired +
+ )} + {r.status === 'threw' && ( +
+ {r.errorName && ( + + name= + {r.errorName} + + )} + {r.errorCode !== undefined && ( + + code= + {String(r.errorCode)} + + )} + {r.errorMessage && ( + + msg= + {r.errorMessage} + + )} +
+ )} +
+ ))} +
+
+ )} +
+
+
+ ); +} From 08bebf9429cfafef5586cd8d5572c84d60ca2255 Mon Sep 17 00:00:00 2001 From: Alex Donesky Date: Mon, 11 May 2026 13:57:38 -0500 Subject: [PATCH 03/10] refactor(analytics): drop wallet_custom_error bucket, fold into unknown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `wallet_custom_error` was the catch-all for any EIP-1193 / EIP-1474 provider-defined code in the 1000-4999 range we hadn't carved out into its own bucket. With named branches now in place for 4001 (handled via `isRejectionError`), 4100, 4200, and 4902, the remaining "we don't know" cases are conceptually identical to `unknown` — having two buckets for the same signal adds noise without insight. After this: - Unrecognised provider codes (e.g. 4900 Disconnected) fall through to `unknown` instead of `wallet_custom_error`. - The catch-all `1000 <= code <= 4999` branch is removed. - Future codes can be promoted from `unknown` into named buckets by adding new branches above the fallback — no schema migration needed since `failure_reason` is still an open string. Updated: - Test for code 4900 now asserts `unknown` instead of `wallet_custom_error`. - Playground `ExpectedBucket` union drops the entry. - CHANGELOG entry reworded to reflect the tighter taxonomy. --- packages/connect-multichain/CHANGELOG.md | 2 +- .../src/multichain/utils/analytics.test.ts | 14 ++++++++++---- .../src/multichain/utils/analytics.ts | 14 +++++++------- .../src/components/AnalyticsTestBench.tsx | 1 - 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/packages/connect-multichain/CHANGELOG.md b/packages/connect-multichain/CHANGELOG.md index 76501968..39f8b4ef 100644 --- a/packages/connect-multichain/CHANGELOG.md +++ b/packages/connect-multichain/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Attach a `failure_reason` property to `mmconnect_wallet_action_failed` and `mmconnect_connection_failed` analytics events. The new `classifyFailureReason` helper tags transport timeouts, transport disconnects, wallet JSON-RPC errors (method unsupported / invalid params / internal), provider custom errors (`4100 wallet_unauthorized`, `4200 wallet_method_unsupported`, `4902 unrecognised_chain`, generic `wallet_custom_error`), "no active session", "unrecognised chain", and read-only RPC node failures (HTTP / request / response), with an `unknown` fallback. The schema-side change lives in [`metamask-sdk-analytics-api#31`](https://github.com/consensys-vertical-apps/metamask-sdk-analytics-api/pull/31). +- Attach a `failure_reason` property to `mmconnect_wallet_action_failed` and `mmconnect_connection_failed` analytics events. The new `classifyFailureReason` helper tags transport timeouts, transport disconnects, the SDK's `no_active_session` sentinel, EIP-1193 wallet errors (`4100 wallet_unauthorized`, `4200 wallet_method_unsupported`, `4902 unrecognised_chain`), JSON-RPC wallet errors (`-32601 wallet_method_unsupported`, `-32602 wallet_invalid_params`, `-32603 wallet_internal_error`, plus the `-32000…-32099` server-error range), and read-only RPC node failures (HTTP / request / response), with an `unknown` fallback for anything else. The schema-side change lives in [`metamask-sdk-analytics-api#31`](https://github.com/consensys-vertical-apps/metamask-sdk-analytics-api/pull/31). ### Fixed diff --git a/packages/connect-multichain/src/multichain/utils/analytics.test.ts b/packages/connect-multichain/src/multichain/utils/analytics.test.ts index f479558c..0c0fc860 100644 --- a/packages/connect-multichain/src/multichain/utils/analytics.test.ts +++ b/packages/connect-multichain/src/multichain/utils/analytics.test.ts @@ -161,10 +161,16 @@ t.describe('classifyFailureReason', () => { }, ); - t.it('classifies provider-defined custom error code', () => { - const error = new RPCInvokeMethodErr('inner', 4900, 'Disconnected'); - t.expect(classifyFailureReason(error)).toBe('wallet_custom_error'); - }); + t.it( + 'falls back to "unknown" for unrecognised provider-defined codes (no wallet_custom_error bucket)', + () => { + // 4900 "Disconnected" — real EIP-1193 code, but we don't surface it + // separately today. Lives in `unknown` until/unless usage justifies + // its own bucket (this is the policy described in the source comment). + const error = new RPCInvokeMethodErr('inner', 4900, 'Disconnected'); + t.expect(classifyFailureReason(error)).toBe('unknown'); + }, + ); t.it('classifies read-only RPC HTTP errors', () => { t.expect( diff --git a/packages/connect-multichain/src/multichain/utils/analytics.ts b/packages/connect-multichain/src/multichain/utils/analytics.ts index 7f13bfb9..6a2dc285 100644 --- a/packages/connect-multichain/src/multichain/utils/analytics.ts +++ b/packages/connect-multichain/src/multichain/utils/analytics.ts @@ -34,7 +34,6 @@ export type FailureReason = | 'wallet_invalid_params' | 'wallet_internal_error' | 'wallet_unauthorized' - | 'wallet_custom_error' | 'session_expired' | 'no_active_session' | 'unrecognised_chain' @@ -204,8 +203,8 @@ export function classifyFailureReason(error: unknown): FailureReason { if (code <= -32000 && code >= -32099) { return 'wallet_internal_error'; } - // EIP-1193 named provider codes — handled individually before the - // catch-all `wallet_custom_error` range below so we don't lose signal. + // EIP-1193 named provider codes — handled individually. Codes in the + // 1000–4999 range that aren't matched here fall through to `unknown`. if (code === 4100) { // Unauthorized — most commonly fires when a method isn't in the // CAIP-25 scope's granted methods list (the multichain permission @@ -223,10 +222,11 @@ export function classifyFailureReason(error: unknown): FailureReason { // above, but reaches us cleanly via code rather than substring. return 'unrecognised_chain'; } - // Provider-defined error range (EIP-1193 + EIP-1474 custom codes). - if (code >= 1000 && code <= 4999) { - return 'wallet_custom_error'; - } + // Anything else in the EIP-1193 / EIP-1474 provider-defined range + // (1000–4999) falls through to `unknown` — we can promote specific codes + // into their own buckets later as the distribution stabilises, without a + // schema migration. Two buckets for "we don't know what this is" adds + // noise without insight. } return 'unknown'; diff --git a/playground/browser-playground/src/components/AnalyticsTestBench.tsx b/playground/browser-playground/src/components/AnalyticsTestBench.tsx index 7aa5e952..b98a8827 100644 --- a/playground/browser-playground/src/components/AnalyticsTestBench.tsx +++ b/playground/browser-playground/src/components/AnalyticsTestBench.tsx @@ -26,7 +26,6 @@ type ExpectedBucket = | 'wallet_invalid_params' | 'wallet_internal_error' | 'wallet_unauthorized' - | 'wallet_custom_error' | 'no_active_session' | 'unrecognised_chain' | 'unknown'; From b7b9df2318a1110320b1937cae632dfa62bf9a36 Mon Sep 17 00:00:00 2001 From: Alex Donesky Date: Mon, 11 May 2026 14:04:21 -0500 Subject: [PATCH 04/10] refactor(analytics): drop unreachable no_active_session bucket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `no_active_session` classifier branch matched the SDK-thrown sentinel string `'No active session found'`, raised by `RequestRouter` and `Multichain` when `getActiveSession()` returns `null`. Both occurrences live inside `setTimeout` callbacks in the deeplink-open path: setTimeout(async () => { const session = await this.transport.getActiveSession(); if (!session) { throw new Error('No active session found'); // unhandled } ... }, 10); That throw is unhandled — it doesn't reach `#withAnalyticsTracking`'s try/catch and never produces a `mmconnect_wallet_action_failed` event. The classifier branch was dead from day one. The real pre-connect signal — `invokeMethod` called before `Transport not initialized, establish connection first` — is thrown from the `transport` getter, *outside* the `RequestRouter`'s analytics wrapper, so it's also never tracked. Wiring up pre-flight tracking is deliberately out of scope for this PR (see "Out of scope" in the PR body). Drop the bucket from the `FailureReason` union, the classifier branch, the test, the CHANGELOG entry, and the corresponding button in the playground analytics test bench. Pre-flight failures will still be diagnosable when we wire them up; we'll add the bucket back at the same time. Keeping it now just risks reviewers (or future us) trying to trigger it and finding it doesn't fire. --- packages/connect-multichain/CHANGELOG.md | 2 +- .../src/multichain/utils/analytics.test.ts | 6 ----- .../src/multichain/utils/analytics.ts | 6 ----- .../src/components/AnalyticsTestBench.tsx | 25 ------------------- 4 files changed, 1 insertion(+), 38 deletions(-) diff --git a/packages/connect-multichain/CHANGELOG.md b/packages/connect-multichain/CHANGELOG.md index 39f8b4ef..940efa25 100644 --- a/packages/connect-multichain/CHANGELOG.md +++ b/packages/connect-multichain/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Attach a `failure_reason` property to `mmconnect_wallet_action_failed` and `mmconnect_connection_failed` analytics events. The new `classifyFailureReason` helper tags transport timeouts, transport disconnects, the SDK's `no_active_session` sentinel, EIP-1193 wallet errors (`4100 wallet_unauthorized`, `4200 wallet_method_unsupported`, `4902 unrecognised_chain`), JSON-RPC wallet errors (`-32601 wallet_method_unsupported`, `-32602 wallet_invalid_params`, `-32603 wallet_internal_error`, plus the `-32000…-32099` server-error range), and read-only RPC node failures (HTTP / request / response), with an `unknown` fallback for anything else. The schema-side change lives in [`metamask-sdk-analytics-api#31`](https://github.com/consensys-vertical-apps/metamask-sdk-analytics-api/pull/31). +- Attach a `failure_reason` property to `mmconnect_wallet_action_failed` and `mmconnect_connection_failed` analytics events. The new `classifyFailureReason` helper tags transport timeouts, transport disconnects, EIP-1193 wallet errors (`4100 wallet_unauthorized`, `4200 wallet_method_unsupported`, `4902 unrecognised_chain`), JSON-RPC wallet errors (`-32601 wallet_method_unsupported`, `-32602 wallet_invalid_params`, `-32603 wallet_internal_error`, plus the `-32000…-32099` server-error range), and read-only RPC node failures (HTTP / request / response), with an `unknown` fallback for anything else. The schema-side change lives in [`metamask-sdk-analytics-api#31`](https://github.com/consensys-vertical-apps/metamask-sdk-analytics-api/pull/31). ### Fixed diff --git a/packages/connect-multichain/src/multichain/utils/analytics.test.ts b/packages/connect-multichain/src/multichain/utils/analytics.test.ts index 0c0fc860..a426837a 100644 --- a/packages/connect-multichain/src/multichain/utils/analytics.test.ts +++ b/packages/connect-multichain/src/multichain/utils/analytics.test.ts @@ -105,12 +105,6 @@ t.describe('classifyFailureReason', () => { t.expect(classifyFailureReason(error)).toBe('transport_disconnect'); }); - t.it('classifies the SDK "No active session found" sentinel', () => { - t.expect(classifyFailureReason(new Error('No active session found'))).toBe( - 'no_active_session', - ); - }); - t.it('classifies the "Unrecognized chain" message', () => { t.expect( classifyFailureReason(new Error('Unrecognized chain ID "0xfa"')), diff --git a/packages/connect-multichain/src/multichain/utils/analytics.ts b/packages/connect-multichain/src/multichain/utils/analytics.ts index 6a2dc285..7bf1eaf1 100644 --- a/packages/connect-multichain/src/multichain/utils/analytics.ts +++ b/packages/connect-multichain/src/multichain/utils/analytics.ts @@ -35,7 +35,6 @@ export type FailureReason = | 'wallet_internal_error' | 'wallet_unauthorized' | 'session_expired' - | 'no_active_session' | 'unrecognised_chain' | 'rpc_node_http_error' | 'rpc_node_request_error' @@ -172,11 +171,6 @@ export function classifyFailureReason(error: unknown): FailureReason { return 'transport_disconnect'; } - // SDK-thrown sentinel. - if (errorMessageRaw === 'No active session found') { - return 'no_active_session'; - } - if (errorMessage.includes('unrecognized chain')) { return 'unrecognised_chain'; } diff --git a/playground/browser-playground/src/components/AnalyticsTestBench.tsx b/playground/browser-playground/src/components/AnalyticsTestBench.tsx index b98a8827..4cb173d3 100644 --- a/playground/browser-playground/src/components/AnalyticsTestBench.tsx +++ b/playground/browser-playground/src/components/AnalyticsTestBench.tsx @@ -26,7 +26,6 @@ type ExpectedBucket = | 'wallet_invalid_params' | 'wallet_internal_error' | 'wallet_unauthorized' - | 'no_active_session' | 'unrecognised_chain' | 'unknown'; @@ -213,23 +212,6 @@ export function AnalyticsTestBench({ }), ); - const triggerNoActiveSession = () => - runTrigger( - 'no_active_session (SDK sentinel)', - 'no_active_session', - () => - // Only meaningful if you call it BEFORE connecting. The button is - // shown unconditionally — if you're connected it will throw a - // different error and land in `unknown`. - invokeMethod({ - scope: defaultScope, - request: { - method: 'personal_sign', - params: ['0x68656c6c6f', '0x0000000000000000000000000000000000000000'], - }, - }), - ); - const triggerUnknown = () => runTrigger( 'unknown (fallback) — empty method', @@ -391,13 +373,6 @@ export function AnalyticsTestBench({ > wallet_invalid_params (signTypedData on bad address) - -
- Not currently tracked (classifier supports the - buckets but no producer emits them):{' '} - rpc_node_http_error - ,{' '} - rpc_node_request_error - ,{' '} - rpc_node_response_error - . These come from handleWithRpcNode which still throws - without calling analytics.track. -
- {results.length > 0 && (

From 4b6bdf8e5075b65b7c40b426f3126869a6377f8e Mon Sep 17 00:00:00 2001 From: Alex Donesky Date: Mon, 11 May 2026 16:41:37 -0500 Subject: [PATCH 10/10] fix(connect-evm): suppress switchChain `_failed` event on successful addEthereumChain recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `wallet_switchEthereumChain` throws "Unrecognized chain ID" and the caller supplied a `chainConfiguration`, the shim catches the error and transparently calls `#addEthereumChain` — from the dapp/user's perspective this is still one logical chain change with one outcome. The previous behaviour fired `mmconnect_wallet_action_failed` for the swallowed switch attempt before the recovery ran, producing four Mixpanel events (`switch _requested`, `switch _failed`, `add _requested`, `add _succeeded`) for what should be three. Hoist the recovery-eligibility check above the `_failed` track call so the event only fires when there's no recovery to attempt: - No `chainConfiguration` provided → switch `_failed` still fires (caller sees the rejection too). - Error is not "Unrecognized chain ID" → switch `_failed`/`_rejected` still fires. - Error is "Unrecognized chain ID" + `chainConfiguration` is set → fall through to `#addEthereumChain`, whose own `_succeeded` / `_failed` represents the whole flow. Net effect: exactly one `_failed` per logical chain change, regardless of which RPC method the wallet rejected. Tests: - New `analytics.track` spy on the switchChain test block (the existing tests only asserted network calls, not analytics surface). - Asserts the suppression on the recovery-success path (3 events, not 4). - Asserts `_failed` still fires when no chainConfiguration is provided. - Asserts `_failed`/`_rejected` still fires for non-"Unrecognized chain" errors with chainConfiguration. - New test: when recovery itself fails, suppression of the switch event is preserved and only the add's `_failed` fires (1 `_failed`, not 2). Mock core extended with `options.dapp`, `versions`, `transportType`, and `storage.getAnonId()` so `getWalletActionAnalyticsProperties` can run end-to-end instead of being swallowed by `#trackWalletAction*`'s defensive try/catch. Related: WAPI-1253. The closed #291 conflated this fix with a non-existent shim-vs-router duplication; this PR addresses only the real intra-shim double-fire. --- packages/connect-evm/CHANGELOG.md | 4 + packages/connect-evm/src/connect.test.ts | 103 +++++++++++++++++++++-- packages/connect-evm/src/connect.ts | 11 ++- 3 files changed, 112 insertions(+), 6 deletions(-) diff --git a/packages/connect-evm/CHANGELOG.md b/packages/connect-evm/CHANGELOG.md index b46e043c..2282c7de 100644 --- a/packages/connect-evm/CHANGELOG.md +++ b/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 diff --git a/packages/connect-evm/src/connect.test.ts b/packages/connect-evm/src/connect.test.ts index eb80cff2..dd797010 100644 --- a/packages/connect-evm/src/connect.test.ts +++ b/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>; + let trackSpy: ReturnType; 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: {} }], }); diff --git a/packages/connect-evm/src/connect.ts b/packages/connect-evm/src/connect.ts index 77500eef..d45448d9 100644 --- a/packages/connect-evm/src/connect.ts +++ b/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; } }