Skip to content

refactor: TransacationService and ClientRequest handler error handle #133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stanleyyconsensys merged 2 commits into from
Jun 25, 2026
Merged

refactor: TransacationService and ClientRequest handler error handle #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ac3232
refactor: TransacationService error handle
stanleyyconsensys Jun 24, 2026
15535ca
fix: comment
stanleyyconsensys Jun 24, 2026
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
4 changes: 1 addition & 3 deletions packages/snap/src/context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,7 @@ const changeTrustOptHandler = new ChangeTrustOptHandler({
confirmationUIController,
});

const onAddressInputHandler = new OnAddressInputHandler({
logger,
});
const onAddressInputHandler = new OnAddressInputHandler();

const onAmountInputHandler = new OnAmountInputHandler({
logger,
Expand Down
46 changes: 43 additions & 3 deletions packages/snap/src/handlers/clientRequest/confirmSend.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
InsufficientBalanceException,
InsufficientBalanceToCoverFeeException,
TransactionValidationException,
XdrParseException,
} from '../../services/transaction/exceptions';
import { KeyringTransactionType } from '../../services/transaction/KeyringTransactionBuilder';
import { WalletService } from '../../services/wallet';
Expand Down Expand Up @@ -522,13 +523,52 @@ describe('ConfirmSendHandler', () => {
});
});

it('rethrows unexpected errors from createValidatedSendTransaction', async () => {
it('returns invalid and tracks when createValidatedSendTransaction throws XdrParseException', async () => {
const { handler, createValidatedSendTransaction } = setup();
const xdrParseError = new XdrParseException(
'Invalid transfer function arguments',
);
createValidatedSendTransaction.mockRejectedValueOnce(xdrParseError);
const trackErrorSpy = jest
.spyOn(snapUtils, 'trackError')
.mockResolvedValue(undefined);

expect(await handler.handle(baseRequest())).toStrictEqual({
valid: false,
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
});
expect(trackErrorSpy).toHaveBeenCalledWith(xdrParseError);
});

it('returns invalid for unexpected errors from createValidatedSendTransaction', async () => {
const { handler, createValidatedSendTransaction } = setup();
const unexpectedError = new Error('unexpected');
createValidatedSendTransaction.mockRejectedValueOnce(unexpectedError);
const trackErrorSpy = jest
.spyOn(snapUtils, 'trackError')
.mockResolvedValue(undefined);

expect(await handler.handle(baseRequest())).toStrictEqual({
valid: false,
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
});
expect(trackErrorSpy).toHaveBeenCalledWith(unexpectedError);
});

it('does not track expected validation errors from createValidatedSendTransaction', async () => {
const { handler, createValidatedSendTransaction } = setup();
createValidatedSendTransaction.mockRejectedValueOnce(
new Error('unexpected'),
new TransactionValidationException('x'),
);
const trackErrorSpy = jest
.spyOn(snapUtils, 'trackError')
.mockResolvedValue(undefined);

await expect(handler.handle(baseRequest())).rejects.toThrow('unexpected');
expect(await handler.handle(baseRequest())).toStrictEqual({
valid: false,
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
});
expect(trackErrorSpy).not.toHaveBeenCalled();
});

it('continues successfully when saving pending transaction fails', async () => {
Expand Down
25 changes: 20 additions & 5 deletions packages/snap/src/handlers/clientRequest/confirmSend.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { ConfirmationInterfaceKey } from '../../ui/confirmation/api';
import {
hasDecimals,
toSmallestUnit,
trackError,
trackTransactionAdded,
trackTransactionApproved,
trackTransactionRejected,
Expand Down Expand Up @@ -214,10 +215,7 @@ export class ConfirmSendHandler extends BaseClientRequestHandler<
transactionId,
};
} catch (error: unknown) {
this.#logger.logErrorWithDetails(
'Failed to confirm send transaction',
error,
);
// Expected validation failures are user-facing outcomes; return them without tracking.
if (error instanceof InsufficientBalanceException) {
return {
valid: false,
Expand All @@ -241,7 +239,24 @@ export class ConfirmSendHandler extends BaseClientRequestHandler<
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
};
}
throw error;

// User rejection must propagate so MetaMask can dismiss the send flow.
if (error instanceof UserRejectedRequestError) {
throw error;
}

// Unexpected errors are swallowed into `{ valid: false }`, so track them for debugging.
await trackError(error);

this.#logger.warn(
'Failed to confirm send transaction due to unexpected issue',
{ error },
);

return {

@stanleyyconsensys stanleyyconsensys Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we dont need to throw error to caller, as it will catch the error and convert the same result if there is a error or code is invalid

it only special handle for UserRejectedRequestError

valid: false,
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
};
}
}

Expand Down
3 changes: 1 addition & 2 deletions packages/snap/src/handlers/clientRequest/onAddressInput.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@ import {
type OnAddressInputJsonRpcRequest,
} from './api';
import { OnAddressInputHandler } from './onAddressInput';
import { logger } from '../../utils/logger';

jest.mock('../../utils/logger');

const stellarAddress =
'GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN';

