From e9f63fd711856c6e0e302e86e6d32fab2fe2f0bf Mon Sep 17 00:00:00 2001
From: unickhow <unick.zhow@gmail.com>
Date: Tue, 9 Sep 2025 22:34:29 +0800
Subject: [PATCH] fix: enhance error handling in connectWithExtensionProvider
 function

---
 .../connectWithExtensionProvider.test.ts      | 82 ++++++++++++++++++-
 .../connectWithExtensionProvider.ts           | 19 ++++-
 2 files changed, 96 insertions(+), 5 deletions(-)

diff --git a/packages/sdk/src/services/MetaMaskSDK/ProviderManager/connectWithExtensionProvider.test.ts b/packages/sdk/src/services/MetaMaskSDK/ProviderManager/connectWithExtensionProvider.test.ts
index 43ca63620..6ad742d16 100644
--- a/packages/sdk/src/services/MetaMaskSDK/ProviderManager/connectWithExtensionProvider.test.ts
+++ b/packages/sdk/src/services/MetaMaskSDK/ProviderManager/connectWithExtensionProvider.test.ts
@@ -1,5 +1,6 @@
 import { EventType, TrackingEvents } from '@metamask/sdk-communication-layer';
 import { MetaMaskSDK } from '../../../sdk';
+import { MetaMaskSDKEvent } from '../../../types/MetaMaskSDKEvents';
 import { PROVIDER_UPDATE_TYPE } from '../../../types/ProviderUpdateType';
 import { STORAGE_PROVIDER_TYPE } from '../../../config';
 import * as loggerModule from '../../../utils/logger';
@@ -68,16 +69,39 @@ describe('connectWithExtensionProvider', () => {
   });
 
   it('should handle error during account request', async () => {
-    mockRequest.mockRejectedValue(new Error('Some error'));
+    const error = new Error('User rejected the request');
+    mockRequest.mockRejectedValue(error);
 
     const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
+    const originalProvider = instance.activeProvider;
+    instance.sdkProvider = originalProvider;
 
-    await connectWithExtensionProvider(instance);
+    await expect(connectWithExtensionProvider(instance)).rejects.toThrow(
+      'User rejected the request',
+    );
 
+    // Should log the error
     expect(consoleWarnSpy).toHaveBeenCalledWith(
       `[MetaMaskSDK: connectWithExtensionProvider()] can't request accounts error`,
-      new Error('Some error'),
+      error,
+    );
+
+    // Should emit ConnectWithResponse event with error
+    expect(instance.emit).toHaveBeenCalledWith(
+      MetaMaskSDKEvent.ConnectWithResponse,
+      { error },
     );
+
+    // Should send analytics event for rejection
+    expect(mockSendAnalyticsEvent).toHaveBeenCalledWith({
+      event: TrackingEvents.REJECTED,
+    });
+
+    // Should restore original provider
+    expect(instance.activeProvider).toBe(originalProvider);
+    expect((global.window as any).ethereum).toBe(originalProvider);
+
+    consoleWarnSpy.mockRestore();
   });
 
   it('should log debug information', async () => {
@@ -119,4 +143,56 @@ describe('connectWithExtensionProvider', () => {
     await connectWithExtensionProvider(instance);
     expect(instance.extensionActive).toBe(true);
   });
+
+  it('should handle error when analytics is disabled', async () => {
+    const error = new Error('Connection cancelled');
+    mockRequest.mockRejectedValue(error);
+
+    // Disable analytics
+    instance.options.enableAnalytics = false;
+
+    const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
+    const originalProvider = instance.activeProvider;
+    instance.sdkProvider = originalProvider;
+
+    await expect(connectWithExtensionProvider(instance)).rejects.toThrow(
+      'Connection cancelled',
+    );
+
+    // Should still emit the error event
+    expect(instance.emit).toHaveBeenCalledWith(
+      MetaMaskSDKEvent.ConnectWithResponse,
+      { error },
+    );
+
+    // Should NOT send analytics event when disabled
+    expect(mockSendAnalyticsEvent).not.toHaveBeenCalledWith({
+      event: TrackingEvents.REJECTED,
+    });
+
+    consoleWarnSpy.mockRestore();
+  });
+
+  it('should handle error when sdkProvider is not set', async () => {
+    const error = new Error('User denied access');
+    mockRequest.mockRejectedValue(error);
+
+    const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
+    instance.sdkProvider = undefined; // No previous provider to restore
+
+    await expect(connectWithExtensionProvider(instance)).rejects.toThrow(
+      'User denied access',
+    );
+
+    // Should emit error event
+    expect(instance.emit).toHaveBeenCalledWith(
+      MetaMaskSDKEvent.ConnectWithResponse,
+      { error },
+    );
+
+    // activeProvider should be set to extension (not restored since no sdkProvider)
+    expect(instance.activeProvider).toStrictEqual((global.window as any).extension);
+
+    consoleWarnSpy.mockRestore();
+  });
 });
diff --git a/packages/sdk/src/services/MetaMaskSDK/ProviderManager/connectWithExtensionProvider.ts b/packages/sdk/src/services/MetaMaskSDK/ProviderManager/connectWithExtensionProvider.ts
index 24e0d600a..bdd4818c0 100644
--- a/packages/sdk/src/services/MetaMaskSDK/ProviderManager/connectWithExtensionProvider.ts
+++ b/packages/sdk/src/services/MetaMaskSDK/ProviderManager/connectWithExtensionProvider.ts
@@ -37,12 +37,27 @@ export async function connectWithExtensionProvider(instance: MetaMaskSDK) {
       `[MetaMaskSDK: connectWithExtensionProvider()] accounts=${accounts}`,
     );
   } catch (err) {
-    // ignore error
     console.warn(
       `[MetaMaskSDK: connectWithExtensionProvider()] can't request accounts error`,
       err,
     );
-    return;
+
+    // Emit appropriate events for connection rejection/cancellation
+    instance.emit(MetaMaskSDKEvent.ConnectWithResponse, { error: err });
+
+    if (instance.options.enableAnalytics) {
+      instance.analytics?.send({ event: TrackingEvents.REJECTED });
+    }
+
+    // Restore original provider if extension connection failed
+    if (instance.sdkProvider) {
+      instance.activeProvider = instance.sdkProvider;
+      if (typeof window !== 'undefined') {
+        window.ethereum = instance.sdkProvider;
+      }
+    }
+
+    throw err; // Re-throw to allow calling code to handle the error
   }
 
   // remember setting for next time (until terminated)