fix: Prevent lengthy spinner on load when selected network is slow

app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.test.ts

@@ -2,9 +2,9 @@ import { PendingJsonRpcResponse } from '@metamask/utils';
 import { JsonRpcEngineEndCallback } from '@metamask/json-rpc-engine';
 import getProviderState, {
   GetProviderState,
+  ProviderStateHandlerRequest,
   ProviderStateHandlerResult,
 } from './get-provider-state';
-import { HandlerRequestType } from './types';
 
 describe('getProviderState', () => {
   let mockEnd: JsonRpcEngineEndCallback;
@@ -21,9 +21,11 @@ describe('getProviderState', () => {
   });
 
   it('should call getProviderState when the handler is invoked', async () => {
-    const req: HandlerRequestType = {
+    const req: ProviderStateHandlerRequest = {
       origin: 'testOrigin',
-      params: [],
+      params: {
+        isInitializingStreamProvider: true,
+      },
       id: '22',
       jsonrpc: '2.0',
       method: 'metamask_getProviderState',
@@ -44,7 +46,9 @@ describe('getProviderState', () => {
       getProviderState: mockGetProviderState,
     });
 
-    expect(mockGetProviderState).toHaveBeenCalledWith(req.origin);
+    expect(mockGetProviderState).toHaveBeenCalledWith(req.origin, {
+      isInitializingStreamProvider: true,
+    });
     expect(res.result).toStrictEqual({
       chainId: '0x539',
       isUnlocked: true,

app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.ts

@@ -4,14 +4,17 @@ import type {
 } from '@metamask/json-rpc-engine';
 import type {
   PendingJsonRpcResponse,
-  JsonRpcParams,
   Hex,
+  JsonRpcRequest,
 } from '@metamask/utils';
 import { MESSAGE_TYPE } from '../../../../../shared/constants/app';
-import {
-  HandlerWrapper,
-  HandlerRequestType as ProviderStateHandlerRequest,
-} from './types';
+import { HandlerWrapper } from './types';
+
+export type ProviderStateHandlerRequest = JsonRpcRequest<{
+  isInitializingStreamProvider?: boolean;
+}> & {
+  origin: string;
+};
 
 /**
  * @property chainId - The current chain ID.
@@ -28,18 +31,18 @@ export type ProviderStateHandlerResult = {
 
 export type GetProviderState = (
   origin: string,
+  options?: { isInitializingStreamProvider?: boolean },
 ) => Promise<ProviderStateHandlerResult>;
 
-type GetProviderStateConstraint<Params extends JsonRpcParams = JsonRpcParams> =
-  {
-    implementation: (
-      _req: ProviderStateHandlerRequest<Params>,
-      res: PendingJsonRpcResponse<ProviderStateHandlerResult>,
-      _next: JsonRpcEngineNextCallback,
-      end: JsonRpcEngineEndCallback,
-      { _getProviderState }: Record<string, GetProviderState>,
-    ) => Promise<void>;
-  } & HandlerWrapper;
+type GetProviderStateConstraint = {
+  implementation: (
+    _req: ProviderStateHandlerRequest,
+    res: PendingJsonRpcResponse<ProviderStateHandlerResult>,
+    _next: JsonRpcEngineNextCallback,
+    end: JsonRpcEngineEndCallback,
+    { _getProviderState }: Record<string, GetProviderState>,
+  ) => Promise<void>;
+} & HandlerWrapper;
 
 /**
  * This RPC method gets background state relevant to the provider.
@@ -64,17 +67,16 @@ export default getProviderState;
  * @param options
  * @param options.getProviderState - An async function that gets the current provider state.
  */
-async function getProviderStateHandler<
-  Params extends JsonRpcParams = JsonRpcParams,
->(
-  req: ProviderStateHandlerRequest<Params>,
+async function getProviderStateHandler(
+  req: ProviderStateHandlerRequest,
   res: PendingJsonRpcResponse<ProviderStateHandlerResult>,
   _next: JsonRpcEngineNextCallback,
   end: JsonRpcEngineEndCallback,
   { getProviderState: _getProviderState }: Record<string, GetProviderState>,
 ): Promise<void> {
+  const isInitializingStreamProvider = req.params?.isInitializingStreamProvider;
   res.result = {
-    ...(await _getProviderState(req.origin)),
+    ...(await _getProviderState(req.origin, { isInitializingStreamProvider })),
   };
   return end();
 }

app/scripts/metamask-controller.js

@@ -3297,6 +3297,7 @@ export default class MetamaskController extends EventEmitter {
 
     // setup memStore subscription hooks
     this.on('update', updatePublicConfigStore);
+    // Update the store asynchronously, out-of-band
     updatePublicConfigStore(this.getState());
 
     return publicConfigStore;
@@ -3306,10 +3307,18 @@ export default class MetamaskController extends EventEmitter {
    * Gets relevant state for the provider of an external origin.
    *
    * @param {string} origin - The origin to get the provider state for.
+   * @param {object} [options] - Options.
+   * @param {boolean} [options.isInitializingStreamProvider] - Whether this method is being used to initialize the StreamProvider (default: false).
    * @returns {Promise<{ isUnlocked: boolean, networkVersion: string, chainId: string, accounts: string[], extensionId: string | undefined }>} An object with relevant state properties.
    */
-  async getProviderState(origin) {
-    const providerNetworkState = await this.getProviderNetworkState(origin);
+  async getProviderState(
+    origin,
+    { isInitializingStreamProvider = false } = {},
+  ) {
+    const providerNetworkState = await this.getProviderNetworkState({
+      origin,
+      isInitializingStreamProvider,
+    });
     const metadata = {};
     if (isManifestV3) {
       const { chrome } = globalThis;
@@ -3330,10 +3339,15 @@ export default class MetamaskController extends EventEmitter {
   /**
    * Retrieves network state information relevant for external providers.
    *
-   * @param {string} origin - The origin identifier for which network state is requested (default: 'metamask').
+   * @param {object} [args] - The arguments to this function.
+   * @param {string} [args.origin] - The origin identifier for which network state is requested (default: 'metamask').
+   * @param {boolean} [args.isInitializingStreamProvider] - Whether this method is being used to initialize the StreamProvider (default: false).
    * @returns {object} An object containing important network state properties, including chainId and networkVersion.
    */
-  async getProviderNetworkState(origin = METAMASK_DOMAIN) {
+  async getProviderNetworkState({
+    origin = METAMASK_DOMAIN,
+    isInitializingStreamProvider = false,
+  } = {}) {
     const networkClientId = this.controllerMessenger.call(
       'SelectedNetworkController:getNetworkClientIdForDomain',
       origin,
@@ -3349,7 +3363,22 @@ export default class MetamaskController extends EventEmitter {
     const { completedOnboarding } = this.onboardingController.state;
 
     let networkVersion = this.deprecatedNetworkVersions[networkClientId];
-    if (networkVersion === undefined && completedOnboarding) {
+    // We use `metamask_getProviderState` to set the initial state of the
+    // StreamProvider. The StreamProvider allows the UI to make network requests
+    // through the background connection, and it must be initialized before we
+    // can show the UI. However, this creates a problem if the selected network
+    // is slow or unresponsive, because then the network request will hang and
+    // thus we will be unable to show the UI. To get around this, we prevent a
+    // request from occurring during initialization
+    // (`isInitializingStreamProvider` = true). `metamask_getProviderState` is
+    // called each time the memState is updated, so eventually we _will_ make
+    // this request (`isInitializingStreamProvider` = false), and if the network
+    // recovers, the network version will be properly retrieved at that time.
+    if (
+      networkVersion === undefined &&
+      completedOnboarding &&
+      !isInitializingStreamProvider
+    ) {
       try {
         const result = await networkClient.provider.request({
           method: 'net_version',
@@ -8980,7 +9009,7 @@ export default class MetamaskController extends EventEmitter {
     this.notifyAllConnections(
       async (origin) => ({
         method: NOTIFICATION_NAMES.chainChanged,
-        params: await this.getProviderNetworkState(origin),
+        params: await this.getProviderNetworkState({ origin }),
       }),
       API_TYPE.EIP1193,
     );
@@ -8989,7 +9018,7 @@ export default class MetamaskController extends EventEmitter {
   async _notifyChainChangeForConnection(connection, origin) {
     this.notifyConnection(connection, {
       method: NOTIFICATION_NAMES.chainChanged,
-      params: await this.getProviderNetworkState(origin),
+      params: await this.getProviderNetworkState({ origin }),
     });
   }
 

package.json

@@ -333,7 +333,7 @@
     "@metamask/ppom-validator": "0.36.0",
     "@metamask/preinstalled-example-snap": "^0.7.1",
     "@metamask/profile-sync-controller": "^23.0.0",
-    "@metamask/providers": "^22.1.0",
+    "@metamask/providers": "^22.1.1",
     "@metamask/rate-limit-controller": "^6.0.3",
     "@metamask/remote-feature-flag-controller": "^1.6.0",
     "@metamask/rpc-errors": "^7.0.0",

test/e2e/tests/network/slow-network.spec.ts

@@ -0,0 +1,75 @@
+import { Suite } from 'mocha';
+import { Mockttp } from 'mockttp';
+import { NetworkStatus } from '@metamask/network-controller';
+import { withFixtures } from '../../helpers';
+import FixtureBuilder from '../../fixture-builder';
+import LoginPage from '../../page-objects/pages/login-page';
+
+describe('Slow network', function (this: Suite) {
+  it('does not show the spinner while attempting to determine the network status', async function () {
+    await withFixtures(
+      {
+        fixtures: new FixtureBuilder({ inputChainId: '0x539' })
+          .withNetworkController({
+            networkConfigurations: {
+              'AAAA-AAAA-AAAA-AAAA': {
+                id: 'AAAA-AAAA-AAAA-AAAA',
+                chainId: '0x9999',
+                nickname: 'A Custom Network',
+                rpcUrl: 'https://customnetwork.test',
+                ticker: 'ETH',
+                blockExplorerUrl: undefined,
+              },
+            },
+            networksMetadata: {
+              'AAAA-AAAA-AAAA-AAAA': {
+                EIPS: {},
+                status: NetworkStatus.Unknown,
+              },
+            },
+            selectedNetworkClientId: 'AAAA-AAAA-AAAA-AAAA',
+            providerConfig: {
+              id: 'AAAA-AAAA-AAAA-AAAA',
+            },
+          })
+          .withEnabledNetworks({
+            eip155: {
+              '0x9999': true,
+            },
+          })
+          .build(),
+        localNodeOptions: 'none',
+        title: this.test?.fullTitle(),
+        testSpecificMock: async (mockServer: Mockttp) => {
+          // Cause requests to the network to be pending indefinitely
+          return await mockServer
+            .forPost('https://customnetwork.test')
+            .thenPassThrough({
+              beforeRequest: async () => {
+                // Wait long enough to cause a timeout
+                await new Promise<void>((resolve) => {
+                  setTimeout(() => {
+                    resolve();
+                  }, 10000);
+                });
+                return undefined;
+              },
+            });
+        },
+      },
+      async ({ driver }) => {
+        await driver.navigate();
+        const loginPage = new LoginPage(driver);
+        await loginPage.checkPageIsLoaded();
+        await loginPage.loginToHomepage();
+
+        // Ensure that the connection spinner shows up.
+        // This wouldn't happen if we waited for the net_version (mocked above)
+        // to fully resolve.
+        await driver.waitForSelector({
+          text: 'Connecting to A Custom Network',
+        });
+      },
+    );
+  });
+});

yarn.lock

@@ -7028,9 +7028,9 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@metamask/providers@npm:^22.1.0":
-  version: 22.1.0
-  resolution: "@metamask/providers@npm:22.1.0"
+"@metamask/providers@npm:^22.1.0, @metamask/providers@npm:^22.1.1":
+  version: 22.1.1
+  resolution: "@metamask/providers@npm:22.1.1"
   dependencies:
     "@metamask/json-rpc-engine": "npm:^10.0.2"
     "@metamask/json-rpc-middleware-stream": "npm:^8.0.6"
@@ -7045,7 +7045,7 @@ __metadata:
     readable-stream: "npm:^3.6.2"
   peerDependencies:
     webextension-polyfill: ^0.10.0 || ^0.11.0 || ^0.12.0
-  checksum: 10/d6dc969296e3d478a904228f27adae3b6dcbfdbf49eb6d571c9d73d7506df3c6e3bf3c3464f1e69e4c0acb6f6072d12d1c8182348e626ca1572f4f22f9c585e6
+  checksum: 10/50194c608fb308cee268c6eefb8c8d439a9d07aa41d07cb5ddcd7e706819aea7e746150f32688fe06d20309b49346440855b5d56aacf286b343da6fcb217cc12
   languageName: node
   linkType: hard
 
@@ -31815,7 +31815,7 @@ __metadata:
     "@metamask/preferences-controller": "npm:^17.0.0"
     "@metamask/preinstalled-example-snap": "npm:^0.7.1"
     "@metamask/profile-sync-controller": "npm:^23.0.0"
-    "@metamask/providers": "npm:^22.1.0"
+    "@metamask/providers": "npm:^22.1.1"
     "@metamask/rate-limit-controller": "npm:^6.0.3"
     "@metamask/remote-feature-flag-controller": "npm:^1.6.0"
     "@metamask/rpc-errors": "npm:^7.0.0"