describe('OnAddressInputHandler', () => {
const handler = new OnAddressInputHandler({ logger });
const handler = new OnAddressInputHandler();

it('returns valid when the address passes validation', async () => {
const request: OnAddressInputJsonRpcRequest = {
Expand Down
18 changes: 4 additions & 14 deletions packages/snap/src/handlers/clientRequest/onAddressInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,9 @@ import {
OnAddressInputJsonRpcRequestStruct,
} from './api';
import type { IClientRequestHandler } from './base';
import type { ILogger } from '../../utils';
import { createPrefixedLogger, validateRequest } from '../../utils';
import { validateRequest } from '../../utils';

export class OnAddressInputHandler implements IClientRequestHandler {
readonly #logger: ILogger;

constructor({ logger }: { logger: ILogger }) {
const prefixedLogger = createPrefixedLogger(
logger,
'[📮 OnAddressInputHandler]',
);
this.#logger = prefixedLogger;
}

/**
* Handles the input of an address.
*
Expand All @@ -39,8 +28,9 @@ export class OnAddressInputHandler implements IClientRequestHandler {
valid: true,
errors: [],
};
} catch (error: unknown) {
this.#logger.logErrorWithDetails('Invalid address', error);
} catch {
// validateRequest will only throw InvalidParamsError,
// so we can safely ignore the error.
return {
valid: false,
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
Expand Down
20 changes: 18 additions & 2 deletions packages/snap/src/handlers/clientRequest/onAmountInput.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
InsufficientBalanceException,
InsufficientBalanceToCoverFeeException,
TransactionValidationException,
XdrParseException,
} from '../../services/transaction/exceptions';
import { WalletService } from '../../services/wallet';
import { getTestWallet } from '../../services/wallet/__mocks__/wallet.fixtures';
Expand Down Expand Up @@ -300,13 +301,28 @@ describe('OnAmountInputHandler', () => {
});
});

it('rethrows unexpected errors from createValidatedSendTransaction', async () => {
it('returns invalid when createValidatedSendTransaction throws XdrParseException', async () => {
const { handler, createValidatedSendTransaction } = setup();
createValidatedSendTransaction.mockRejectedValueOnce(
new XdrParseException('Invalid transfer function arguments'),
);

expect(await handler.handle(baseRequest())).toStrictEqual({
valid: false,
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
});
});

it('returns invalid for unexpected errors from createValidatedSendTransaction', async () => {
const { handler, createValidatedSendTransaction } = setup();
createValidatedSendTransaction.mockRejectedValueOnce(
new Error('unexpected'),
);

await expect(handler.handle(baseRequest())).rejects.toThrow('unexpected');
expect(await handler.handle(baseRequest())).toStrictEqual({
valid: false,
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
});
});

it('throws InvalidParamsError when the request fails struct validation', async () => {
Expand Down
29 changes: 10 additions & 19 deletions packages/snap/src/handlers/clientRequest/onAmountInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,11 @@ import {
} from './api';
import { BaseClientRequestHandler } from './base';
import type { AssetMetadataService } from '../../services/asset-metadata';
import { AccountNotActivatedException } from '../../services/network/exceptions';
import type { AccountNotActivatedException } from '../../services/network/exceptions';
import type { TransactionService } from '../../services/transaction';
import {
InsufficientBalanceException,
InsufficientBalanceToCoverFeeException,
TransactionValidationException,
} from '../../services/transaction/exceptions';
import { hasDecimals, toSmallestUnit } from '../../utils';
import type { ILogger } from '../../utils/logger';
Expand All @@ -31,8 +30,6 @@ export class OnAmountInputHandler extends BaseClientRequestHandler<
OnAmountInputJsonRpcRequest,
OnAmountInputJsonRpcResponse
> {
readonly #logger: ILogger;

readonly #assetMetadataService: AssetMetadataService;

readonly #transactionService: TransactionService;
Expand Down Expand Up @@ -61,7 +58,6 @@ export class OnAmountInputHandler extends BaseClientRequestHandler<
});
this.#assetMetadataService = assetMetadataService;
this.#transactionService = transactionService;
this.#logger = prefixedLogger;
}

/**
Expand Down Expand Up @@ -112,10 +108,10 @@ export class OnAmountInputHandler extends BaseClientRequestHandler<
errors: [],
};
} catch (error: unknown) {
this.#logger.logErrorWithDetails(
'Failed to validate amount input',
error,
);
// Called on every amount change while the user types. Return structured
// validation errors without tracking to reduce Sentry noise — insufficient
// balance is expected during entry. Other failures also return invalid
// without tracking; confirmSend tracks unexpected errors on submit.
if (error instanceof InsufficientBalanceException) {
return {
valid: false,
Expand All @@ -130,16 +126,11 @@ export class OnAmountInputHandler extends BaseClientRequestHandler<
],
};
}
if (
error instanceof TransactionValidationException ||
error instanceof AccountNotActivatedException
) {
return {
valid: false,
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
};
}
throw error;

return {
valid: false,
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
};
}
}

Expand Down
Loading
Loading