Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions packages/core-backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- Add `forceReconnection()` method to `BackendWebSocketService` for controlled subscription state cleanup ([#6861](https://github.com/MetaMask/core/pull/6861))
- Performs a controlled disconnect-then-reconnect sequence with exponential backoff
- Useful for recovering from subscription/unsubscription issues and cleaning up orphaned subscriptions
- Add `BackendWebSocketService:forceReconnection` messenger action
- Add stable connection timer to prevent rapid reconnection loops ([#6861](https://github.com/MetaMask/core/pull/6861))
- Connection must stay stable for 10 seconds before resetting reconnect attempts
- Prevents issues when server accepts connection then immediately closes it

### Changed

- Update `AccountActivityService` to use new `forceReconnection()` method instead of manually calling disconnect/connect ([#6861](https://github.com/MetaMask/core/pull/6861))
- Improve reconnection scheduling in `BackendWebSocketService` to be idempotent ([#6861](https://github.com/MetaMask/core/pull/6861))
- Prevents duplicate reconnection timers and inflated attempt counters
- Scheduler checks if reconnect is already scheduled before creating new timer
- Renamed internal `#scheduleReconnect()` to `#scheduleConnect()` for clarity
- Improve error handling in `BackendWebSocketService.connect()` ([#6861](https://github.com/MetaMask/core/pull/6861))
- Always schedule reconnect on connection failure (exponential backoff prevents aggressive retries)
- Remove redundant schedule calls from error paths
- Update `BackendWebSocketService.disconnect()` to reset reconnect attempts counter ([#6861](https://github.com/MetaMask/core/pull/6861))
- Update `BackendWebSocketService.disconnect()` return type from `Promise<void>` to `void` ([#6861](https://github.com/MetaMask/core/pull/6861))
- Improve logging throughout `BackendWebSocketService` for better debugging ([#6861](https://github.com/MetaMask/core/pull/6861))

### Fixed

- Fix potential race condition in `BackendWebSocketService.connect()` that could bypass exponential backoff when reconnect is already scheduled ([#6861](https://github.com/MetaMask/core/pull/6861))
- Fix memory leak from orphaned timers when multiple reconnects are scheduled ([#6861](https://github.com/MetaMask/core/pull/6861))
- Fix issue where reconnect attempts counter could grow unnecessarily with duplicate scheduled reconnects ([#6861](https://github.com/MetaMask/core/pull/6861))

## [2.1.0]

### Added
Expand Down
38 changes: 13 additions & 25 deletions packages/core-backend/src/AccountActivityService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const getMessenger = () => {
// Create mock action handlers
const mockGetSelectedAccount = jest.fn();
const mockConnect = jest.fn();
const mockDisconnect = jest.fn();
const mockForceReconnection = jest.fn();
const mockSubscribe = jest.fn();
const mockChannelHasSubscription = jest.fn();
const mockGetSubscriptionsByChannel = jest.fn();
Expand All @@ -89,8 +89,8 @@ const getMessenger = () => {
mockConnect,
);
rootMessenger.registerActionHandler(
'BackendWebSocketService:disconnect',
mockDisconnect,
'BackendWebSocketService:forceReconnection',
mockForceReconnection,
);
rootMessenger.registerActionHandler(
'BackendWebSocketService:subscribe',
Expand Down Expand Up @@ -123,7 +123,7 @@ const getMessenger = () => {
mocks: {
getSelectedAccount: mockGetSelectedAccount,
connect: mockConnect,
disconnect: mockDisconnect,
forceReconnection: mockForceReconnection,
subscribe: mockSubscribe,
channelHasSubscription: mockChannelHasSubscription,
getSubscriptionsByChannel: mockGetSubscriptionsByChannel,
Expand Down Expand Up @@ -222,7 +222,7 @@ type WithServiceCallback<ReturnValue> = (payload: {
mocks: {
getSelectedAccount: jest.Mock;
connect: jest.Mock;
disconnect: jest.Mock;
forceReconnection: jest.Mock;
subscribe: jest.Mock;
channelHasSubscription: jest.Mock;
getSubscriptionsByChannel: jest.Mock;
Expand Down Expand Up @@ -464,28 +464,22 @@ describe('AccountActivityService', () => {
);
});

it('should handle disconnect failures during force reconnection by logging error and continuing gracefully', async () => {
it('should handle subscription failure by calling forceReconnection', async () => {
await withService(async ({ service, mocks }) => {
// Mock disconnect to fail - this prevents the reconnect step from executing
mocks.disconnect.mockRejectedValue(
new Error('Disconnect failed during force reconnection'),
);

// Trigger scenario that causes force reconnection by making subscribe fail
// Mock subscribe to fail
mocks.subscribe.mockRejectedValue(new Error('Subscription failed'));

// Should handle both subscription failure and disconnect failure gracefully - should not throw
// Should handle subscription failure gracefully - should not throw
const result = await service.subscribe({ address: '0x123abc' });
expect(result).toBeUndefined();

// Verify the subscription was attempted
expect(mocks.subscribe).toHaveBeenCalledTimes(1);

// Verify disconnect was attempted (but failed, preventing reconnection)
expect(mocks.disconnect).toHaveBeenCalledTimes(1);
// Verify forceReconnection was called (lines 289-290)
expect(mocks.forceReconnection).toHaveBeenCalledTimes(1);

// Connect is only called once at the start because disconnect failed,
// so the reconnect step never executes (it's in the same try-catch block)
// Connect is only called once at the start
expect(mocks.connect).toHaveBeenCalledTimes(1);
});
});
Expand Down Expand Up @@ -536,14 +530,8 @@ describe('AccountActivityService', () => {
// unsubscribe catches errors and forces reconnection instead of throwing
await service.unsubscribe(mockSubscription);

// Should have attempted to force reconnection with exact sequence
expect(mocks.disconnect).toHaveBeenCalledTimes(1);
expect(mocks.connect).toHaveBeenCalledTimes(1);

// Verify disconnect was called before connect
const disconnectOrder = mocks.disconnect.mock.invocationCallOrder[0];
const connectOrder = mocks.connect.mock.invocationCallOrder[0];
expect(disconnectOrder).toBeLessThan(connectOrder);
// Should have attempted to force reconnection
expect(mocks.forceReconnection).toHaveBeenCalledTimes(1);
},
);
});
Expand Down
15 changes: 5 additions & 10 deletions packages/core-backend/src/AccountActivityService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export type AccountActivityServiceActions = AccountActivityServiceMethodActions;
export const ACCOUNT_ACTIVITY_SERVICE_ALLOWED_ACTIONS = [
'AccountsController:getSelectedAccount',
'BackendWebSocketService:connect',
'BackendWebSocketService:disconnect',
'BackendWebSocketService:forceReconnection',
'BackendWebSocketService:subscribe',
'BackendWebSocketService:getConnectionInfo',
'BackendWebSocketService:channelHasSubscription',
Expand Down Expand Up @@ -559,16 +559,11 @@ export class AccountActivityService {
* Force WebSocket reconnection to clean up subscription state
*/
async #forceReconnection(): Promise<void> {
try {
log('Forcing WebSocket reconnection to clean up subscription state');

// All subscriptions will be cleaned up automatically on WebSocket disconnect
log('Forcing WebSocket reconnection to clean up subscription state');

await this.#messenger.call('BackendWebSocketService:disconnect');
await this.#messenger.call('BackendWebSocketService:connect');
} catch (error) {
log('Failed to force WebSocket reconnection', { error });
}
// Use the dedicated forceReconnection method which performs a controlled
// disconnect-then-connect sequence to clean up subscription state
await this.#messenger.call('BackendWebSocketService:forceReconnection');
}

// =============================================================================
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,27 @@ export type BackendWebSocketServiceDisconnectAction = {
handler: BackendWebSocketService['disconnect'];
};

/**
* Forces a WebSocket reconnection to clean up subscription state
*
* This method is useful when subscription state may be out of sync and needs to be reset.
* It performs a controlled disconnect-then-reconnect sequence:
* - Disconnects cleanly to trigger subscription cleanup
* - Schedules reconnection with exponential backoff to prevent rapid loops
* - All subscriptions will be cleaned up automatically on disconnect
*
* Use cases:
* - Recovering from subscription/unsubscription issues
* - Cleaning up orphaned subscriptions
* - Forcing a fresh subscription state
*
* @returns Promise that resolves when disconnection is complete (reconnection is scheduled)
*/
export type BackendWebSocketServiceForceReconnectionAction = {
type: `BackendWebSocketService:forceReconnection`;
handler: BackendWebSocketService['forceReconnection'];
};

/**
* Sends a message through the WebSocket
*
Expand Down Expand Up @@ -159,6 +180,7 @@ export type BackendWebSocketServiceSubscribeAction = {
export type BackendWebSocketServiceMethodActions =
| BackendWebSocketServiceConnectAction
| BackendWebSocketServiceDisconnectAction
| BackendWebSocketServiceForceReconnectionAction
| BackendWebSocketServiceSendMessageAction
| BackendWebSocketServiceSendRequestAction
| BackendWebSocketServiceGetConnectionInfoAction
Expand Down
Loading
Loading