Skip to content
2 changes: 2 additions & 0 deletions packages/multichain-account-service/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Batch account creation with single `keyring.addAccounts` call.
- Fetch all accounts in single `AccountsController:getAccounts` call instead of multiple `getAccount` calls.
- Significantly reduces lock acquisitions and API calls for batch operations.
- Do not report `TimeoutError` errors ([#8249](https://github.com/MetaMask/core/pull/8249))
- All other kind of errors are still reported as usual.

### Removed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type { EthKeyring } from '@metamask/keyring-internal-api';
import type { MultichainAccountServiceOptions } from './MultichainAccountService';
import { MultichainAccountService } from './MultichainAccountService';
import type { Bip44AccountProvider } from './providers';
import { TimeoutError } from './providers';
import { AccountProviderWrapper } from './providers/AccountProviderWrapper';
import {
EVM_ACCOUNT_PROVIDER_NAME,
Expand Down Expand Up @@ -1142,6 +1143,31 @@ describe('MultichainAccountService', () => {
providerError,
);
});

it('does not capture exception when provider throws a TimeoutError', async () => {
const rootMessenger = getRootMessenger();
const captureExceptionSpy = jest.spyOn(rootMessenger, 'captureException');
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();

const { service, mocks } = await setup({
rootMessenger,
accounts: [MOCK_HD_ACCOUNT_1],
});

mocks.SolAccountProvider.resyncAccounts.mockRejectedValue(
new TimeoutError('Timed out after: 500ms'),
);

await service.resyncAccounts(); // Should not throw.

expect(captureExceptionSpy).not.toHaveBeenCalled();
expect(consoleWarnSpy).toHaveBeenCalled();
expect(consoleErrorSpy).not.toHaveBeenCalled();

consoleWarnSpy.mockRestore();
consoleErrorSpy.mockRestore();
});
});

describe('setBasicFunctionality', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type { InternalAccount } from '@metamask/keyring-internal-api';
import { areUint8ArraysEqual, assert } from '@metamask/utils';

import { traceFallback } from './analytics';
import { reportError } from './errors';
import { projectLogger as log } from './logger';
import type { MultichainAccountGroup } from './MultichainAccountGroup';
import { MultichainAccountWallet } from './MultichainAccountWallet';
Expand All @@ -36,7 +37,6 @@ import type {
MultichainAccountServiceConfig,
MultichainAccountServiceMessenger,
} from './types';
import { createSentryError } from './utils';

export const serviceName = 'MultichainAccountService';

Expand Down Expand Up @@ -298,14 +298,14 @@ export class MultichainAccountService {
try {
await provider.resyncAccounts(accounts);
} catch (error) {
const errorMessage = `Unable to re-sync provider "${provider.getName()}"`;
log(errorMessage);
console.error(errorMessage);

const sentryError = createSentryError(errorMessage, error as Error, {
provider: provider.getName(),
});
this.#messenger.captureException?.(sentryError);
reportError(
this.#messenger,
`Unable to re-sync provider "${provider.getName()}"`,
error,
{
provider: provider.getName(),
},
);
}
}),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type { InternalAccount } from '@metamask/keyring-internal-api';

import type { WalletState } from './MultichainAccountWallet';
import { MultichainAccountWallet } from './MultichainAccountWallet';
import { TimeoutError } from './providers';
import type { MockAccountProvider, RootMessenger } from './tests';
import {
MOCK_HD_ACCOUNT_1,
Expand Down Expand Up @@ -307,6 +308,28 @@ describe('MultichainAccountWallet', () => {
);
});

