fix: warmup timeout (#87)

* fix: warmup timeout

* fix: warmup timeout

Author: EdouardBougon

README.md

@@ -32,15 +32,49 @@ const result = await client.invokeMethod({
 await client.revokeSession();
 ```
 
-### Configuring Transport Timeout
+### Configuring Transport Timeouts
 
-You can configure a default timeout (in milliseconds) for all requests made through the transport by passing the `defaultTimeout` option to `getDefaultTransport`:
+#### Default Request Timeout
+
+By default, the transport has **no timeout** (`-1`) for requests. This is because most operations require user interaction (e.g., confirming transactions in the MetaMask extension), and we don't want to prematurely cancel requests while the user is reviewing them.
+
+However, you can configure a default timeout (in milliseconds) for all requests by passing the `defaultTimeout` option:
+
+```typescript
+const transport = getDefaultTransport({ defaultTimeout: 30000 }); // 30 seconds timeout for all requests
+const client = getMultichainClient({ transport });
+```
+
+To explicitly disable timeouts (wait indefinitely), set the timeout to `-1`:
+
+```typescript
+const transport = getDefaultTransport({ defaultTimeout: -1 }); // No timeout (default behavior)
+```
+
+#### Warmup Timeout
+
+The `warmupTimeout` is a special timeout used specifically for the **first request** sent immediately after the transport establishes its connection. This is useful because:
+
+- Some transports need a brief moment to fully initialize before they can reliably process requests
+- The initial "warmup" request is typically a lightweight check (e.g., `wallet_getSession`) that doesn't require user interaction
+- This timeout is usually much shorter than the regular request timeout
 
 ```typescript
-const transport = getDefaultTransport({ defaultTimeout: 5000 }); // 5 seconds timeout for all requests
+const transport = getDefaultTransport({
+  warmupTimeout: 200,  // 200 ms for the initial warmup request
+  defaultTimeout: -1    // No timeout for subsequent requests (user interactions)
+});
 const client = getMultichainClient({ transport });
 ```
 
+**Key differences between `warmupTimeout` and `defaultTimeout`:**
+
+| Property         | Purpose                       | Typical Value     | When Applied                             |
+| ---------------- | ----------------------------- | ----------------- | ---------------------------------------- |
+| `warmupTimeout`  | Initial connection validation | 200 ms            | Only the first request after `connect()` |
+| `defaultTimeout` | Regular request operations    | `-1` (no timeout) | All subsequent requests                  |
+```
+
 ## Extending RPC Types
 
 The client's RPC requests are strongly typed, enforcing the RPC methods and params to be defined ahead of usage. The client supports extending
@@ -103,6 +137,7 @@ A transport must implement the following interface:
 
 ```typescript
 type Transport = {
+  warmupTimeout?: number;  // Optional timeout for the initial warmup request
   connect: () => Promise<void>;
   disconnect: () => Promise<void>;
   isConnected: () => boolean;
@@ -121,31 +156,50 @@ import { TransportError, TransportTimeoutError } from '@metamask/multichain-api-
 import type { Transport, TransportRequest, TransportResponse } from '@metamask/multichain-api-client';
 
 type CustomTransportOptions = {
-  defaultTimeout?: number; // ms
+  defaultTimeout?: number;  // Default timeout for all requests (use -1 for no timeout)
+  warmupTimeout?: number;   // Optional timeout for the initial warmup request
 };
 
 export function getCustomTransport(options: CustomTransportOptions = {}): Transport {
-  const { defaultTimeout = 5000 } = options;
+  const { defaultTimeout = -1, warmupTimeout } = options; // Default: no timeout
 
   return {
+    warmupTimeout,  // Expose warmupTimeout for the client to use
     connect: async () => { ... },
     disconnect: async () => { ... },
     isConnected: () => { ...},
-    request: async <TRequest extends TransportRequest, TResponse extends TransportResponse>( request: TRequest, { timeout }: { timeout?: number } = {}): Promise<TResponse> => { ... },
+    request: async <TRequest extends TransportRequest, TResponse extends TransportResponse>(
+      request: TRequest,
+      { timeout = defaultTimeout }: { timeout?: number } = {}
+    ): Promise<TResponse> => {
+      // If timeout is -1, don't apply any timeout
+      if (timeout === -1) {
+        return performRequest(request); // Your actual request logic
+      }
+
+      // Otherwise, wrap the request with a timeout
+      return withTimeout(
+        performRequest(request),
+        timeout,
+        () => new TransportTimeoutError()
+      );
+    },
     onNotification: (callback: (data: unknown) => void) => { ... },
   };
 }
 
-// Usage
-const transport = getCustomTransport({ defaultTimeout: 8000 });
+// Usage examples
+const transport = getCustomTransport({
+  warmupTimeout: 500,  // 500 ms for initial connection check
+  defaultTimeout: -1    // No timeout for user interactions (default)
+});
 const client = getMultichainClient({ transport });
 
-// Per-request override
+// Per-request timeout override
 await client.invokeMethod({
   scope: 'eip155:1',
   request: { method: 'eth_chainId', params: [] },
-  // The transport's request implementation can expose a timeout override
-  { timeout: 10000  // 10 seconds timeout for this request only
+  { timeout: 10000 }  // 10 seconds timeout for this specific request
 });
 ```
 

src/helpers/utils.test.ts

@@ -106,5 +106,18 @@ describe('utils', () => {
         'custom',
       );
     });
+
+    it('should not apply timeout when timeoutMs is -1', async () => {
+      const slowPromise = new Promise<string>((resolve) => {
+        setTimeout(() => resolve('completed'), 100);
+      });
+      const result = await withTimeout(slowPromise, -1);
+      expect(result).toBe('completed');
+    });
+
+    it('should handle rejection when timeoutMs is -1', async () => {
+      const failingPromise = Promise.reject(new Error('failed'));
+      await expect(withTimeout(failingPromise, -1)).rejects.toThrow('failed');
+    });
   });
 });

