From cffedfc936fef834e4ae35fbb8821b7132143758 Mon Sep 17 00:00:00 2001 From: Spencer Stock <46308524+spencerstock@users.noreply.github.com> Date: Mon, 23 Jun 2025 14:26:14 -0600 Subject: [PATCH 1/4] websocket refactor for improved connection reliability (#1664) --- .../connection/WalletLinkConnection.test.ts | 490 +++++++++++++++--- .../relay/connection/WalletLinkConnection.ts | 346 +++++++++---- .../relay/connection/WalletLinkWebSocket.ts | 52 +- 3 files changed, 718 insertions(+), 170 deletions(-) diff --git a/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.test.ts b/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.test.ts index a4c9626814..7e095afdf3 100644 --- a/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.test.ts +++ b/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.test.ts @@ -5,44 +5,81 @@ import { APP_VERSION_KEY, WALLET_USER_NAME_KEY } from '../constants.js'; import { WalletLinkSession } from '../type/WalletLinkSession.js'; import { WalletLinkCipher } from './WalletLinkCipher.js'; import { - WalletLinkConnection, - WalletLinkConnectionUpdateListener, + WalletLinkConnection, + WalletLinkConnectionUpdateListener, } from './WalletLinkConnection.js'; +import { ConnectionState } from './WalletLinkWebSocket.js'; const decryptMock = vi.fn().mockImplementation((text) => Promise.resolve(`decrypted ${text}`)); vi.spyOn(WalletLinkCipher.prototype, 'decrypt').mockImplementation(decryptMock); +const HEARTBEAT_INTERVAL = 10000; + +// Mock WebSocket to prevent real connections +vi.mock('./WalletLinkWebSocket.js', () => { + return { + ConnectionState: { + DISCONNECTED: 'disconnected', + CONNECTING: 'connecting', + CONNECTED: 'connected', + }, + WalletLinkWebSocket: vi.fn().mockImplementation(() => { + const mockWs = { + connect: vi.fn().mockResolvedValue(undefined), + disconnect: vi.fn(), + sendData: vi.fn(), + setConnectionStateListener: vi.fn(), + setIncomingDataListener: vi.fn(), + cleanup: vi.fn(), + }; + return mockWs; + }), + }; +}); + +// Mock window timer functions +beforeEach(() => { + vi.stubGlobal('setInterval', vi.fn()); + vi.stubGlobal('clearInterval', vi.fn()); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + describe('WalletLinkConnection', () => { const session = WalletLinkSession.create(new ScopedLocalStorage('walletlink', 'test')); let connection: WalletLinkConnection; let listener: WalletLinkConnectionUpdateListener; - let mockWorker: any; beforeEach(() => { vi.clearAllMocks(); - mockWorker = { - postMessage: vi.fn(), - terminate: vi.fn(), - addEventListener: vi.fn(), + listener = { + linkedUpdated: vi.fn(), + handleWeb3ResponseMessage: vi.fn(), + chainUpdated: vi.fn(), + accountUpdated: vi.fn(), + metadataUpdated: vi.fn(), + resetAndReload: vi.fn(), }; - global.Worker = vi.fn().mockImplementation(() => mockWorker); connection = new WalletLinkConnection({ session, linkAPIUrl: 'http://link-api-url', - listener: { - linkedUpdated: vi.fn(), - handleWeb3ResponseMessage: vi.fn(), - chainUpdated: vi.fn(), - accountUpdated: vi.fn(), - metadataUpdated: vi.fn(), - resetAndReload: vi.fn(), - }, - }); - listener = (connection as any).listener; + listener, + }); + }); + + afterEach(async () => { + // Only destroy if connection exists and hasn't been destroyed + if (connection && !(connection as any).destroyed) { + // Mock the makeRequest to prevent timeout errors + vi.spyOn(connection as any, 'makeRequest').mockResolvedValue({ type: 'SetSessionConfigOK' }); + await connection.destroy(); + } }); describe('incomingDataListener', () => { @@ -60,7 +97,12 @@ describe('WalletLinkConnection', () => { }, }; - (connection as any).ws.incomingDataListener?.({ + // Get the incoming data listener from the mock + const ws = (connection as any).ws; + const incomingDataListener = ws.setIncomingDataListener.mock.calls[0][0]; + + // Call the listener with the session config + incomingDataListener({ ...sessionConfig, type: 'SessionConfigUpdated', }); @@ -151,76 +193,400 @@ describe('WalletLinkConnection', () => { }); }); - describe('Heartbeat Worker Management', () => { - it('should create a heartbeat worker when startHeartbeat is called', () => { - (connection as any).startHeartbeat(); + describe('visibility and focus handling', () => { + let visibilityChangeHandler: () => void; + let focusHandler: () => void; + let pageshowHandler: (event: PageTransitionEvent) => void; + + beforeEach(() => { + // Capture event handlers + const addEventListenerSpy = vi.spyOn(document, 'addEventListener'); + const windowAddEventListenerSpy = vi.spyOn(window, 'addEventListener'); + + // Create a new connection to capture the handlers + connection = new WalletLinkConnection({ + session, + linkAPIUrl: 'http://link-api-url', + listener, + }); + + // Extract the handlers + visibilityChangeHandler = addEventListenerSpy.mock.calls.find( + call => call[0] === 'visibilitychange' + )?.[1] as () => void; + + focusHandler = windowAddEventListenerSpy.mock.calls.find( + call => call[0] === 'focus' + )?.[1] as () => void; + + pageshowHandler = windowAddEventListenerSpy.mock.calls.find( + call => call[0] === 'pageshow' + )?.[1] as (event: PageTransitionEvent) => void; + }); - expect(global.Worker).toHaveBeenCalledWith(expect.any(URL), { type: 'module' }); + it('should set up visibility change and focus handlers on construction', () => { + expect(visibilityChangeHandler).toBeDefined(); + expect(focusHandler).toBeDefined(); + expect(pageshowHandler).toBeDefined(); + }); + + it('should reconnect with fresh WebSocket when document becomes visible and disconnected', () => { + const reconnectSpy = vi.spyOn(connection as any, 'reconnectWithFreshWebSocket'); - expect(mockWorker.postMessage).toHaveBeenCalledWith({ type: 'start' }); + // Set disconnected state + (connection as any)._connected = false; + + // Mock document.hidden as false (visible) + Object.defineProperty(document, 'hidden', { + value: false, + writable: true, + }); + + visibilityChangeHandler(); + + expect(reconnectSpy).toHaveBeenCalledTimes(1); }); - it('should stop heartbeat worker when stopHeartbeat is called', () => { - (connection as any).startHeartbeat(); + it('should send heartbeat when document becomes visible and connected', () => { + const heartbeatSpy = vi.spyOn(connection as any, 'heartbeat').mockImplementation(() => {}); + + // Set connected state + (connection as any)._connected = true; - vi.clearAllMocks(); + // Mock document.hidden as false (visible) + Object.defineProperty(document, 'hidden', { + value: false, + writable: true, + }); - (connection as any).stopHeartbeat(); + visibilityChangeHandler(); - expect(mockWorker.postMessage).toHaveBeenCalledWith({ type: 'stop' }); - expect(mockWorker.terminate).toHaveBeenCalled(); + expect(heartbeatSpy).toHaveBeenCalledTimes(1); }); - it('should terminate existing worker before creating new one', () => { - (connection as any).startHeartbeat(); - const firstWorker = mockWorker; + it('should not reconnect when document is hidden', () => { + const reconnectSpy = vi.spyOn(connection as any, 'reconnectWithFreshWebSocket'); + + // Mock document.hidden as true + Object.defineProperty(document, 'hidden', { + value: true, + writable: true, + }); - const secondWorker = { - postMessage: vi.fn(), - terminate: vi.fn(), - addEventListener: vi.fn(), - }; - global.Worker = vi.fn().mockImplementation(() => secondWorker); + visibilityChangeHandler(); + + expect(reconnectSpy).not.toHaveBeenCalled(); + }); + + it('should reconnect on focus event when disconnected', () => { + const reconnectSpy = vi.spyOn(connection as any, 'reconnectWithFreshWebSocket'); + + // Set disconnected state + (connection as any)._connected = false; + (connection as any).destroyed = false; + + focusHandler(); + + expect(reconnectSpy).toHaveBeenCalledTimes(1); + }); + + it('should not reconnect on focus event when connected', () => { + const reconnectSpy = vi.spyOn(connection as any, 'reconnectWithFreshWebSocket'); + + // Set connected state + (connection as any)._connected = true; + + focusHandler(); + + expect(reconnectSpy).not.toHaveBeenCalled(); + }); + + it('should handle pageshow event with persisted flag', () => { + const reconnectSpy = vi.spyOn(connection as any, 'reconnectWithFreshWebSocket'); + + // Set disconnected state + (connection as any)._connected = false; + + const event = new Event('pageshow') as PageTransitionEvent; + Object.defineProperty(event, 'persisted', { value: true }); + + pageshowHandler(event); + + expect(reconnectSpy).toHaveBeenCalledTimes(1); + }); + + it('should remove event listeners on destroy', async () => { + const removeEventListenerSpy = vi.spyOn(document, 'removeEventListener'); + const windowRemoveEventListenerSpy = vi.spyOn(window, 'removeEventListener'); + + // Mock makeRequest to prevent timeout + vi.spyOn(connection as any, 'makeRequest').mockResolvedValue({ type: 'SetSessionConfigOK' }); + + await connection.destroy(); + + expect(removeEventListenerSpy).toHaveBeenCalledWith('visibilitychange', visibilityChangeHandler); + expect(windowRemoveEventListenerSpy).toHaveBeenCalledWith('focus', focusHandler); + }); + }); + + describe('reconnectWithFreshWebSocket', () => { + it('should disconnect old WebSocket and create new one', () => { + const oldWs = (connection as any).ws; + const disconnectSpy = vi.spyOn(oldWs, 'disconnect'); + + (connection as any).reconnectWithFreshWebSocket(); + + expect(disconnectSpy).toHaveBeenCalledTimes(1); + expect(oldWs.cleanup).toHaveBeenCalledTimes(1); + + // New WebSocket should be created + expect((connection as any).ws).not.toBe(oldWs); + // activeWsInstance should point to the new WebSocket + expect((connection as any).activeWsInstance).toBe((connection as any).ws); + }); + + it('should not reconnect if destroyed', () => { + (connection as any).destroyed = true; + const oldWs = (connection as any).ws; + const disconnectSpy = vi.spyOn(oldWs, 'disconnect'); + + (connection as any).reconnectWithFreshWebSocket(); + + expect(disconnectSpy).not.toHaveBeenCalled(); + }); + }); + + describe('WebSocket connection state handling', () => { + let ws: any; + let stateListener: (state: ConnectionState) => void; + + beforeEach(() => { + ws = (connection as any).ws; + stateListener = ws.setConnectionStateListener.mock.calls[0][0]; + }); - (connection as any).startHeartbeat(); + it('should track active WebSocket instance', () => { + expect((connection as any).activeWsInstance).toBe(ws); + }); + + it('should ignore events from non-active WebSocket instances', async () => { + // Mock handleConnected to track if connection logic runs + const handleConnectedSpy = vi.spyOn(connection as any, 'handleConnected'); + + // Create a different WebSocket instance + const oldWs = ws; + (connection as any).activeWsInstance = { different: 'instance' }; + + // Trigger state change on old instance + await stateListener.call(oldWs, ConnectionState.CONNECTED); + + // Connection logic should not run for non-active instances + expect(handleConnectedSpy).not.toHaveBeenCalled(); + }); + + it('should handle reconnection with delay', async () => { + vi.useFakeTimers(); + + // First disconnection - no delay + (connection as any).reconnectAttempts = 0; + (connection as any).activeWsInstance = ws; + await stateListener(ConnectionState.DISCONNECTED); + + // Wait for the async reconnect function to execute + await vi.runAllTimersAsync(); + expect((connection as any).reconnectAttempts).toBe(1); + + // Reset for second disconnection + (connection as any).isReconnecting = false; + (connection as any).activeWsInstance = ws; + (connection as any).destroyed = false; + + // Second disconnection - 3 second delay + await stateListener(ConnectionState.DISCONNECTED); + + // The reconnection should be delayed by 3 seconds + vi.advanceTimersByTime(2999); + expect((connection as any).reconnectAttempts).toBe(1); // Still 1, not incremented yet + + // Complete the delay and allow async operations + await vi.advanceTimersByTimeAsync(1); + expect((connection as any).reconnectAttempts).toBe(2); + + vi.useRealTimers(); + }); + + it('should prevent concurrent reconnection attempts', async () => { + (connection as any).isReconnecting = true; + (connection as any).activeWsInstance = ws; + const createWebSocketSpy = vi.spyOn(connection as any, 'createWebSocket'); + + await stateListener(ConnectionState.DISCONNECTED); + + expect(createWebSocketSpy).not.toHaveBeenCalled(); + }); + + it('should reset reconnect attempts on successful connection', async () => { + (connection as any).reconnectAttempts = 5; + (connection as any).activeWsInstance = ws; + vi.spyOn(connection as any, 'handleConnected').mockResolvedValue(true); + vi.spyOn(connection as any, 'fetchUnseenEventsAPI').mockResolvedValue([]); + + await stateListener(ConnectionState.CONNECTED); + + expect((connection as any).reconnectAttempts).toBe(0); + }); + + it('should cleanup old WebSocket on reconnection', async () => { + vi.useFakeTimers(); + (connection as any).activeWsInstance = ws; + (connection as any).destroyed = false; + + await stateListener(ConnectionState.DISCONNECTED); + + // Wait for the async reconnect function to execute + await vi.runAllTimersAsync(); + + expect(ws.cleanup).toHaveBeenCalledTimes(1); + vi.useRealTimers(); + }); + + it('should clear and restart heartbeat timer on connection state changes', async () => { + // Use the globally mocked functions + const clearIntervalMock = vi.mocked(clearInterval); + const setIntervalMock = vi.mocked(setInterval); + + // Mock setInterval to return a numeric ID + setIntervalMock.mockReturnValue(456 as any); + + // Mock successful connection + (connection as any).activeWsInstance = ws; + vi.spyOn(connection as any, 'handleConnected').mockResolvedValue(true); + vi.spyOn(connection as any, 'fetchUnseenEventsAPI').mockResolvedValue([]); + + // Simulate connected state + await stateListener(ConnectionState.CONNECTED); + expect(setIntervalMock).toHaveBeenCalledWith(expect.any(Function), HEARTBEAT_INTERVAL); + expect((connection as any).heartbeatIntervalId).toBe(456); + + // Simulate disconnected state + await stateListener(ConnectionState.DISCONNECTED); + expect(clearIntervalMock).toHaveBeenCalledWith(456); + expect((connection as any).heartbeatIntervalId).toBeUndefined(); + }); - // First worker should be terminated - expect(firstWorker.terminate).toHaveBeenCalled(); + it('should reset lastHeartbeatResponse on disconnect', async () => { + (connection as any).lastHeartbeatResponse = Date.now(); + (connection as any).activeWsInstance = ws; + + await stateListener(ConnectionState.DISCONNECTED); - // New worker should be created and started - expect(secondWorker.postMessage).toHaveBeenCalledWith({ type: 'start' }); + expect((connection as any).lastHeartbeatResponse).toBe(0); }); - it('should handle heartbeat messages from worker', () => { + it('should send immediate heartbeat after connection', async () => { + vi.useFakeTimers(); const heartbeatSpy = vi.spyOn(connection as any, 'heartbeat').mockImplementation(() => {}); + (connection as any).activeWsInstance = ws; + vi.spyOn(connection as any, 'handleConnected').mockResolvedValue(true); + vi.spyOn(connection as any, 'fetchUnseenEventsAPI').mockResolvedValue([]); + + // Mock setTimeout for the immediate heartbeat + const setTimeoutSpy = vi.spyOn(global, 'setTimeout'); + + await stateListener(ConnectionState.CONNECTED); + + // Check that setTimeout was called for immediate heartbeat + expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 100); + + // Execute the immediate heartbeat + vi.advanceTimersByTime(100); + expect(heartbeatSpy).toHaveBeenCalledTimes(1); + + vi.useRealTimers(); + }); - (connection as any).startHeartbeat(); - const messageListener = mockWorker.addEventListener.mock.calls.find( - (call: any[]) => call[0] === 'message' - )?.[1]; + }); - expect(messageListener).toBeDefined(); + describe('heartbeat mechanism', () => { + beforeEach(() => { + // Mock makeRequest to prevent actual network calls + vi.spyOn(connection as any, 'makeRequest').mockResolvedValue({ type: 'Heartbeat' }); + }); - messageListener({ data: { type: 'heartbeat' } }); + it('should update lastHeartbeatResponse on heartbeat', () => { + const now = Date.now(); + vi.spyOn(Date, 'now').mockReturnValue(now); + + (connection as any).updateLastHeartbeat(); + + expect((connection as any).lastHeartbeatResponse).toBe(now); + }); - expect(heartbeatSpy).toHaveBeenCalled(); + it('should handle heartbeat timeout and disconnect', () => { + const ws = (connection as any).ws; + const disconnectSpy = vi.spyOn(ws, 'disconnect'); + + // Set last heartbeat response to more than 2 intervals ago + (connection as any).lastHeartbeatResponse = Date.now() - (HEARTBEAT_INTERVAL * 3); + (connection as any)._connected = true; + + (connection as any).heartbeat(); + + // Should disconnect the WebSocket instead of calling reconnectWithFreshWebSocket + expect(disconnectSpy).toHaveBeenCalledTimes(1); }); - it('should handle stop when no worker exists', () => { - expect(() => { - (connection as any).stopHeartbeat(); - }).not.toThrow(); + it('should send heartbeat message when connection is healthy', () => { + const ws = (connection as any).ws; + const sendDataSpy = vi.spyOn(ws, 'sendData'); + + // Set recent heartbeat response + (connection as any).lastHeartbeatResponse = Date.now(); + (connection as any)._connected = true; + + (connection as any).heartbeat(); + + // Should send 'h' as heartbeat message + expect(sendDataSpy).toHaveBeenCalledWith('h'); + }); + }); - expect(mockWorker.postMessage).not.toHaveBeenCalled(); - expect(mockWorker.terminate).not.toHaveBeenCalled(); + describe('cleanup on destroy', () => { + beforeEach(() => { + // Mock makeRequest to prevent timeout errors + vi.spyOn(connection as any, 'makeRequest').mockResolvedValue({ type: 'SetSessionConfigOK' }); + }); + + it('should cleanup WebSocket instance if cleanup method exists', async () => { + const ws = (connection as any).ws; + + await connection.destroy(); + + expect(ws.cleanup).toHaveBeenCalledTimes(1); }); - it('should setup worker listeners correctly', () => { - (connection as any).startHeartbeat(); + it('should clear activeWsInstance on destroy', async () => { + expect((connection as any).activeWsInstance).toBeDefined(); + + await connection.destroy(); + + expect((connection as any).activeWsInstance).toBeUndefined(); + }); - expect(mockWorker.addEventListener).toHaveBeenCalledWith('message', expect.any(Function)); - expect(mockWorker.addEventListener).toHaveBeenCalledWith('error', expect.any(Function)); + it('should clear heartbeat interval on destroy', async () => { + // clearInterval is already mocked globally + const clearIntervalMock = vi.mocked(clearInterval); + + // Set up a heartbeat interval + (connection as any).heartbeatIntervalId = 123; + + await connection.destroy(); + + expect(clearIntervalMock).toHaveBeenCalledWith(123); + expect((connection as any).heartbeatIntervalId).toBeUndefined(); }); }); + + }); diff --git a/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.ts b/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.ts index fa3491b65d..1d89055b7d 100644 --- a/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.ts +++ b/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.ts @@ -36,6 +36,12 @@ export class WalletLinkConnection { private destroyed = false; private lastHeartbeatResponse = 0; private nextReqId = IntNumber(1); + private heartbeatIntervalId?: number; + private reconnectAttempts = 0; + private visibilityChangeHandler?: () => void; + private focusHandler?: () => void; + private activeWsInstance?: WalletLinkWebSocket; + private isReconnecting = false; private readonly session: WalletLinkSession; @@ -43,7 +49,8 @@ export class WalletLinkConnection { private cipher: WalletLinkCipher; private ws: WalletLinkWebSocket; private http: WalletLinkHTTP; - private heartbeatWorker?: Worker; + private readonly linkAPIUrl: string; + private readonly WebSocketClass: typeof WebSocket; /** * Constructor @@ -56,58 +63,143 @@ export class WalletLinkConnection { this.session = session; this.cipher = new WalletLinkCipher(session.secret); this.listener = listener; + this.linkAPIUrl = linkAPIUrl; + this.WebSocketClass = WebSocket; + + const ws = this.createWebSocket(); + this.ws = ws; + + this.http = new WalletLinkHTTP(linkAPIUrl, session.id, session.key); + + this.setupVisibilityChangeHandler(); + } + + private createWebSocket(): WalletLinkWebSocket { + const ws = new WalletLinkWebSocket(`${this.linkAPIUrl}/rpc`, this.WebSocketClass); + + // Track this as the active WebSocket instance + this.activeWsInstance = ws; - const ws = new WalletLinkWebSocket(`${linkAPIUrl}/rpc`, WebSocket); ws.setConnectionStateListener(async (state) => { + // Ignore events from non-active WebSocket instances + if (ws !== this.activeWsInstance) { + return; + } + // attempt to reconnect every 5 seconds when disconnected let connected = false; switch (state) { case ConnectionState.DISCONNECTED: - // Stop heartbeat when disconnected - this.stopHeartbeat(); - - // if DISCONNECTED and not destroyed + // Clear heartbeat timer when disconnected + if (this.heartbeatIntervalId) { + clearInterval(this.heartbeatIntervalId); + this.heartbeatIntervalId = undefined; + } + + // Reset lastHeartbeatResponse to prevent false timeout on reconnection + this.lastHeartbeatResponse = 0; + + // Reset connected state to false on disconnect + connected = false; + + // if DISCONNECTED and not destroyed, create a fresh WebSocket connection if (!this.destroyed) { - const connect = async () => { - // wait 5 seconds - await new Promise((resolve) => setTimeout(resolve, 5000)); - // check whether it's destroyed again - if (!this.destroyed) { - // reconnect - ws.connect().catch(() => { - connect(); - }); + const reconnect = async () => { + // Prevent multiple concurrent reconnection attempts + if (this.isReconnecting) { + return; + } + + this.isReconnecting = true; + + // 0 second delay on first attempt, then 3 seconds + const delay = this.reconnectAttempts === 0 ? 0 : 3000; + + // wait before reconnecting + await new Promise((resolve) => setTimeout(resolve, delay)); + + // check whether it's destroyed again and ensure this is still the active instance + if (!this.destroyed && ws === this.activeWsInstance) { + this.reconnectAttempts++; + + // Clean up the old WebSocket instance + if ('cleanup' in this.ws && typeof this.ws.cleanup === 'function') { + this.ws.cleanup(); + } + + // Create a fresh WebSocket instance + this.ws = this.createWebSocket(); + this.ws + .connect() + .catch(() => { + // Reconnection failed, will retry + }) + .finally(() => { + this.isReconnecting = false; + }); + } else { + this.isReconnecting = false; } }; - connect(); + reconnect(); } break; case ConnectionState.CONNECTED: + // Reset reconnect attempts on successful connection + this.reconnectAttempts = 0; + // perform authentication upon connection - // if CONNECTED, authenticate, and then check link status - connected = await this.handleConnected(); + try { + // if CONNECTED, authenticate, and then check link status + connected = await this.handleConnected(); + + // Always fetch unseen events when WebSocket state changes to CONNECTED + this.fetchUnseenEventsAPI().catch(() => { + // Failed to fetch unseen events after connection + }); + } catch (_error) { + // Don't set connected to true if authentication fails + break; + } + + // Update connected state immediately after successful authentication + // This ensures heartbeats won't be skipped + this.connected = connected; // send heartbeat every n seconds while connected - // if CONNECTED, start the heartbeat timer using WebWorker + // if CONNECTED, start the heartbeat timer + // first timer event updates lastHeartbeat timestamp + // subsequent calls send heartbeat message this.updateLastHeartbeat(); - this.startHeartbeat(); - // check for unseen events - if (this.shouldFetchUnseenEventsOnConnect) { - this.fetchUnseenEventsAPI(); + // Clear existing heartbeat timer + if (this.heartbeatIntervalId) { + clearInterval(this.heartbeatIntervalId); } + + this.heartbeatIntervalId = window.setInterval(() => { + this.heartbeat(); + }, HEARTBEAT_INTERVAL); + + // Send an immediate heartbeat + setTimeout(() => { + this.heartbeat(); + }, 100); + break; case ConnectionState.CONNECTING: break; } - // distinctUntilChanged - if (this.connected !== connected) { + // Update connected state for DISCONNECTED and CONNECTING cases + // For CONNECTED case, it's already set above + if (state !== ConnectionState.CONNECTED) { this.connected = connected; } }); + ws.setIncomingDataListener((m) => { switch (m.type) { // handle server's heartbeat responses @@ -141,9 +233,63 @@ export class WalletLinkConnection { this.requestResolutions.get(m.id)?.(m); } }); - this.ws = ws; - this.http = new WalletLinkHTTP(linkAPIUrl, session.id, session.key); + return ws; + } + + private setupVisibilityChangeHandler(): void { + this.visibilityChangeHandler = () => { + if (!document.hidden && !this.destroyed) { + if (!this.connected) { + // Force a fresh connection if we're disconnected + this.reconnectWithFreshWebSocket(); + } else { + // Otherwise send a heartbeat to check if connection is still alive + this.heartbeat(); + } + } + }; + + // Handle focus events (when user switches back to the tab/app) + this.focusHandler = () => { + if (!this.destroyed && !this.connected) { + this.reconnectWithFreshWebSocket(); + } + }; + + // Add event listeners + document.addEventListener('visibilitychange', this.visibilityChangeHandler); + window.addEventListener('focus', this.focusHandler); + + window.addEventListener('pageshow', (event) => { + if (event.persisted) { + if (this.focusHandler) { + this.focusHandler(); + } + } + }); + } + + private reconnectWithFreshWebSocket(): void { + if (this.destroyed) return; + + // Clear the active instance reference before disconnecting + const oldWs = this.ws; + this.activeWsInstance = undefined; + + // Disconnect current WebSocket + oldWs.disconnect(); + + // Clean up the old instance + if ('cleanup' in oldWs && typeof oldWs.cleanup === 'function') { + oldWs.cleanup(); + } + + // Create and connect fresh WebSocket + this.ws = this.createWebSocket(); + this.ws.connect().catch(() => { + // Fresh reconnection failed + }); } /** @@ -174,8 +320,31 @@ export class WalletLinkConnection { ); this.destroyed = true; - this.stopHeartbeat(); + + // Clear the active instance reference + this.activeWsInstance = undefined; + + // Clear heartbeat timer + if (this.heartbeatIntervalId) { + clearInterval(this.heartbeatIntervalId); + this.heartbeatIntervalId = undefined; + } + + // Remove event listeners + if (this.visibilityChangeHandler) { + document.removeEventListener('visibilitychange', this.visibilityChangeHandler); + } + if (this.focusHandler) { + window.removeEventListener('focus', this.focusHandler); + } + this.ws.disconnect(); + + // Call cleanup on the WebSocket instance if it has the method + if ('cleanup' in this.ws && typeof this.ws.cleanup === 'function') { + this.ws.cleanup(); + } + this.listener = undefined; } @@ -204,6 +373,8 @@ export class WalletLinkConnection { this.listener?.linkedUpdated(linked); } + + /** * Execute once when linked */ @@ -226,23 +397,20 @@ export class WalletLinkConnection { return; } - const decryptedData = await this.cipher.decrypt(m.data); - const message: WalletLinkEventData = JSON.parse(decryptedData); + try { + const decryptedData = await this.cipher.decrypt(m.data); + const message: WalletLinkEventData = JSON.parse(decryptedData); - if (message.type !== 'WEB3_RESPONSE') return; + if (message.type !== 'WEB3_RESPONSE') return; - const { id, response } = message; - this.listener?.handleWeb3ResponseMessage(id, response); + this.listener?.handleWeb3ResponseMessage(message.id, message.response); + } catch (_error) { + // Had error decrypting + } } - private shouldFetchUnseenEventsOnConnect = false; - public async checkUnseenEvents() { - if (!this.connected) { - this.shouldFetchUnseenEventsOnConnect = true; - return; - } - + // Add a small delay to ensure any pending operations complete await new Promise((resolve) => setTimeout(resolve, 250)); try { await this.fetchUnseenEventsAPI(); @@ -252,10 +420,15 @@ export class WalletLinkConnection { } private async fetchUnseenEventsAPI() { - this.shouldFetchUnseenEventsOnConnect = false; + try { + const responseEvents = await this.http.fetchUnseenEvents(); - const responseEvents = await this.http.fetchUnseenEvents(); - responseEvents.forEach((e) => this.handleIncomingEvent(e)); + responseEvents.forEach((e) => { + this.handleIncomingEvent(e); + }); + } catch (_error) { + // Failed to fetch unseen events + } } /** @@ -308,62 +481,21 @@ export class WalletLinkConnection { this.lastHeartbeatResponse = Date.now(); } - private startHeartbeat(): void { - if (this.heartbeatWorker) { - this.heartbeatWorker.terminate(); - } - - try { - // We put the heartbeat interval on a worker to avoid dropping the websocket connection when the webpage is backgrounded. - const workerUrl = new URL('./HeartbeatWorker.js', import.meta.url); - this.heartbeatWorker = new Worker(workerUrl, { type: 'module' }); - this.setupWorkerListeners(); - - this.heartbeatWorker.postMessage({ type: 'start' }); - } catch (error) { - console.warn('Failed to create external heartbeat worker', error); - } - } - - private setupWorkerListeners(): void { - if (!this.heartbeatWorker) return; - - this.heartbeatWorker.addEventListener('message', (event: MessageEvent<{ type: 'heartbeat' | 'started' | 'stopped' }>) => { - const { type } = event.data; - - switch (type) { - case 'heartbeat': - this.heartbeat(); - break; - case 'started': - case 'stopped': - // noop - break; - } - }); - - this.heartbeatWorker.addEventListener('error', (error) => { - console.error('Heartbeat worker error:', error); - }); - } - - private stopHeartbeat(): void { - if (this.heartbeatWorker) { - this.heartbeatWorker.postMessage({ type: 'stop' }); - this.heartbeatWorker.terminate(); - this.heartbeatWorker = undefined; - } - } - private heartbeat(): void { if (Date.now() - this.lastHeartbeatResponse > HEARTBEAT_INTERVAL * 2) { this.ws.disconnect(); return; } + + // Only send heartbeat if we're connected + if (!this.connected) { + return; + } + try { this.ws.sendData('h'); - } catch { - // noop + } catch (_error) { + // Error sending heartbeat } } @@ -401,7 +533,9 @@ export class WalletLinkConnection { sessionId: this.session.id, sessionKey: this.session.key, }); - if (res.type === 'Fail') return false; + if (res.type === 'Fail') { + return false; + } this.sendData({ type: 'IsLinked', @@ -448,13 +582,21 @@ export class WalletLinkConnection { }; private handleAccountUpdated = async (encryptedEthereumAddress: string) => { - const address = await this.cipher.decrypt(encryptedEthereumAddress); - this.listener?.accountUpdated(address); + try { + const address = await this.cipher.decrypt(encryptedEthereumAddress); + this.listener?.accountUpdated(address); + } catch { + // Had error decrypting + } }; private handleMetadataUpdated = async (key: string, encryptedMetadataValue: string) => { - const decryptedValue = await this.cipher.decrypt(encryptedMetadataValue); - this.listener?.metadataUpdated(key, decryptedValue); + try { + const decryptedValue = await this.cipher.decrypt(encryptedMetadataValue); + this.listener?.metadataUpdated(key, decryptedValue); + } catch { + // Had error decrypting + } }; private handleWalletUsernameUpdated = async (walletUsername: string) => { @@ -466,8 +608,12 @@ export class WalletLinkConnection { }; private handleChainUpdated = async (encryptedChainId: string, encryptedJsonRpcUrl: string) => { - const chainId = await this.cipher.decrypt(encryptedChainId); - const jsonRpcUrl = await this.cipher.decrypt(encryptedJsonRpcUrl); - this.listener?.chainUpdated(chainId, jsonRpcUrl); + try { + const chainId = await this.cipher.decrypt(encryptedChainId); + const jsonRpcUrl = await this.cipher.decrypt(encryptedJsonRpcUrl); + this.listener?.chainUpdated(chainId, jsonRpcUrl); + } catch { + // Had error decrypting + } }; } diff --git a/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkWebSocket.ts b/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkWebSocket.ts index a2443abadb..11b79f1250 100644 --- a/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkWebSocket.ts +++ b/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkWebSocket.ts @@ -9,9 +9,15 @@ export enum ConnectionState { } export class WalletLinkWebSocket { + // used to differentiate instances + private static instanceCounter = 0; + private static activeInstances = new Set(); + private static pendingData: string[] = []; + + private readonly instanceId: number; private readonly url: string; private webSocket: WebSocket | null = null; - private pendingData: string[] = []; + private isDisconnecting = false; private connectionStateListener?: (_: ConnectionState) => void; setConnectionStateListener(listener: (_: ConnectionState) => void): void { @@ -33,6 +39,8 @@ export class WalletLinkWebSocket { private readonly WebSocketClass: typeof WebSocket = WebSocket ) { this.url = url.replace(/^http/, 'ws'); + this.instanceId = WalletLinkWebSocket.instanceCounter++; + WalletLinkWebSocket.activeInstances.add(this.instanceId); } /** @@ -43,6 +51,9 @@ export class WalletLinkWebSocket { if (this.webSocket) { throw new Error('webSocket object is not null'); } + if (this.isDisconnecting) { + throw new Error('WebSocket is disconnecting, cannot reconnect on same instance'); + } return new Promise((resolve, reject) => { let webSocket: WebSocket; try { @@ -54,17 +65,23 @@ export class WalletLinkWebSocket { this.connectionStateListener?.(ConnectionState.CONNECTING); webSocket.onclose = (evt) => { this.clearWebSocket(); - reject(new Error(`websocket error ${evt.code}: ${evt.reason}`)); + + // Only reject the connection promise if we haven't connected yet + if (webSocket.readyState !== WebSocket.OPEN) { + reject(new Error(`websocket error ${evt.code}: ${evt.reason}`)); + } + this.connectionStateListener?.(ConnectionState.DISCONNECTED); }; + webSocket.onopen = (_) => { resolve(); this.connectionStateListener?.(ConnectionState.CONNECTED); - if (this.pendingData.length > 0) { - const pending = [...this.pendingData]; + if (WalletLinkWebSocket.pendingData.length > 0) { + const pending = [...WalletLinkWebSocket.pendingData]; pending.forEach((data) => this.sendData(data)); - this.pendingData = []; + WalletLinkWebSocket.pendingData = []; } }; webSocket.onmessage = (evt) => { @@ -77,7 +94,6 @@ export class WalletLinkWebSocket { const message = JSON.parse(evt.data) as ServerMessage; this.incomingDataListener?.(message); } catch { - /* empty */ } } }; @@ -92,8 +108,12 @@ export class WalletLinkWebSocket { if (!webSocket) { return; } + + // Mark as disconnecting to prevent reconnection attempts on this instance + this.isDisconnecting = true; this.clearWebSocket(); + // Clear listeners this.connectionStateListener?.(ConnectionState.DISCONNECTED); this.connectionStateListener = undefined; this.incomingDataListener = undefined; @@ -112,10 +132,19 @@ export class WalletLinkWebSocket { public sendData(data: string): void { const { webSocket } = this; if (!webSocket) { - this.pendingData.push(data); - this.connect(); + WalletLinkWebSocket.pendingData.push(data); + if (!this.isDisconnecting) { + this.connect(); + } return; } + + // Check if WebSocket is actually open before sending + if (webSocket.readyState !== WebSocket.OPEN) { + WalletLinkWebSocket.pendingData.push(data); + return; + } + webSocket.send(data); } @@ -130,4 +159,11 @@ export class WalletLinkWebSocket { webSocket.onmessage = null; webSocket.onopen = null; } + + /** + * remove ws from active instances + */ + public cleanup(): void { + WalletLinkWebSocket.activeInstances.delete(this.instanceId); + } } From 6b093e2694596c4b8cf6468e5df4e85ff588f4e2 Mon Sep 17 00:00:00 2001 From: Felix Zhang Date: Mon, 14 Jul 2025 11:25:42 -0700 Subject: [PATCH 2/4] remove heartbeat worker --- .../relay/connection/HeartbeatWorker.js | 62 ----- .../relay/connection/HeartbeatWorker.test.ts | 234 ------------------ 2 files changed, 296 deletions(-) delete mode 100644 packages/wallet-sdk/src/sign/walletlink/relay/connection/HeartbeatWorker.js delete mode 100644 packages/wallet-sdk/src/sign/walletlink/relay/connection/HeartbeatWorker.test.ts diff --git a/packages/wallet-sdk/src/sign/walletlink/relay/connection/HeartbeatWorker.js b/packages/wallet-sdk/src/sign/walletlink/relay/connection/HeartbeatWorker.js deleted file mode 100644 index f772671838..0000000000 --- a/packages/wallet-sdk/src/sign/walletlink/relay/connection/HeartbeatWorker.js +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright (c) 2018-2025 Coinbase, Inc. - -/** - * This worker is used to send heartbeat messages to the main thread. - * It is used to keep the websocket connection alive when the webpage is backgrounded. - * - */ - -// Define the heartbeat interval constant directly in the worker to avoid import issues -const HEARTBEAT_INTERVAL = 10000; - -let heartbeatInterval; - -// Listen for messages from the main thread -self.addEventListener('message', (event) => { - const { type } = event.data; - - switch (type) { - case 'start': - startHeartbeat(); - break; - case 'stop': - stopHeartbeat(); - break; - default: - console.warn('Unknown message type received by HeartbeatWorker:', type); - } -}); - -function startHeartbeat() { - // Clear any existing interval - if (heartbeatInterval) { - clearInterval(heartbeatInterval); - } - - // Start the heartbeat interval - heartbeatInterval = setInterval(() => { - // Send heartbeat message to main thread - const response = { type: 'heartbeat' }; - self.postMessage(response); - }, HEARTBEAT_INTERVAL); - - // Send confirmation that heartbeat started - const response = { type: 'started' }; - self.postMessage(response); -} - -function stopHeartbeat() { - if (heartbeatInterval) { - clearInterval(heartbeatInterval); - heartbeatInterval = undefined; - } - - // Send confirmation that heartbeat stopped - const response = { type: 'stopped' }; - self.postMessage(response); -} - -// Handle worker termination -self.addEventListener('beforeunload', () => { - stopHeartbeat(); -}); \ No newline at end of file diff --git a/packages/wallet-sdk/src/sign/walletlink/relay/connection/HeartbeatWorker.test.ts b/packages/wallet-sdk/src/sign/walletlink/relay/connection/HeartbeatWorker.test.ts deleted file mode 100644 index 026ce2fd4a..0000000000 --- a/packages/wallet-sdk/src/sign/walletlink/relay/connection/HeartbeatWorker.test.ts +++ /dev/null @@ -1,234 +0,0 @@ -import '@vitest/web-worker'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - -describe('HeartbeatWorker', () => { - let worker: Worker; - - beforeEach(async () => { - // Create a new worker instance for each test - worker = new Worker(new URL('./HeartbeatWorker.js', import.meta.url)); - }); - - afterEach(() => { - if (worker) { - worker.terminate(); - } - }); - - describe('Message Handling', () => { - it('should start heartbeat and send confirmation', async () => { - const messagePromise = new Promise((resolve) => { - worker.addEventListener('message', resolve, { once: true }); - }); - - worker.postMessage({ type: 'start' }); - - const event = await messagePromise; - expect(event.data).toEqual({ type: 'started' }); - }); - - it('should send heartbeat messages at regular intervals', async () => { - worker.postMessage({ type: 'start' }); - - await new Promise((resolve) => { - worker.addEventListener('message', (event) => { - if (event.data.type === 'started') { - resolve(); - } - }, { once: true }); - }); - - const heartbeats: MessageEvent[] = []; - const heartbeatPromise = new Promise((resolve) => { - let count = 0; - worker.addEventListener('message', (event) => { - if (event.data.type === 'heartbeat') { - heartbeats.push(event); - count++; - if (count >= 2) { - resolve(); - } - } - }); - }); - - // Wait for at least 2 heartbeat messages (this will take ~20 seconds in real time) - // For testing, we'll use a shorter timeout and verify the structure - await Promise.race([ - heartbeatPromise, - new Promise((_, reject) => setTimeout(() => reject(new Error('Timeout waiting for heartbeats')), 25000)) - ]); - - expect(heartbeats.length).toBeGreaterThanOrEqual(2); - heartbeats.forEach(event => { - expect(event.data).toEqual({ type: 'heartbeat' }); - }); - }, 30000); // 30 second timeout for this test - - it('should stop heartbeat and send confirmation', async () => { - worker.postMessage({ type: 'start' }); - - await new Promise((resolve) => { - worker.addEventListener('message', (event) => { - if (event.data.type === 'started') { - resolve(); - } - }, { once: true }); - }); - - const stopPromise = new Promise((resolve) => { - worker.addEventListener('message', (event) => { - if (event.data.type === 'stopped') { - resolve(event); - } - }, { once: true }); - }); - - worker.postMessage({ type: 'stop' }); - - const event = await stopPromise; - expect(event.data).toEqual({ type: 'stopped' }); - }); - - it('should handle unknown message types gracefully', async () => { - const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); - - worker.postMessage({ type: 'unknown' }); - - // Give the worker time to process the message - await new Promise(resolve => setTimeout(resolve, 100)); - - // Note: We can't directly verify console.warn was called in the worker context - // but we can verify the worker doesn't crash or send unexpected messages - - const messagePromise = new Promise((resolve) => { - worker.addEventListener('message', resolve, { once: true }); - }); - - worker.postMessage({ type: 'start' }); - const event = await messagePromise; - expect(event.data).toEqual({ type: 'started' }); - - consoleSpy.mockRestore(); - }); - }); - - describe('Heartbeat Interval Management', () => { - it('should handle restart without issues', async () => { - worker.postMessage({ type: 'start' }); - - await new Promise((resolve) => { - worker.addEventListener('message', (event) => { - if (event.data.type === 'started') { - resolve(); - } - }, { once: true }); - }); - - // Start again (should clear previous interval) - const secondStartPromise = new Promise((resolve) => { - worker.addEventListener('message', (event) => { - if (event.data.type === 'started') { - resolve(event); - } - }, { once: true }); - }); - - worker.postMessage({ type: 'start' }); - const event = await secondStartPromise; - expect(event.data).toEqual({ type: 'started' }); - }); - - it('should stop cleanly even when no heartbeat is running', async () => { - const stopPromise = new Promise((resolve) => { - worker.addEventListener('message', resolve, { once: true }); - }); - - // Stop without starting first - worker.postMessage({ type: 'stop' }); - - const event = await stopPromise; - expect(event.data).toEqual({ type: 'stopped' }); - }); - }); - - describe('Message Flow', () => { - it('should handle complete start-heartbeat-stop cycle', async () => { - const messages: any[] = []; - - worker.addEventListener('message', (event) => { - messages.push(event.data); - }); - - worker.postMessage({ type: 'start' }); - - await new Promise((resolve) => { - const checkMessages = () => { - if (messages.some(msg => msg.type === 'started')) { - resolve(); - } else { - setTimeout(checkMessages, 10); - } - }; - checkMessages(); - }); - - await new Promise((resolve) => { - const checkMessages = () => { - if (messages.some(msg => msg.type === 'heartbeat')) { - resolve(); - } else { - setTimeout(checkMessages, 100); - } - }; - checkMessages(); - }); - - worker.postMessage({ type: 'stop' }); - - await new Promise((resolve) => { - const checkMessages = () => { - if (messages.some(msg => msg.type === 'stopped')) { - resolve(); - } else { - setTimeout(checkMessages, 10); - } - }; - checkMessages(); - }); - - // Verify we got all expected message types - expect(messages.some(msg => msg.type === 'started')).toBe(true); - expect(messages.some(msg => msg.type === 'heartbeat')).toBe(true); - expect(messages.some(msg => msg.type === 'stopped')).toBe(true); - }, 15000); // 15 second timeout - - it('should use correct heartbeat interval timing', async () => { - const heartbeatTimes: number[] = []; - - worker.addEventListener('message', (event) => { - if (event.data.type === 'heartbeat') { - heartbeatTimes.push(Date.now()); - } - }); - - worker.postMessage({ type: 'start' }); - - await new Promise((resolve) => { - const checkHeartbeats = () => { - if (heartbeatTimes.length >= 2) { - resolve(); - } else { - setTimeout(checkHeartbeats, 100); - } - }; - checkHeartbeats(); - }); - - // Verify the interval is approximately 10 seconds (allow some tolerance) - const interval = heartbeatTimes[1] - heartbeatTimes[0]; - expect(interval).toBeGreaterThan(9500); // 9.5 seconds minimum - expect(interval).toBeLessThan(10500); // 10.5 seconds maximum - }, 25000); // 25 second timeout - }); -}); \ No newline at end of file From 3db503ed0caa61ed1882b96a6f60a6546b63bef3 Mon Sep 17 00:00:00 2001 From: Felix Zhang Date: Mon, 14 Jul 2025 11:52:22 -0700 Subject: [PATCH 3/4] lint + test --- packages/wallet-sdk/package.json | 2 +- .../sign/walletlink/WalletLinkSigner.test.ts | 19 --- .../connection/WalletLinkConnection.test.ts | 129 +++++++++--------- .../relay/connection/WalletLinkConnection.ts | 4 +- .../relay/connection/WalletLinkWebSocket.ts | 1 + .../walletlink/relay/ui/WLMobileRelayUI.ts | 4 +- packages/wallet-sdk/vitest.config.ts | 1 + 7 files changed, 70 insertions(+), 90 deletions(-) diff --git a/packages/wallet-sdk/package.json b/packages/wallet-sdk/package.json index 6d2fabd4e1..3757d6157d 100644 --- a/packages/wallet-sdk/package.json +++ b/packages/wallet-sdk/package.json @@ -23,7 +23,7 @@ "test": "vitest", "test:coverage": "yarn test:unit && open coverage/lcov-report/index.html", "prebuild": "rm -rf ./dist && node -p \"'export const VERSION = \\'' + require('./package.json').version + '\\';\\nexport const NAME = \\'' + require('./package.json').name + '\\';'\" > src/sdk-info.ts", - "build": "node compile-assets.cjs && tsc -p ./tsconfig.build.json && tsc-alias && cp -a src/vendor-js dist && cp src/sign/walletlink/relay/connection/HeartbeatWorker.js dist/sign/walletlink/relay/connection/", + "build": "node compile-assets.cjs && tsc -p ./tsconfig.build.json && tsc-alias && cp -a src/vendor-js dist", "dev": "yarn build && tsc --watch & nodemon --watch dist --delay 1 --exec tsc-alias", "typecheck": "tsc --noEmit", "lint": "eslint . --ext .ts,.tsx --fix", diff --git a/packages/wallet-sdk/src/sign/walletlink/WalletLinkSigner.test.ts b/packages/wallet-sdk/src/sign/walletlink/WalletLinkSigner.test.ts index 2882478618..8a608fcb04 100644 --- a/packages/wallet-sdk/src/sign/walletlink/WalletLinkSigner.test.ts +++ b/packages/wallet-sdk/src/sign/walletlink/WalletLinkSigner.test.ts @@ -221,25 +221,6 @@ describe('LegacyProvider', () => { }); }); - test.skip('eth_signTypedData_v1', async () => { - const hashSpy = vi.spyOn(eip712, 'hashForSignTypedDataLegacy'); - const response = await provider?.request({ - method: 'eth_signTypedData_v1', - params: [[MOCK_TYPED_DATA], MOCK_ADDERESS], - }); - expect(hashSpy).toHaveBeenCalled(); - expect(sendRequestSpy).toBeCalledWith({ - method: 'signEthereumMessage', - params: { - address: MOCK_ADDERESS.toLowerCase(), - message: ENCODED_MESSAGE, - addPrefix: false, - typedDataJson: ENCODED_TYPED_DATA_JSON, - }, - }); - expect(response).toBe('signTypedData mocked result'); - }); - test('eth_signTypedData_v3', async () => { const hashSpy = vi.spyOn(eip712, 'hashForSignTypedData_v3'); const response = await provider?.request({ diff --git a/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.test.ts b/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.test.ts index 7e095afdf3..ab9a18d472 100644 --- a/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.test.ts +++ b/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.test.ts @@ -1,14 +1,14 @@ import { vi } from 'vitest'; -import { ScopedLocalStorage } from ':core/storage/ScopedLocalStorage.js'; import { APP_VERSION_KEY, WALLET_USER_NAME_KEY } from '../constants.js'; import { WalletLinkSession } from '../type/WalletLinkSession.js'; import { WalletLinkCipher } from './WalletLinkCipher.js'; import { - WalletLinkConnection, - WalletLinkConnectionUpdateListener, + WalletLinkConnection, + WalletLinkConnectionUpdateListener, } from './WalletLinkConnection.js'; import { ConnectionState } from './WalletLinkWebSocket.js'; +import { ScopedLocalStorage } from ':core/storage/ScopedLocalStorage.js'; const decryptMock = vi.fn().mockImplementation((text) => Promise.resolve(`decrypted ${text}`)); @@ -212,15 +212,15 @@ describe('WalletLinkConnection', () => { // Extract the handlers visibilityChangeHandler = addEventListenerSpy.mock.calls.find( - call => call[0] === 'visibilitychange' + (call) => call[0] === 'visibilitychange' )?.[1] as () => void; focusHandler = windowAddEventListenerSpy.mock.calls.find( - call => call[0] === 'focus' + (call) => call[0] === 'focus' )?.[1] as () => void; pageshowHandler = windowAddEventListenerSpy.mock.calls.find( - call => call[0] === 'pageshow' + (call) => call[0] === 'pageshow' )?.[1] as (event: PageTransitionEvent) => void; }); @@ -232,10 +232,10 @@ describe('WalletLinkConnection', () => { it('should reconnect with fresh WebSocket when document becomes visible and disconnected', () => { const reconnectSpy = vi.spyOn(connection as any, 'reconnectWithFreshWebSocket'); - + // Set disconnected state (connection as any)._connected = false; - + // Mock document.hidden as false (visible) Object.defineProperty(document, 'hidden', { value: false, @@ -249,10 +249,10 @@ describe('WalletLinkConnection', () => { it('should send heartbeat when document becomes visible and connected', () => { const heartbeatSpy = vi.spyOn(connection as any, 'heartbeat').mockImplementation(() => {}); - + // Set connected state (connection as any)._connected = true; - + // Mock document.hidden as false (visible) Object.defineProperty(document, 'hidden', { value: false, @@ -266,7 +266,7 @@ describe('WalletLinkConnection', () => { it('should not reconnect when document is hidden', () => { const reconnectSpy = vi.spyOn(connection as any, 'reconnectWithFreshWebSocket'); - + // Mock document.hidden as true Object.defineProperty(document, 'hidden', { value: true, @@ -280,7 +280,7 @@ describe('WalletLinkConnection', () => { it('should reconnect on focus event when disconnected', () => { const reconnectSpy = vi.spyOn(connection as any, 'reconnectWithFreshWebSocket'); - + // Set disconnected state (connection as any)._connected = false; (connection as any).destroyed = false; @@ -292,7 +292,7 @@ describe('WalletLinkConnection', () => { it('should not reconnect on focus event when connected', () => { const reconnectSpy = vi.spyOn(connection as any, 'reconnectWithFreshWebSocket'); - + // Set connected state (connection as any)._connected = true; @@ -303,7 +303,7 @@ describe('WalletLinkConnection', () => { it('should handle pageshow event with persisted flag', () => { const reconnectSpy = vi.spyOn(connection as any, 'reconnectWithFreshWebSocket'); - + // Set disconnected state (connection as any)._connected = false; @@ -324,7 +324,10 @@ describe('WalletLinkConnection', () => { await connection.destroy(); - expect(removeEventListenerSpy).toHaveBeenCalledWith('visibilitychange', visibilityChangeHandler); + expect(removeEventListenerSpy).toHaveBeenCalledWith( + 'visibilitychange', + visibilityChangeHandler + ); expect(windowRemoveEventListenerSpy).toHaveBeenCalledWith('focus', focusHandler); }); }); @@ -333,12 +336,12 @@ describe('WalletLinkConnection', () => { it('should disconnect old WebSocket and create new one', () => { const oldWs = (connection as any).ws; const disconnectSpy = vi.spyOn(oldWs, 'disconnect'); - + (connection as any).reconnectWithFreshWebSocket(); expect(disconnectSpy).toHaveBeenCalledTimes(1); expect(oldWs.cleanup).toHaveBeenCalledTimes(1); - + // New WebSocket should be created expect((connection as any).ws).not.toBe(oldWs); // activeWsInstance should point to the new WebSocket @@ -372,46 +375,46 @@ describe('WalletLinkConnection', () => { it('should ignore events from non-active WebSocket instances', async () => { // Mock handleConnected to track if connection logic runs const handleConnectedSpy = vi.spyOn(connection as any, 'handleConnected'); - + // Create a different WebSocket instance const oldWs = ws; (connection as any).activeWsInstance = { different: 'instance' }; - + // Trigger state change on old instance await stateListener.call(oldWs, ConnectionState.CONNECTED); - + // Connection logic should not run for non-active instances expect(handleConnectedSpy).not.toHaveBeenCalled(); }); it('should handle reconnection with delay', async () => { vi.useFakeTimers(); - + // First disconnection - no delay (connection as any).reconnectAttempts = 0; (connection as any).activeWsInstance = ws; await stateListener(ConnectionState.DISCONNECTED); - + // Wait for the async reconnect function to execute await vi.runAllTimersAsync(); expect((connection as any).reconnectAttempts).toBe(1); - + // Reset for second disconnection (connection as any).isReconnecting = false; (connection as any).activeWsInstance = ws; (connection as any).destroyed = false; - + // Second disconnection - 3 second delay await stateListener(ConnectionState.DISCONNECTED); - + // The reconnection should be delayed by 3 seconds vi.advanceTimersByTime(2999); expect((connection as any).reconnectAttempts).toBe(1); // Still 1, not incremented yet - + // Complete the delay and allow async operations await vi.advanceTimersByTimeAsync(1); expect((connection as any).reconnectAttempts).toBe(2); - + vi.useRealTimers(); }); @@ -419,9 +422,9 @@ describe('WalletLinkConnection', () => { (connection as any).isReconnecting = true; (connection as any).activeWsInstance = ws; const createWebSocketSpy = vi.spyOn(connection as any, 'createWebSocket'); - + await stateListener(ConnectionState.DISCONNECTED); - + expect(createWebSocketSpy).not.toHaveBeenCalled(); }); @@ -430,9 +433,9 @@ describe('WalletLinkConnection', () => { (connection as any).activeWsInstance = ws; vi.spyOn(connection as any, 'handleConnected').mockResolvedValue(true); vi.spyOn(connection as any, 'fetchUnseenEventsAPI').mockResolvedValue([]); - + await stateListener(ConnectionState.CONNECTED); - + expect((connection as any).reconnectAttempts).toBe(0); }); @@ -440,12 +443,12 @@ describe('WalletLinkConnection', () => { vi.useFakeTimers(); (connection as any).activeWsInstance = ws; (connection as any).destroyed = false; - + await stateListener(ConnectionState.DISCONNECTED); - + // Wait for the async reconnect function to execute await vi.runAllTimersAsync(); - + expect(ws.cleanup).toHaveBeenCalledTimes(1); vi.useRealTimers(); }); @@ -454,20 +457,20 @@ describe('WalletLinkConnection', () => { // Use the globally mocked functions const clearIntervalMock = vi.mocked(clearInterval); const setIntervalMock = vi.mocked(setInterval); - + // Mock setInterval to return a numeric ID setIntervalMock.mockReturnValue(456 as any); - + // Mock successful connection (connection as any).activeWsInstance = ws; vi.spyOn(connection as any, 'handleConnected').mockResolvedValue(true); vi.spyOn(connection as any, 'fetchUnseenEventsAPI').mockResolvedValue([]); - + // Simulate connected state await stateListener(ConnectionState.CONNECTED); expect(setIntervalMock).toHaveBeenCalledWith(expect.any(Function), HEARTBEAT_INTERVAL); expect((connection as any).heartbeatIntervalId).toBe(456); - + // Simulate disconnected state await stateListener(ConnectionState.DISCONNECTED); expect(clearIntervalMock).toHaveBeenCalledWith(456); @@ -477,9 +480,9 @@ describe('WalletLinkConnection', () => { it('should reset lastHeartbeatResponse on disconnect', async () => { (connection as any).lastHeartbeatResponse = Date.now(); (connection as any).activeWsInstance = ws; - + await stateListener(ConnectionState.DISCONNECTED); - + expect((connection as any).lastHeartbeatResponse).toBe(0); }); @@ -489,23 +492,21 @@ describe('WalletLinkConnection', () => { (connection as any).activeWsInstance = ws; vi.spyOn(connection as any, 'handleConnected').mockResolvedValue(true); vi.spyOn(connection as any, 'fetchUnseenEventsAPI').mockResolvedValue([]); - + // Mock setTimeout for the immediate heartbeat const setTimeoutSpy = vi.spyOn(global, 'setTimeout'); - + await stateListener(ConnectionState.CONNECTED); - + // Check that setTimeout was called for immediate heartbeat expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 100); - + // Execute the immediate heartbeat vi.advanceTimersByTime(100); expect(heartbeatSpy).toHaveBeenCalledTimes(1); - + vi.useRealTimers(); }); - - }); describe('heartbeat mechanism', () => { @@ -517,22 +518,22 @@ describe('WalletLinkConnection', () => { it('should update lastHeartbeatResponse on heartbeat', () => { const now = Date.now(); vi.spyOn(Date, 'now').mockReturnValue(now); - + (connection as any).updateLastHeartbeat(); - + expect((connection as any).lastHeartbeatResponse).toBe(now); }); it('should handle heartbeat timeout and disconnect', () => { const ws = (connection as any).ws; const disconnectSpy = vi.spyOn(ws, 'disconnect'); - + // Set last heartbeat response to more than 2 intervals ago - (connection as any).lastHeartbeatResponse = Date.now() - (HEARTBEAT_INTERVAL * 3); + (connection as any).lastHeartbeatResponse = Date.now() - HEARTBEAT_INTERVAL * 3; (connection as any)._connected = true; - + (connection as any).heartbeat(); - + // Should disconnect the WebSocket instead of calling reconnectWithFreshWebSocket expect(disconnectSpy).toHaveBeenCalledTimes(1); }); @@ -540,13 +541,13 @@ describe('WalletLinkConnection', () => { it('should send heartbeat message when connection is healthy', () => { const ws = (connection as any).ws; const sendDataSpy = vi.spyOn(ws, 'sendData'); - + // Set recent heartbeat response (connection as any).lastHeartbeatResponse = Date.now(); (connection as any)._connected = true; - + (connection as any).heartbeat(); - + // Should send 'h' as heartbeat message expect(sendDataSpy).toHaveBeenCalledWith('h'); }); @@ -560,33 +561,31 @@ describe('WalletLinkConnection', () => { it('should cleanup WebSocket instance if cleanup method exists', async () => { const ws = (connection as any).ws; - + await connection.destroy(); - + expect(ws.cleanup).toHaveBeenCalledTimes(1); }); it('should clear activeWsInstance on destroy', async () => { expect((connection as any).activeWsInstance).toBeDefined(); - + await connection.destroy(); - + expect((connection as any).activeWsInstance).toBeUndefined(); }); it('should clear heartbeat interval on destroy', async () => { // clearInterval is already mocked globally const clearIntervalMock = vi.mocked(clearInterval); - + // Set up a heartbeat interval (connection as any).heartbeatIntervalId = 123; - + await connection.destroy(); - + expect(clearIntervalMock).toHaveBeenCalledWith(123); expect((connection as any).heartbeatIntervalId).toBeUndefined(); }); }); - - }); diff --git a/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.ts b/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.ts index 1d89055b7d..f53a7fd7f3 100644 --- a/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.ts +++ b/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkConnection.ts @@ -1,6 +1,5 @@ // Copyright (c) 2018-2023 Coinbase, Inc. -import { IntNumber } from ':core/type/index.js'; import { APP_VERSION_KEY, WALLET_USER_NAME_KEY } from '../constants.js'; import { ClientMessage } from '../type/ClientMessage.js'; import { ServerMessage, ServerMessageType } from '../type/ServerMessage.js'; @@ -10,6 +9,7 @@ import { Web3Response } from '../type/Web3Response.js'; import { WalletLinkCipher } from './WalletLinkCipher.js'; import { WalletLinkHTTP } from './WalletLinkHTTP.js'; import { ConnectionState, WalletLinkWebSocket } from './WalletLinkWebSocket.js'; +import { IntNumber } from ':core/type/index.js'; const HEARTBEAT_INTERVAL = 10000; const REQUEST_TIMEOUT = 60000; @@ -373,8 +373,6 @@ export class WalletLinkConnection { this.listener?.linkedUpdated(linked); } - - /** * Execute once when linked */ diff --git a/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkWebSocket.ts b/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkWebSocket.ts index 11b79f1250..0e62fda14a 100644 --- a/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkWebSocket.ts +++ b/packages/wallet-sdk/src/sign/walletlink/relay/connection/WalletLinkWebSocket.ts @@ -94,6 +94,7 @@ export class WalletLinkWebSocket { const message = JSON.parse(evt.data) as ServerMessage; this.incomingDataListener?.(message); } catch { + // noop } } }; diff --git a/packages/wallet-sdk/src/sign/walletlink/relay/ui/WLMobileRelayUI.ts b/packages/wallet-sdk/src/sign/walletlink/relay/ui/WLMobileRelayUI.ts index 82ede3cd98..79b89c9b5e 100644 --- a/packages/wallet-sdk/src/sign/walletlink/relay/ui/WLMobileRelayUI.ts +++ b/packages/wallet-sdk/src/sign/walletlink/relay/ui/WLMobileRelayUI.ts @@ -1,7 +1,7 @@ -import { CBW_MOBILE_DEEPLINK_URL } from ':core/constants.js'; -import { RelayUI } from './RelayUI.js'; import { RedirectDialog } from './components/RedirectDialog/RedirectDialog.js'; import { getLocation } from './components/util.js'; +import { RelayUI } from './RelayUI.js'; +import { CBW_MOBILE_DEEPLINK_URL } from ':core/constants.js'; export class WLMobileRelayUI implements RelayUI { private readonly redirectDialog: RedirectDialog; diff --git a/packages/wallet-sdk/vitest.config.ts b/packages/wallet-sdk/vitest.config.ts index af6327c3c1..99a7bf6869 100644 --- a/packages/wallet-sdk/vitest.config.ts +++ b/packages/wallet-sdk/vitest.config.ts @@ -6,6 +6,7 @@ export default defineConfig({ alias: { ':core': path.resolve(__dirname, 'src/core'), ':util': path.resolve(__dirname, 'src/util'), + ':sign': path.resolve(__dirname, 'src/sign'), }, environment: 'jsdom', globals: true, From 027987300b2c859e8966d8ec5d1c8ed4ffd423b4 Mon Sep 17 00:00:00 2001 From: Felix Zhang Date: Mon, 14 Jul 2025 12:03:42 -0700 Subject: [PATCH 4/4] bump version --- packages/wallet-sdk/package.json | 2 +- packages/wallet-sdk/src/sdk-info.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/wallet-sdk/package.json b/packages/wallet-sdk/package.json index 3757d6157d..ad29c2a01d 100644 --- a/packages/wallet-sdk/package.json +++ b/packages/wallet-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@coinbase/wallet-sdk", - "version": "4.3.5", + "version": "4.3.6", "description": "Coinbase Wallet JavaScript SDK", "keywords": [ "coinbase", diff --git a/packages/wallet-sdk/src/sdk-info.ts b/packages/wallet-sdk/src/sdk-info.ts index 03398a1205..17f9d13761 100644 --- a/packages/wallet-sdk/src/sdk-info.ts +++ b/packages/wallet-sdk/src/sdk-info.ts @@ -1,2 +1,2 @@ -export const VERSION = '4.3.5'; +export const VERSION = '4.3.6'; export const NAME = '@coinbase/wallet-sdk';