it('does not capture exception when a provider times out creating accounts', async () => {
const groupIndex = 1;
const { wallet, providers, messenger } = setup({
accounts: [[MOCK_HD_ACCOUNT_1]],
});
const [provider] = providers;
provider.createAccounts.mockRejectedValueOnce(
new TimeoutError('Timed out after: 500ms'),
);
const captureExceptionSpy = jest.spyOn(messenger, 'captureException');
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
await expect(
wallet.createMultichainAccountGroup(groupIndex),
).rejects.toThrow('Timed out after: 500ms');
expect(captureExceptionSpy).not.toHaveBeenCalled();
expect(consoleWarnSpy).toHaveBeenCalled();
expect(consoleErrorSpy).not.toHaveBeenCalled();
consoleWarnSpy.mockRestore();
consoleErrorSpy.mockRestore();
});

it('defers non-EVM account creation to alignment after group creation (waitForAllProvidersToFinishCreatingAccounts = false)', async () => {
const groupIndex = 1;

Expand Down Expand Up @@ -601,6 +624,31 @@ describe('MultichainAccountWallet', () => {
);
});

it('does not capture exception when a provider times out creating accounts in batch', async () => {
const { wallet, providers, messenger } = setup({
accounts: [[]],
});

const [evmProvider] = providers;
evmProvider.createAccounts.mockRejectedValueOnce(
new TimeoutError('Timed out after: 500ms'),
);

const captureExceptionSpy = jest.spyOn(messenger, 'captureException');
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();

await expect(
wallet.createMultichainAccountGroups({ to: 2 }),
).rejects.toThrow('Timed out after: 500ms');

expect(captureExceptionSpy).not.toHaveBeenCalled();
expect(consoleWarnSpy).toHaveBeenCalled();
expect(consoleErrorSpy).not.toHaveBeenCalled();
consoleWarnSpy.mockRestore();
consoleErrorSpy.mockRestore();
});

it('creates accounts for all providers synchronously when waitForAllProvidersToFinishCreatingAccounts is true', async () => {
const { wallet, providers } = setup({
accounts: [[], []],
Expand Down Expand Up @@ -1062,8 +1110,11 @@ describe('MultichainAccountWallet', () => {

// Thrown provider should have been called once and not rescheduled
expect(providers[0].discoverAccounts).toHaveBeenCalledTimes(1);
expect(consoleSpy).toHaveBeenCalledWith(expect.any(Error));
expect((consoleSpy.mock.calls[0][0] as Error).message).toBe(
expect(consoleSpy).toHaveBeenCalledWith(
expect.any(String),
expect.any(Error),
);
expect((consoleSpy.mock.calls[0][1] as Error).message).toBe(
'Failed to discover accounts',
);

Expand All @@ -1082,12 +1133,33 @@ describe('MultichainAccountWallet', () => {
providers[1].discoverAccounts.mockResolvedValueOnce([]);
await wallet.discoverAccounts();
expect(captureExceptionSpy).toHaveBeenCalledWith(
new Error('Unable to discover accounts'),
new Error(
'Unable to discover accounts with provider "Mocked Provider 0"',
),
);
expect(captureExceptionSpy.mock.lastCall[0]).toHaveProperty(
'cause',
providerError,
);
});

it('does not capture exception when a provider times out during account discovery', async () => {
const { wallet, providers, messenger } = setup({
accounts: [[], []],
});
providers[0].discoverAccounts.mockRejectedValueOnce(
new TimeoutError('Timed out after: 500ms'),
);
const captureExceptionSpy = jest.spyOn(messenger, 'captureException');
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
providers[1].discoverAccounts.mockResolvedValueOnce([]);
await wallet.discoverAccounts();
expect(captureExceptionSpy).not.toHaveBeenCalled();
expect(consoleWarnSpy).toHaveBeenCalled();
expect(consoleErrorSpy).not.toHaveBeenCalled();
consoleWarnSpy.mockRestore();
consoleErrorSpy.mockRestore();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api';
import { assert } from '@metamask/utils';
import { Mutex } from 'async-mutex';

import { reportError } from './errors';
import type { Logger } from './logger';
import {
createModuleLogger,
Expand All @@ -33,7 +34,6 @@ import type { MultichainAccountServiceMessenger } from './types';
import {
assertGroupIndexIsValid,
assertGroupIndexRangeIsValid,
createSentryError,
GroupIndexRange,
toErrorMessage,
} from './utils';
Expand Down Expand Up @@ -243,29 +243,16 @@ export class MultichainAccountWallet<
},
});
} catch (error) {
const modeDescription = isBatching
? 'some accounts (batch)'
: 'some accounts';
const rangeDescription = isBatching
? `from group index ${from} to ${to}`
: `for group index ${to}`;

const errorMessage = `Unable to create ${modeDescription} ${rangeDescription} with provider "${provider.getName()}". Error: ${(error as Error).message}`;
this.#log(`${ERROR_PREFIX} ${errorMessage}:`, error);

const sentryError = createSentryError(
`Unable to create ${modeDescription} with provider "${provider.getName()}"`,
error as Error,
reportError(
this.#messenger,
`Unable to create ${isBatching ? 'some accounts (batch)' : 'some accounts'} with provider "${provider.getName()}"`,
error,
{
range: {
from,
to,
},
range: { from, to },
provider: provider.getName(),
isBatching,
},
);
this.#messenger.captureException?.(sentryError);
throw error;
}
}
Expand Down Expand Up @@ -751,23 +738,23 @@ export class MultichainAccountWallet<
});
} catch (error) {
context.stopped = true;
console.error(error);

log(
message(
`failed (with: "${(error as Error).message}")`,
`failed (with: "${toErrorMessage(error)}")`,
targetGroupIndex,
),
error,
);
const sentryError = createSentryError(
'Unable to discover accounts',
error as Error,

reportError(
this.#messenger,
`Unable to discover accounts with provider "${providerName}"`,
error,
{
provider: providerName,
groupIndex: targetGroupIndex,
},
);
this.#messenger.captureException?.(sentryError);
break;
}

Expand Down
33 changes: 33 additions & 0 deletions packages/multichain-account-service/src/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { logErrorAs } from './logger';
import { isTimeoutError } from './providers/utils';
import { createSentryError } from './utils';

/**
* Reports an error by logging it and optionally capturing it in Sentry.
*
* Timeout errors are treated as warnings (not reported to Sentry). All other
* errors are logged as errors and captured via `captureException`.
*
* @param messenger - Object with an optional `captureException` method.
* @param messenger.captureException - Optional method to capture exceptions in Sentry.
* @param message - The static message describing what failed.
* @param error - The caught error.
* @param context - Optional context to attach to the Sentry error.
*/
export function reportError(
messenger: { captureException?: (error: Error) => void },
message: string,
error: unknown,
context?: Record<string, unknown>,
): void {
if (isTimeoutError(error)) {
logErrorAs('warn', message, error);
console.warn(message, error);
} else {
logErrorAs('error', message, error);
console.error(message, error);

const sentryError = createSentryError(message, error as Error, context);
messenger.captureException?.(sentryError);
}
}
18 changes: 18 additions & 0 deletions packages/multichain-account-service/src/logger.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { createProjectLogger, createModuleLogger } from '@metamask/utils';

import { toErrorMessage } from './utils';

export const projectLogger = createProjectLogger('multichain-account-service');

export { createModuleLogger };
Expand All @@ -8,3 +10,19 @@ export const WARNING_PREFIX = 'WARNING --';
export const ERROR_PREFIX = 'ERROR --';

export type Logger = typeof projectLogger;

/**
* Logs an error with either WARNING or ERROR prefix, appending the error message.
*
* @param level - 'warn' for WARNING prefix, 'error' for ERROR prefix.
* @param message - The static message describing what failed.
* @param error - The caught error.
*/
export function logErrorAs(
level: 'warn' | 'error',
message: string,
error: unknown,
): void {
const prefix = level === 'warn' ? WARNING_PREFIX : ERROR_PREFIX;
projectLogger(`${prefix} ${message}: ${toErrorMessage(error)}`);
}
Loading
Loading