src/helpers/utils.ts

@@ -68,10 +68,15 @@ export async function withRetry<T>(
 /**
  * Returns a promise that resolves or rejects like the given promise, but fails if the timeout is exceeded.
  * @param promise - The promise to monitor
- * @param timeoutMs - Maximum duration in ms
+ * @param timeoutMs - Maximum duration in ms. Use -1 to disable timeout.
  * @param errorFactory - Optional callback to generate a custom error on timeout
  */
 export function withTimeout<T>(promise: Promise<T>, timeoutMs: number, errorFactory?: () => Error): Promise<T> {
+  // If timeout is -1, return the promise without timeout
+  if (timeoutMs === -1) {
+    return promise;
+  }
+
   return new Promise<T>((resolve, reject) => {
     const timer = setTimeout(() => {
       if (errorFactory) {

src/index.ts

@@ -23,11 +23,12 @@ import type { Transport } from './types/transport';
 function getDefaultTransport({
   extensionId,
   defaultTimeout,
-}: { extensionId?: string; defaultTimeout?: number } = {}): Transport {
+  warmupTimeout,
+}: { extensionId?: string; defaultTimeout?: number; warmupTimeout?: number } = {}): Transport {
   const isChrome = isChromeRuntime();
   return isChrome
-    ? getExternallyConnectableTransport({ extensionId, defaultTimeout })
-    : getWindowPostMessageTransport({ defaultTimeout });
+    ? getExternallyConnectableTransport({ extensionId, defaultTimeout, warmupTimeout })
+    : getWindowPostMessageTransport({ defaultTimeout, warmupTimeout });
 }
 
 export { getMultichainClient, getDefaultTransport, getExternallyConnectableTransport, getWindowPostMessageTransport };

src/multichainClient.test.ts

@@ -27,7 +27,7 @@ describe('getMultichainClient', () => {
 
     expect(result).toEqual(mockSession);
     // First call from initialization
-    expect(mockTransport.request).toHaveBeenNthCalledWith(1, { method: 'wallet_getSession' });
+    expect(mockTransport.request).toHaveBeenNthCalledWith(1, { method: 'wallet_getSession' }, { timeout: 1_000 });
     // Second call is the createSession request including options object
     expect(mockTransport.request).toHaveBeenNthCalledWith(
       2,
@@ -44,9 +44,14 @@ describe('getMultichainClient', () => {
     const result = await client.getSession();
 
     expect(result).toEqual(mockSession);
-    expect(mockTransport.request).toHaveBeenCalledWith({
-      method: 'wallet_getSession',
-    });
+    // First call from initialization with warmupTimeout
+    expect(mockTransport.request).toHaveBeenNthCalledWith(1, { method: 'wallet_getSession' }, { timeout: 1_000 });
+    // Second call from explicit getSession()
+    expect(mockTransport.request).toHaveBeenNthCalledWith(
+      2,
+      { method: 'wallet_getSession', params: undefined },
+      { timeout: undefined },
+    );
   });
 
   describe('revokeSession', () => {
@@ -85,7 +90,7 @@ describe('getMultichainClient', () => {
       },
     });
     expect(signAndSendResult).toEqual({ signature: 'mock-signature' });
-    expect(mockTransport.request).toHaveBeenNthCalledWith(1, { method: 'wallet_getSession' });
+    expect(mockTransport.request).toHaveBeenNthCalledWith(1, { method: 'wallet_getSession' }, { timeout: 1_000 });
     expect(mockTransport.request).toHaveBeenNthCalledWith(
       2,
       {

src/multichainClient.ts

@@ -70,7 +70,9 @@ export function getMultichainClient<T extends RpcApi = DefaultRpcApi>({
       await ensureConnected();
 
       // Use withRetry to handle the case where the Multichain API requests don't resolve on page load (cf. https://github.com/MetaMask/metamask-mobile/issues/16550)
-      await withRetry(() => transport.request({ method: 'wallet_getSession' }));
+      await withRetry(() =>
+        transport.request({ method: 'wallet_getSession' }, { timeout: transport.warmupTimeout ?? 1_000 }),
+      );
     })();
 
     return await initializationPromise;

src/transports/constants.ts

@@ -5,4 +5,5 @@ export const INPAGE = 'metamask-inpage';
 export const MULTICHAIN_SUBSTREAM_NAME = 'metamask-multichain-provider';
 export const METAMASK_PROVIDER_STREAM_NAME = 'metamask-provider';
 export const METAMASK_EXTENSION_CONNECT_CAN_RETRY = 'METAMASK_EXTENSION_CONNECT_CAN_RETRY';
-export const DEFAULT_REQUEST_TIMEOUT = 200; // 200ms
+export const DEFAULT_REQUEST_TIMEOUT = -1; // No timeout by default
+export const DEFAULT_WARMUP_TIMEOUT = 200; // 200 ms for initial warmup request

src/transports/externallyConnectableTransport.test.ts

@@ -179,4 +179,24 @@ describe('ExternallyConnectableTransport', () => {
     const response = await secondPromise;
     expect(response).toEqual({ id: MOCK_INITIAL_REQUEST_ID + 1, jsonrpc: '2.0', result: mockSession });
   });
+
+  it('should expose warmupTimeout when provided', () => {
+    const transportWithWarmup = getExternallyConnectableTransport({
+      extensionId: testExtensionId,
+      warmupTimeout: 500,
+    });
+    expect(transportWithWarmup.warmupTimeout).toBe(500);
+  });
+
+  it('should have default warmupTimeout of 200ms when not provided', () => {
+    expect(transport.warmupTimeout).toBe(200);
+  });
+
+  it('should support -1 as warmupTimeout to disable timeout', () => {
+    const transportWithNoTimeout = getExternallyConnectableTransport({
+      extensionId: testExtensionId,
+      warmupTimeout: -1,
+    });
+    expect(transportWithNoTimeout.warmupTimeout).toBe(-1);
+  });
 });

src/transports/externallyConnectableTransport.ts

@@ -2,7 +2,7 @@ import { detectMetamaskExtensionId } from '../helpers/metamaskExtensionId';
 import { getUniqueId, withTimeout } from '../helpers/utils';
 import { TransportError, TransportTimeoutError } from '../types/errors';
 import type { Transport, TransportResponse } from '../types/transport';
-import { DEFAULT_REQUEST_TIMEOUT, REQUEST_CAIP } from './constants';
+import { DEFAULT_REQUEST_TIMEOUT, DEFAULT_WARMUP_TIMEOUT, REQUEST_CAIP } from './constants';
 
 /**
  * Creates a transport that communicates with the MetaMask extension via Chrome's externally_connectable API
@@ -23,10 +23,10 @@ import { DEFAULT_REQUEST_TIMEOUT, REQUEST_CAIP } from './constants';
  * ```
  */
 export function getExternallyConnectableTransport(
-  params: { extensionId?: string; defaultTimeout?: number } = {},
+  params: { extensionId?: string; defaultTimeout?: number; warmupTimeout?: number } = {},
 ): Transport {
   let { extensionId } = params;
-  const { defaultTimeout = DEFAULT_REQUEST_TIMEOUT } = params;
+  const { defaultTimeout = DEFAULT_REQUEST_TIMEOUT, warmupTimeout = DEFAULT_WARMUP_TIMEOUT } = params;
   let chromePort: chrome.runtime.Port | undefined;
   let requestId = getUniqueId();
   const pendingRequests = new Map<number, (value: any) => void>();
@@ -74,6 +74,7 @@ export function getExternallyConnectableTransport(
   }
 
   return {
+    warmupTimeout,
     connect: async () => {
       try {
         if (!extensionId) {

src/transports/windowPostMessageTransport.test.ts

@@ -350,4 +350,18 @@ describe('WindowPostMessageTransport', () => {
     const result = await secondPromise;
     expect(result).toEqual({ id: MOCK_INITIAL_REQUEST_ID + 1, result: mockSession });
   });
+
+  it('should expose warmupTimeout when provided', () => {
+    const transportWithWarmup = getWindowPostMessageTransport({ warmupTimeout: 500 });
+    expect(transportWithWarmup.warmupTimeout).toBe(500);
+  });
+
+  it('should have default warmupTimeout of 200ms when not provided', () => {
+    expect(transport.warmupTimeout).toBe(200);
+  });
+
+  it('should support -1 as warmupTimeout to disable timeout', () => {
+    const transportWithNoTimeout = getWindowPostMessageTransport({ warmupTimeout: -1 });
+    expect(transportWithNoTimeout.warmupTimeout).toBe(-1);
+  });
 });