Skip to content

Commit cdb36fa

Browse files
refactor: Simplify RPC handling in SnapController (#3397)
Stop storing an RPC handler in the runtime for Snaps, instead just move the handler code directly to `handleRequest`. Also removes the `RequestQueue` previously used during the Snap starting up, instead we let as many requests as needed wait for `startPromise`. This simplifies RPC request routing significantly. Recommend reviewing this with whitespace changes not displayed 😄
1 parent b0b5264 commit cdb36fa

File tree

5 files changed

+81
-322
lines changed

5 files changed

+81
-322
lines changed
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"branches": 94.42,
3-
"functions": 98.4,
4-
"lines": 98.69,
5-
"statements": 98.52
2+
"branches": 94.97,
3+
"functions": 98.38,
4+
"lines": 98.76,
5+
"statements": 98.59
66
}

packages/snaps-controllers/src/snaps/RequestQueue.test.ts

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

packages/snaps-controllers/src/snaps/RequestQueue.ts

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

packages/snaps-controllers/src/snaps/SnapController.test.tsx

Lines changed: 0 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -4880,85 +4880,6 @@ describe('SnapController', () => {
48804880
snapController.destroy();
48814881
});
48824882

4883-
it('handlers will throw if there are too many pending requests before a snap has started', async () => {
4884-
const rootMessenger = getControllerMessenger();
4885-
const messenger = getSnapControllerMessenger(rootMessenger);
4886-
const fakeSnap = getPersistedSnapObject({ status: SnapStatus.Stopped });
4887-
const snapId = fakeSnap.id;
4888-
const snapController = getSnapController(
4889-
getSnapControllerOptions({
4890-
messenger,
4891-
state: {
4892-
snaps: {
4893-
[snapId]: fakeSnap,
4894-
},
4895-
},
4896-
}),
4897-
);
4898-
4899-
let resolveExecutePromise: any;
4900-
const deferredExecutePromise = new Promise((resolve) => {
4901-
resolveExecutePromise = resolve;
4902-
});
4903-
4904-
rootMessenger.registerActionHandler(
4905-
'ExecutionService:executeSnap',
4906-
async () => deferredExecutePromise,
4907-
);
4908-
4909-
// Fill up the request queue
4910-
const finishPromise = Promise.all(
4911-
new Array(100).fill(1).map(async (_, id) =>
4912-
snapController.handleRequest({
4913-
snapId,
4914-
origin: MOCK_ORIGIN,
4915-
handler: HandlerType.OnRpcRequest,
4916-
request: {
4917-
jsonrpc: '2.0',
4918-
method: 'bar',
4919-
params: {},
4920-
id,
4921-
},
4922-
}),
4923-
),
4924-
);
4925-
4926-
await expect(
4927-
snapController.handleRequest({
4928-
snapId,
4929-
origin: MOCK_ORIGIN,
4930-
handler: HandlerType.OnRpcRequest,
4931-
request: {
4932-
jsonrpc: '2.0',
4933-
method: 'bar',
4934-
params: {},
4935-
id: 100,
4936-
},
4937-
}),
4938-
).rejects.toThrow(
4939-
'Exceeds maximum number of requests waiting to be resolved, please try again.',
4940-
);
4941-
4942-
// Before processing the pending requests,
4943-
// we need an rpc message handler function to be returned
4944-
jest
4945-
.spyOn(messenger, 'call')
4946-
.mockImplementation(async (method, ..._args: unknown[]) => {
4947-
if (method === 'ExecutionService:executeSnap') {
4948-
return deferredExecutePromise;
4949-
} else if (method === 'ExecutionService:handleRpcRequest') {
4950-
return Promise.resolve(undefined);
4951-
}
4952-
return true;
4953-
});
4954-
4955-
// Resolve the promise that the pending requests are waiting for and wait for them to finish
4956-
resolveExecutePromise();
4957-
await finishPromise;
4958-
4959-
snapController.destroy();
4960-
});
4961-
49624883
it('crashes the Snap on unhandled errors', async () => {
49634884
const { manifest, sourceCode, svgIcon } =
49644885
await getMockSnapFilesWithUpdatedChecksum({

0 commit comments

Comments
 (0)