Skip to content

Commit f741fc8

Browse files
Remove dead code in snaps-rpc-methods (#2050)
Remove some dead code in snaps-rpc-methods and increase testing coverage of error cases. Also changes an error to throw using the proper JSON-RPC error code.
1 parent a0a0428 commit f741fc8

File tree

4 files changed

+85
-50
lines changed

4 files changed

+85
-50
lines changed

packages/snaps-rpc-methods/jest.config.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ module.exports = deepmerge(baseConfig, {
1010
],
1111
coverageThreshold: {
1212
global: {
13-
branches: 89.92,
13+
branches: 92.91,
1414
functions: 100,
15-
lines: 98.19,
16-
statements: 96.78,
15+
lines: 99.19,
16+
statements: 97.5,
1717
},
1818
},
1919
});

packages/snaps-rpc-methods/src/permitted/common/snapInstallation.ts

Lines changed: 0 additions & 41 deletions
This file was deleted.

packages/snaps-rpc-methods/src/permitted/requestSnaps.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,4 +447,76 @@ describe('implementation', () => {
447447
jsonrpc: '2.0',
448448
});
449449
});
450+
451+
it('throws if params is not an object', async () => {
452+
const { implementation } = requestSnapsHandler;
453+
454+
const hooks = getMockHooks();
455+
456+
const engine = new JsonRpcEngine();
457+
engine.push((req, res, next, end) => {
458+
const result = implementation(
459+
req as JsonRpcRequest<RequestSnapsParams>,
460+
res as PendingJsonRpcResponse<RequestSnapsResult>,
461+
next,
462+
end,
463+
hooks,
464+
);
465+
466+
result?.catch(end);
467+
});
468+
469+
const response = (await engine.handle({
470+
jsonrpc: '2.0',
471+
id: 1,
472+
method: 'wallet_requestSnaps',
473+
params: [],
474+
})) as JsonRpcSuccess<RequestSnapsResult>;
475+
476+
expect(response).toStrictEqual({
477+
error: {
478+
code: -32602,
479+
message: '"params" must be an object.',
480+
stack: expect.any(String),
481+
},
482+
id: 1,
483+
jsonrpc: '2.0',
484+
});
485+
});
486+
487+
it('throws if params is an empty object', async () => {
488+
const { implementation } = requestSnapsHandler;
489+
490+
const hooks = getMockHooks();
491+
492+
const engine = new JsonRpcEngine();
493+
engine.push((req, res, next, end) => {
494+
const result = implementation(
495+
req as JsonRpcRequest<RequestSnapsParams>,
496+
res as PendingJsonRpcResponse<RequestSnapsResult>,
497+
next,
498+
end,
499+
hooks,
500+
);
501+
502+
result?.catch(end);
503+
});
504+
505+
const response = (await engine.handle({
506+
jsonrpc: '2.0',
507+
id: 1,
508+
method: 'wallet_requestSnaps',
509+
params: {},
510+
})) as JsonRpcSuccess<RequestSnapsResult>;
511+
512+
expect(response).toStrictEqual({
513+
error: {
514+
code: -32602,
515+
message: 'Request must have at least one requested snap.',
516+
stack: expect.any(String),
517+
},
518+
id: 1,
519+
jsonrpc: '2.0',
520+
});
521+
});
450522
});

packages/snaps-rpc-methods/src/permitted/requestSnaps.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ import { hasProperty, isObject } from '@metamask/utils';
2424

2525
import { WALLET_SNAP_PERMISSION_KEY } from '../restricted/invokeSnap';
2626
import type { MethodHooksObject } from '../utils';
27-
import type { InstallSnapsHook } from './common/snapInstallation';
28-
import { handleInstallSnaps } from './common/snapInstallation';
2927

3028
const hookNames: MethodHooksObject<RequestSnapsHooks> = {
3129
installSnaps: true,
@@ -50,7 +48,9 @@ export type RequestSnapsHooks = {
5048
/**
5149
* Installs the requested snaps if they are permitted.
5250
*/
53-
installSnaps: InstallSnapsHook;
51+
installSnaps: (
52+
requestedSnaps: RequestSnapsParams,
53+
) => Promise<RequestSnapsResult>;
5454

5555
/**
5656
* Initiates a permission request for the requesting origin.
@@ -185,8 +185,12 @@ async function requestSnapsImplementation(
185185
}
186186

187187
try {
188-
if (!Object.keys(requestedSnaps).length) {
189-
throw new Error('Request must have at least one requested snap.');
188+
if (Object.keys(requestedSnaps).length === 0) {
189+
return end(
190+
rpcErrors.invalidParams({
191+
message: 'Request must have at least one requested snap.',
192+
}),
193+
);
190194
}
191195

192196
const requestedPermissions = {
@@ -202,7 +206,7 @@ async function requestSnapsImplementation(
202206
WALLET_SNAP_PERMISSION_KEY
203207
] as RequestSnapsResult;
204208
} else if (hasRequestedSnaps(existingPermissions, requestedSnaps)) {
205-
res.result = await handleInstallSnaps(requestedSnaps, installSnaps);
209+
res.result = await installSnaps(requestedSnaps);
206210
} else {
207211
const mergedPermissionsRequest = getSnapPermissionsRequest(
208212
existingPermissions,

0 commit comments

Comments
 (0)