Skip to content

Commit 53fe1cb

Browse files
agangladaaganglada
committed
chore: check if the error is already a snap error
1 parent 4f793ae commit 53fe1cb

4 files changed

Lines changed: 93 additions & 29 deletions

File tree

‎packages/snap/snap.manifest.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"url": "https://github.com/MetaMask/snap-solana-wallet.git"
88
},
99
"source": {
10-
"shasum": "SNeUb5hAzMEExl6/sODXFY5l7FrC7cEFb2j2lL8PhGM=",
10+
"shasum": "4RjH2Fi3PLR31YtD0lKhmaWKUg15umuOdm+tEeD/DAU=",
1111
"location": {
1212
"npm": {
1313
"filePath": "dist/bundle.js",

‎packages/snap/src/core/utils/errors.test.ts‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { expect } from '@jest/globals';
22
import { SnapError } from '@metamask/snaps-sdk';
33

4-
import { handle } from './errors';
4+
import { withCatchAndThrowSnapError } from './errors';
55
import logger from './logger';
66

77
// Mock the logger to avoid actual console output during tests
@@ -21,7 +21,7 @@ describe('errors', () => {
2121
const mockFn = jest.fn().mockResolvedValue('success');
2222
const scope = 'test-scope';
2323

24-
const result = await handle(scope, mockFn);
24+
const result = await withCatchAndThrowSnapError(scope, mockFn);
2525

2626
expect(result).toBe('success');
2727
expect(mockFn).toHaveBeenCalledTimes(1);
@@ -33,7 +33,9 @@ describe('errors', () => {
3333
const mockFn = jest.fn().mockRejectedValue(originalError);
3434
const scope = 'test-scope';
3535

36-
await expect(handle(scope, mockFn)).rejects.toThrow(SnapError);
36+
await expect(withCatchAndThrowSnapError(scope, mockFn)).rejects.toThrow(
37+
SnapError,
38+
);
3739

3840
expect(mockFn).toHaveBeenCalledTimes(1);
3941
expect(mockLogger.error).toHaveBeenCalledTimes(1);
@@ -45,7 +47,7 @@ describe('errors', () => {
4547
const scope = 'test-scope';
4648

4749
try {
48-
await handle(scope, mockFn);
50+
await withCatchAndThrowSnapError(scope, mockFn);
4951
} catch (error) {
5052
// Expected to throw
5153
}
@@ -66,7 +68,9 @@ describe('errors', () => {
6668
const mockFn = jest.fn().mockRejectedValue(nonErrorValue);
6769
const scope = 'test-scope';
6870

69-
await expect(handle(scope, mockFn)).rejects.toThrow(SnapError);
71+
await expect(withCatchAndThrowSnapError(scope, mockFn)).rejects.toThrow(
72+
SnapError,
73+
);
7074

7175
expect(mockLogger.error).toHaveBeenCalledTimes(1);
7276
const logCall = mockLogger.error.mock.calls[0];
@@ -78,7 +82,9 @@ describe('errors', () => {
7882
const mockFn = jest.fn().mockRejectedValue(null);
7983
const scope = 'test-scope';
8084

81-
await expect(handle(scope, mockFn)).rejects.toThrow(SnapError);
85+
await expect(withCatchAndThrowSnapError(scope, mockFn)).rejects.toThrow(
86+
SnapError,
87+
);
8288

8389
expect(mockLogger.error).toHaveBeenCalledTimes(1);
8490
});
@@ -90,7 +96,7 @@ describe('errors', () => {
9096

9197
let caughtError: unknown;
9298
try {
93-
await handle(scope, mockFn);
99+
await withCatchAndThrowSnapError(scope, mockFn);
94100
} catch (error) {
95101
caughtError = error;
96102
}
@@ -113,7 +119,7 @@ describe('errors', () => {
113119
const mockFn = jest.fn().mockResolvedValue(testCase.value);
114120
const scope = 'test-scope';
115121

116-
const result = await handle(scope, mockFn);
122+
const result = await withCatchAndThrowSnapError(scope, mockFn);
117123

118124
expect(result).toBe(testCase.value);
119125
expect(mockLogger.error).not.toHaveBeenCalled();
@@ -132,7 +138,9 @@ describe('errors', () => {
132138
const mockFn = jest.fn().mockRejectedValue(errorType);
133139
const scope = 'test-scope';
134140

135-
await expect(handle(scope, mockFn)).rejects.toThrow(SnapError);
141+
await expect(withCatchAndThrowSnapError(scope, mockFn)).rejects.toThrow(
142+
SnapError,
143+
);
136144
}
137145

138146
expect(mockLogger.error).toHaveBeenCalledTimes(errorTypes.length);
@@ -154,7 +162,7 @@ describe('errors', () => {
154162
const scope = 'test-scope';
155163

156164
try {
157-
await handle(scope, mockFn);
165+
await withCatchAndThrowSnapError(scope, mockFn);
158166
} catch (error) {
159167
// Expected to throw
160168
}
@@ -170,7 +178,9 @@ describe('errors', () => {
170178
const mockFn = jest.fn().mockImplementation(async () => rejectedPromise);
171179
const scope = 'test-scope';
172180

173-
await expect(handle(scope, mockFn)).rejects.toThrow(SnapError);
181+
await expect(withCatchAndThrowSnapError(scope, mockFn)).rejects.toThrow(
182+
SnapError,
183+
);
174184

175185
expect(mockLogger.error).toHaveBeenCalledTimes(1);
176186
});

‎packages/snap/src/core/utils/errors.ts‎

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,65 @@
1-
import { SnapError } from '@metamask/snaps-sdk';
1+
import {
2+
MethodNotFoundError,
3+
ParseError,
4+
ResourceNotFoundError,
5+
ResourceUnavailableError,
6+
ChainDisconnectedError,
7+
TransactionRejected,
8+
DisconnectedError,
9+
InternalError,
10+
UnauthorizedError,
11+
UnsupportedMethodError,
12+
InvalidInputError,
13+
InvalidParamsError,
14+
InvalidRequestError,
15+
LimitExceededError,
16+
SnapError,
17+
MethodNotSupportedError,
18+
UserRejectedRequestError,
19+
} from '@metamask/snaps-sdk';
220

321
import logger from './logger';
422

5-
export const handle = async <ResponseT>(
23+
/**
24+
* Determines if the given error is a Snap RPC error.
25+
*
26+
* @param error - The error instance to be checked.
27+
* @returns A boolean indicating whether the error is a Snap RPC error.
28+
*/
29+
export function isSnapRpcError(error: Error): boolean {
30+
const errors = [
31+
SnapError,
32+
MethodNotFoundError,
33+
UserRejectedRequestError,
34+
MethodNotSupportedError,
35+
MethodNotFoundError,
36+
ParseError,
37+
ResourceNotFoundError,
38+
ResourceUnavailableError,
39+
TransactionRejected,
40+
ChainDisconnectedError,
41+
DisconnectedError,
42+
UnauthorizedError,
43+
UnsupportedMethodError,
44+
InternalError,
45+
InvalidInputError,
46+
InvalidParamsError,
47+
InvalidRequestError,
48+
LimitExceededError,
49+
];
50+
return errors.some((errType) => error instanceof errType);
51+
}
52+
53+
export const withCatchAndThrowSnapError = async <ResponseT>(
654
scope: string,
755
fn: () => Promise<ResponseT>,
856
): Promise<ResponseT> => {
957
try {
1058
return await fn();
1159
} catch (errorInstance: any) {
12-
const error = new SnapError(errorInstance as Error);
60+
const error = isSnapRpcError(errorInstance)
61+
? errorInstance
62+
: new SnapError(errorInstance);
1363

1464
logger.error(
1565
{ error },

‎packages/snap/src/index.ts‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { CronjobMethod } from './core/handlers/onCronjob/cronjobs/CronjobMethod'
2828
import { onProtocolRequest as onProtocolRequestHandler } from './core/handlers/onProtocolRequest/onProtocolRequest';
2929
import { handlers as onRpcRequestHandlers } from './core/handlers/onRpcRequest';
3030
import { RpcRequestMethod } from './core/handlers/onRpcRequest/types';
31-
import { handle } from './core/utils/errors';
31+
import { withCatchAndThrowSnapError } from './core/utils/errors';
3232
import { getClientStatus } from './core/utils/interface';
3333
import logger from './core/utils/logger';
3434
import { validateOrigin } from './core/validation/validators';
@@ -79,7 +79,7 @@ export const onRpcRequest: OnRpcRequestHandler = async ({
7979
) as unknown as Error;
8080
}
8181

82-
const result = await handle('onRpcRequest', async () =>
82+
const result = await withCatchAndThrowSnapError('onRpcRequest', async () =>
8383
handler({ origin, request }),
8484
);
8585

@@ -114,8 +114,9 @@ export const onKeyringRequest: OnKeyringRequestHandler = async ({
114114
(request.params as Record<string, Json>).origin = 'https://metamask.io';
115115
}
116116

117-
const result = await handle('onKeyringRequest', async () =>
118-
handleKeyringRequest(keyring, request),
117+
const result = await withCatchAndThrowSnapError(
118+
'onKeyringRequest',
119+
async () => handleKeyringRequest(keyring, request),
119120
);
120121

121122
return result ?? null;
@@ -157,7 +158,7 @@ export const onUserInput: OnUserInputHandler = async ({
157158
return;
158159
}
159160

160-
await handle('onUserInput', async () =>
161+
await withCatchAndThrowSnapError('onUserInput', async () =>
161162
handler({ id, event, context, snapContext }),
162163
);
163164
};
@@ -183,7 +184,7 @@ export const onCronjob: OnCronjobHandler = async ({ request }) => {
183184
]),
184185
);
185186

186-
const result = await handle('onCronjob', async () => {
187+
const result = await withCatchAndThrowSnapError('onCronjob', async () => {
187188
/**
188189
* Don't run cronjobs if client is locked or inactive
189190
* - We dont want to call cronjobs if the client is locked
@@ -231,37 +232,40 @@ export const onCronjob: OnCronjobHandler = async ({ request }) => {
231232
};
232233

233234
export const onAssetsLookup: OnAssetsLookupHandler = async (params) => {
234-
const result = await handle('onAssetsLookup', async () =>
235+
const result = await withCatchAndThrowSnapError('onAssetsLookup', async () =>
235236
onAssetsLookupHandler(params),
236237
);
237238
return result ?? null;
238239
};
239240

240241
export const onAssetsConversion: OnAssetsConversionHandler = async (params) => {
241-
const result = await handle('onAssetsConversion', async () =>
242-
onAssetsConversionHandler(params),
242+
const result = await withCatchAndThrowSnapError(
243+
'onAssetsConversion',
244+
async () => onAssetsConversionHandler(params),
243245
);
244246
return result ?? null;
245247
};
246248

247249
export const onProtocolRequest: OnProtocolRequestHandler = async (params) => {
248-
const result = await handle('onProtocolRequest', async () =>
249-
onProtocolRequestHandler(params),
250+
const result = await withCatchAndThrowSnapError(
251+
'onProtocolRequest',
252+
async () => onProtocolRequestHandler(params),
250253
);
251254
return result ?? null;
252255
};
253256

254257
export const onAssetHistoricalPrice: OnAssetHistoricalPriceHandler = async (
255258
params,
256259
) => {
257-
const result = await handle('onAssetHistoricalPrice', async () =>
258-
onAssetHistoricalPriceHandler(params),
260+
const result = await withCatchAndThrowSnapError(
261+
'onAssetHistoricalPrice',
262+
async () => onAssetHistoricalPriceHandler(params),
259263
);
260264
return result ?? null;
261265
};
262266

263267
export const onClientRequest: OnClientRequestHandler = async ({ request }) => {
264-
const result = await handle('onClientRequest', async () =>
268+
const result = await withCatchAndThrowSnapError('onClientRequest', async () =>
265269
clientRequestHandler.handle(request),
266270
);
267271
return result ?? null;

0 commit comments

Comments
 (0)