fix: multichain sdk state + bug fixing (#1358)

* fix: better tracking state in sdk

* fix: better manage connection state and fix notifications on mwp

* fix: set default timeouts for browser transport

* fix: better handle rejections in mwp transport

* fix: remove over verbose logs from transports

Author: elribonazo

packages/sdk-multichain/src/connect.test.ts

@@ -115,6 +115,7 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
 			//Expect to find all the transport mocks DISCONNECTED
 			t.expect(mockedData.mockDefaultTransport.__isConnected).toBe(false);
 			t.expect(mockedData.mockDappClient.state).toBe('DISCONNECTED');
+			t.expect(sdk.state === 'disconnected').toBe(true);
 
 			mockedData.mockDefaultTransport.connect.mockClear();
 			(mockedData.mockDappClient as any).connect.mockClear();
@@ -265,6 +266,7 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
 			t.expect(() => sdk.transport).toThrow();
 
 			await t.expect(() => sdk.connect(scopes, caipAccountIds)).rejects.toThrow(sessionError);
+			t.expect(sdk.state === 'disconnected').toBe(true);
 		});
 
 		t.it(`${platform} should handle session revocation errors on session upgrade`, async () => {

packages/sdk-multichain/src/domain/logger/index.ts

@@ -6,7 +6,7 @@ import type { StoreClient } from '../store/client';
  * Supported debug namespace types for the MetaMask SDK logger.
  * These namespaces help categorize and filter debug output.
  */
-export type LoggerNameSpaces = 'metamask-sdk' | 'metamask-sdk:core' | 'metamask-sdk:provider' | 'metamask-sdk:ui';
+export type LoggerNameSpaces = 'metamask-sdk' | 'metamask-sdk:core' | 'metamask-sdk:provider' | 'metamask-sdk:ui' | 'metamask-sdk:transport';
 
 /**
  * Creates a debug logger instance with the specified namespace and color.

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

@@ -165,10 +165,8 @@ export class MultichainSDK extends MultichainCore {
 			if (!this.transport.isConnected()) {
 				this.state = 'connecting';
 				await this.transport.connect();
-				this.state = 'connected';
-			} else {
-				this.state = 'connected';
 			}
+			this.state = 'connected';
 		} else {
 			this.state = 'loaded';
 		}
@@ -187,6 +185,8 @@ export class MultichainSDK extends MultichainCore {
 				}
 			}
 		} catch (error) {
+			await this.storage.removeTransport();
+			this.state = 'pending';
 			logger('MetaMaskSDK error during initialization', error);
 		}
 	}
@@ -288,15 +288,18 @@ export class MultichainSDK extends MultichainCore {
 							.then(() => {
 								this.options.ui.factory.unload();
 								this.options.ui.factory.modal?.unmount();
+								this.state = 'connected';
 							})
 							.catch((err) => {
 								if (err instanceof ProtocolError) {
 									//Ignore Request expired errors to allow modal to regenerate expired qr codes
 									if (err.code !== ErrorCode.REQUEST_EXPIRED) {
+										this.state = 'disconnected';
 										reject(err);
 									}
 									// If request is expires, the QRCode will automatically be regenerated we can ignore this case
 								} else {
+									this.state = 'disconnected';
 									reject(err);
 								}
 							});
@@ -305,10 +308,8 @@ export class MultichainSDK extends MultichainCore {
 				async (error?: Error) => {
 					if (!error) {
 						await this.storage.setTransport(TransportType.MPW);
-						this.state = 'connected';
 						resolve();
 					} else {
-						this.state = 'disconnected';
 						await this.storage.removeTransport();
 						reject(error);
 					}
@@ -366,41 +367,37 @@ export class MultichainSDK extends MultichainCore {
 					this.openDeeplink(deeplink, universalLink);
 				}
 			});
-			this.state = 'connecting';
-			this.transport
-				.connect({ scopes, caipAccountIds })
-				.then(() => {
-					this.state = 'connected';
-					return resolve();
-				})
-				.catch(reject);
+			this.transport.connect({ scopes, caipAccountIds }).then(resolve).catch(reject);
 		});
 	}
 
+	private async handleConnection(promise: Promise<void>) {
+		this.state = 'connecting';
+		return promise
+			.then(() => {
+				this.state = 'connected';
+			})
+			.catch((err) => {
+				this.state = 'disconnected';
+				return Promise.reject(err);
+			});
+	}
+
 	async connect(scopes: Scope[], caipAccountIds: CaipAccountId[]): Promise<void> {
 		const { ui } = this.options;
 		const platformType = getPlatformType();
 		const isWeb = platformType === PlatformType.MetaMaskMobileWebview || platformType === PlatformType.DesktopWeb;
 		const { preferExtension = true, preferDesktop = false, headless: _headless = false } = ui;
 
 		if (this.__transport?.isConnected()) {
-			return this.__transport.connect({ scopes, caipAccountIds });
+			return this.handleConnection(this.__transport.connect({ scopes, caipAccountIds }));
 		}
 
 		if (isWeb && hasExtension() && preferExtension) {
 			//If metamask extension is available, connect to it
 			const defaultTransport = await this.setupDefaultTransport();
 			// Web transport has no initial payload
-			return defaultTransport
-				.connect()
-				.then(() => {
-					this.state = 'connected';
-					return Promise.resolve();
-				})
-				.catch((err) => {
-					this.state = 'loaded';
-					return Promise.reject(err);
-				});
+			return this.handleConnection(defaultTransport.connect({ scopes, caipAccountIds }));
 		}
 
 		// Connection will now be InstallModal + QRCodes or Deeplinks, both require mwp
@@ -417,11 +414,11 @@ export class MultichainSDK extends MultichainCore {
 		const secure = isSecure();
 		if (secure && !isDesktopPreferred) {
 			// Desktop is not preferred option, so we use deeplinks (mobile web)
-			return this.deeplinkConnect(scopes, caipAccountIds);
+			return this.handleConnection(this.deeplinkConnect(scopes, caipAccountIds));
 		}
 
 		// Show install modal for RN, Web + Node
-		return this.showInstallModal(isDesktopPreferred, scopes, caipAccountIds);
+		return this.handleConnection(this.showInstallModal(isDesktopPreferred, scopes, caipAccountIds));
 	}
 
 	public override emit(event: string, args: any): void {

packages/sdk-multichain/src/multichain/transports/default/index.ts

@@ -7,7 +7,6 @@ const DEFAULT_REQUEST_TIMEOUT = 60 * 1000;
 
 export class DefaultTransport implements ExtendedTransport {
 	#notificationCallbacks: Set<(data: unknown) => void> = new Set();
-	#requestId = 0;
 	#transport: Transport = getDefaultTransport();
 	#defaultRequestOptions = {
 		timeout: DEFAULT_REQUEST_TIMEOUT,
@@ -28,6 +27,9 @@ export class DefaultTransport implements ExtendedTransport {
 
 		//Get wallet session
 		const sessionRequest = await this.request({ method: 'wallet_getSession' }, this.#defaultRequestOptions);
+		if (sessionRequest.error) {
+			throw new Error(sessionRequest.error.message);
+		}
 		let walletSession = sessionRequest.result as SessionData;
 		if (walletSession && options) {
 			const currentScopes = Object.keys(walletSession?.sessionScopes ?? {}) as Scope[];
@@ -67,7 +69,7 @@ export class DefaultTransport implements ExtendedTransport {
 		return this.#transport.isConnected();
 	}
 
-	async request<TRequest extends TransportRequest, TResponse extends TransportResponse>(request: TRequest, options?: { timeout?: number }) {
+	async request<TRequest extends TransportRequest, TResponse extends TransportResponse>(request: TRequest, options: { timeout?: number } = this.#defaultRequestOptions) {
 		return this.#transport.request(request, options) as Promise<TResponse>;
 	}
 

packages/sdk-multichain/src/multichain/transports/mwp/index.ts

@@ -3,7 +3,7 @@ import type { Session, SessionRequest } from '@metamask/mobile-wallet-protocol-c
 import { SessionStore } from '@metamask/mobile-wallet-protocol-core';
 import type { DappClient } from '@metamask/mobile-wallet-protocol-dapp-client';
 
-import type { ExtendedTransport, RPCAPI, Scope, SessionData, StoreAdapter } from '../../../domain';
+import { createLogger, type ExtendedTransport, type RPCAPI, type Scope, type SessionData, type StoreAdapter } from '../../../domain';
 import type { CaipAccountId } from '@metamask/utils';
 import { addValidAccounts, getOptionalScopes, getValidAccounts } from '../../utils';
 
@@ -19,6 +19,8 @@ type PendingRequests = {
 	timeout: NodeJS.Timeout;
 };
 
+const logger = createLogger('metamask-sdk:transport');
+
 /**
  * Mobile Wallet Protocol transport implementation
  * Bridges the MWP DappClient with the multichain API client Transport interface
@@ -82,14 +84,18 @@ export class MWPTransport implements ExtendedTransport {
 							...messagePayload,
 							method: request.method === 'wallet_getSession' || request.method === 'wallet_createSession' ? 'wallet_sessionChanged' : request.method,
 						} as unknown as TransportResponse<unknown>;
+						const notification = {
+							method: request.method === 'wallet_getSession' || request.method === 'wallet_createSession' ? 'wallet_sessionChanged' : request.method,
+							params: requestWithName.result,
+						};
 						clearTimeout(request.timeout);
+						this.notifyCallbacks(notification);
 						request.resolve(requestWithName);
 						this.pendingRequests.delete(messagePayload.id);
 						return;
 					}
-				} else if ('name' in message && message.name === 'metamask-multichain-provider' && messagePayload.method === 'wallet_sessionChanged') {
+				} else {
 					this.notifyCallbacks(message.data);
-					return;
 				}
 			}
 		}
@@ -98,26 +104,40 @@ export class MWPTransport implements ExtendedTransport {
 	private async onResumeSuccess(resumeResolve: () => void, resumeReject: (err: Error) => void, options?: { scopes: Scope[]; caipAccountIds: CaipAccountId[] }): Promise<void> {
 		try {
 			const sessionRequest = await this.request({ method: 'wallet_getSession' });
+			if (sessionRequest.error) {
+				return resumeReject(new Error(sessionRequest.error.message));
+			}
 			let walletSession = sessionRequest.result as SessionData;
-			if (options) {
+			if (walletSession && options) {
 				const currentScopes = Object.keys(walletSession?.sessionScopes ?? {}) as Scope[];
 				const proposedScopes = options?.scopes ?? [];
 				const isSameScopes = currentScopes.every((scope) => proposedScopes.includes(scope)) && proposedScopes.every((scope) => currentScopes.includes(scope));
 				if (!isSameScopes) {
 					const optionalScopes = addValidAccounts(getOptionalScopes(options?.scopes ?? []), getValidAccounts(options?.caipAccountIds ?? []));
 					const sessionRequest: CreateSessionParams<RPCAPI> = { optionalScopes };
 					const response = await this.request({ method: 'wallet_createSession', params: sessionRequest });
+					if (response.error) {
+						return resumeReject(new Error(response.error.message));
+					}
 					await this.request({ method: 'wallet_revokeSession', params: walletSession });
 					walletSession = response.result as SessionData;
 				}
+			} else if (!walletSession) {
+				const optionalScopes = addValidAccounts(getOptionalScopes(options?.scopes ?? []), getValidAccounts(options?.caipAccountIds ?? []));
+				const sessionRequest: CreateSessionParams<RPCAPI> = { optionalScopes };
+				const response = await this.request({ method: 'wallet_createSession', params: sessionRequest });
+				if (response.error) {
+					return resumeReject(new Error(response.error.message));
+				}
+				walletSession = response.result as SessionData;
 			}
 			this.notifyCallbacks({
 				method: 'wallet_sessionChanged',
 				params: walletSession,
 			});
-			resumeResolve();
+			return resumeResolve();
 		} catch (err) {
-			resumeReject(err);
+			return resumeReject(err);
 		}
 	}
 
@@ -133,6 +153,7 @@ export class MWPTransport implements ExtendedTransport {
 		try {
 			const [activeSession] = await sessionStore.list();
 			if (activeSession) {
+				logger('active session found', activeSession);
 				session = activeSession;
 			}
 		} catch {}

packages/sdk-multichain/src/session.test.ts

@@ -209,7 +209,7 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
 
 			await t.expect(() => sdk.connect(scopes, caipAccountIds)).rejects.toThrow(sessionError);
 
-			t.expect(sdk.state === 'loaded').toBe(true);
+			t.expect(sdk.state === 'disconnected').toBe(true);
 		});
 	});
 }