Skip to content

Commit 9ef5f0f

Browse files
fix: Improve executor error messaging (#3521)
Improve error messaging for execution service errors to be more granular and help indicate what the problem could be. Introduces a diagnostic execution status for each worker in the execution service which is used to determine the error message. --------- Co-authored-by: Maarten Zuidhoorn <[email protected]>
1 parent 71d4834 commit 9ef5f0f

File tree

7 files changed

+96
-7
lines changed

7 files changed

+96
-7
lines changed

packages/snaps-controllers/src/services/AbstractExecutionService.test.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,12 @@ describe('AbstractExecutionService', () => {
5858
`,
5959
endowments: ['console'],
6060
}),
61-
).rejects.toThrow('The Snaps execution environment failed to start.');
61+
).rejects.toThrow(
62+
'The executor for "npm:@metamask/example-snap" was unreachable. The executor did not respond in time.',
63+
);
6264
});
6365

64-
it('throws an error if execution environment fails to init', async () => {
66+
it('throws an error if execution environment fails to start initialization', async () => {
6567
const { service } = createService(MockExecutionService);
6668

6769
// @ts-expect-error Accessing private property and returning unusable worker.
@@ -78,7 +80,33 @@ describe('AbstractExecutionService', () => {
7880
`,
7981
endowments: ['console'],
8082
}),
81-
).rejects.toThrow('The Snaps execution environment failed to start.');
83+
).rejects.toThrow(
84+
`The executor for "npm:@metamask/example-snap" couldn't start initialization. The offscreen document may not exist.`,
85+
);
86+
});
87+
88+
it('throws an error if execution environment fails to init', async () => {
89+
const { service } = createService(MockExecutionService);
90+
91+
// @ts-expect-error Accessing private property and returning unusable worker.
92+
service.initEnvStream = async (snapId: string) =>
93+
new Promise((_resolve) => {
94+
// @ts-expect-error Accessing private property to mirror updating state
95+
service.setSnapStatus(snapId, 'initializing');
96+
// no-op
97+
});
98+
99+
await expect(
100+
service.executeSnap({
101+
snapId: MOCK_SNAP_ID,
102+
sourceCode: `
103+
console.log('foo');
104+
`,
105+
endowments: ['console'],
106+
}),
107+
).rejects.toThrow(
108+
'The executor for "npm:@metamask/example-snap" failed to initialize. The iframe/webview/worker failed to load.',
109+
);
82110
});
83111

84112
it('throws an error if Snap fails to init', async () => {

packages/snaps-controllers/src/services/AbstractExecutionService.ts

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,21 @@ export type Job<WorkerType> = {
6060
export type TerminateJobArgs<WorkerType> = Partial<Job<WorkerType>> &
6161
Pick<Job<WorkerType>, 'id'>;
6262

63+
/**
64+
Statuses used for diagnostic purposes
65+
- created: The initial state, no initialization has started
66+
- initializing: Snap execution environment is initializing
67+
- initialized: Snap execution environment has initialized
68+
- executing: Snap source code is being executed
69+
- running: Snap executed and ready for RPC requests
70+
*/
71+
type ExecutionStatus =
72+
| 'created'
73+
| 'initializing'
74+
| 'initialized'
75+
| 'executing'
76+
| 'running';
77+
6378
export abstract class AbstractExecutionService<WorkerType>
6479
implements ExecutionService
6580
{
@@ -69,6 +84,8 @@ export abstract class AbstractExecutionService<WorkerType>
6984

7085
readonly #jobs: Map<string, Job<WorkerType>>;
7186

87+
readonly #status: Map<string, ExecutionStatus>;
88+
7289
readonly #setupSnapProvider: SetupSnapProvider;
7390

7491
readonly #messenger: ExecutionServiceMessenger;
@@ -90,6 +107,7 @@ export abstract class AbstractExecutionService<WorkerType>
90107
usePing = true,
91108
}: ExecutionServiceArgs) {
92109
this.#jobs = new Map();
110+
this.#status = new Map();
93111
this.#setupSnapProvider = setupSnapProvider;
94112
this.#messenger = messenger;
95113
this.#initTimeout = initTimeout;
@@ -186,6 +204,7 @@ export abstract class AbstractExecutionService<WorkerType>
186204
this.terminateJob(job);
187205

188206
this.#jobs.delete(snapId);
207+
this.#status.delete(snapId);
189208
log(`Snap "${snapId}" terminated.`);
190209
}
191210

@@ -244,7 +263,17 @@ export abstract class AbstractExecutionService<WorkerType>
244263
if (result === hasTimedOut) {
245264
// For certain environments, such as the iframe we may have already created the worker and wish to terminate it.
246265
this.terminateJob({ id: snapId });
247-
throw new Error('The Snaps execution environment failed to start.');
266+
267+
const status = this.#status.get(snapId);
268+
if (status === 'created') {
269+
// Currently this error can only be thrown by OffscreenExecutionService.
270+
throw new Error(
271+
`The executor for "${snapId}" couldn't start initialization. The offscreen document may not exist.`,
272+
);
273+
}
274+
throw new Error(
275+
`The executor for "${snapId}" failed to initialize. The iframe/webview/worker failed to load.`,
276+
);
248277
}
249278

250279
const { worker, stream: envStream } = result;
@@ -307,6 +336,16 @@ export abstract class AbstractExecutionService<WorkerType>
307336
stream: BasePostMessageStream;
308337
}>;
309338

339+
/**
340+
* Set the execution status of the Snap.
341+
*
342+
* @param snapId - The Snap ID.
343+
* @param status - The current execution status.
344+
*/
345+
protected setSnapStatus(snapId: string, status: ExecutionStatus) {
346+
this.#status.set(snapId, status);
347+
}
348+
310349
async terminateAllSnaps() {
311350
await Promise.all(
312351
[...this.#jobs.keys()].map(async (snapId) => this.terminateSnap(snapId)),
@@ -332,6 +371,8 @@ export abstract class AbstractExecutionService<WorkerType>
332371
throw new Error(`"${snapId}" is already running.`);
333372
}
334373

374+
this.setSnapStatus(snapId, 'created');
375+
335376
const timer = new Timer(this.#initTimeout);
336377

337378
// This may resolve even if the environment has failed to start up fully
@@ -350,7 +391,9 @@ export abstract class AbstractExecutionService<WorkerType>
350391
);
351392

352393
if (pingResult === hasTimedOut) {
353-
throw new Error('The Snaps execution environment failed to start.');
394+
throw new Error(
395+
`The executor for "${snapId}" was unreachable. The executor did not respond in time.`,
396+
);
354397
}
355398
}
356399

@@ -362,6 +405,8 @@ export abstract class AbstractExecutionService<WorkerType>
362405
// Snap gets at least half the init timeout.
363406
const remainingTime = Math.max(timer.remaining, this.#initTimeout / 2);
364407

408+
this.setSnapStatus(snapId, 'initialized');
409+
365410
const request = {
366411
jsonrpc: '2.0',
367412
method: 'executeSnap',
@@ -371,6 +416,8 @@ export abstract class AbstractExecutionService<WorkerType>
371416

372417
assertIsJsonRpcRequest(request);
373418

419+
this.setSnapStatus(snapId, 'executing');
420+
374421
const result = await withTimeout(
375422
this.#command(job.id, request),
376423
remainingTime,
@@ -380,6 +427,10 @@ export abstract class AbstractExecutionService<WorkerType>
380427
throw new Error(`${snapId} failed to start.`);
381428
}
382429

430+
if (result === 'OK') {
431+
this.setSnapStatus(snapId, 'running');
432+
}
433+
383434
return result as string;
384435
}
385436

packages/snaps-controllers/src/services/iframe/IframeExecutionService.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ export class IframeExecutionService extends AbstractExecutionService<Window> {
3737
worker: Window;
3838
stream: BasePostMessageStream;
3939
}> {
40+
this.setSnapStatus(snapId, 'initializing');
41+
4042
const iframeWindow = await createWindow({
4143
uri: this.iframeUrl.toString(),
4244
id: snapId,

packages/snaps-controllers/src/services/node-js/NodeProcessExecutionService.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ import type { TerminateJobArgs } from '..';
77
import { AbstractExecutionService } from '..';
88

99
export class NodeProcessExecutionService extends AbstractExecutionService<ChildProcess> {
10-
protected async initEnvStream(): Promise<{
10+
protected async initEnvStream(snapId: string): Promise<{
1111
worker: ChildProcess;
1212
stream: BasePostMessageStream;
1313
}> {
14+
this.setSnapStatus(snapId, 'initializing');
15+
1416
const worker = fork(
1517
require.resolve('@metamask/snaps-execution-environments/node-process'),
1618
{

packages/snaps-controllers/src/services/node-js/NodeThreadExecutionService.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@ import type { TerminateJobArgs } from '..';
66
import { AbstractExecutionService } from '..';
77

88
export class NodeThreadExecutionService extends AbstractExecutionService<Worker> {
9-
protected async initEnvStream(): Promise<{
9+
protected async initEnvStream(snapId: string): Promise<{
1010
worker: Worker;
1111
stream: BasePostMessageStream;
1212
}> {
13+
this.setSnapStatus(snapId, 'initializing');
14+
1315
const worker = new Worker(
1416
require.resolve('@metamask/snaps-execution-environments/node-thread'),
1517
{

packages/snaps-controllers/src/services/proxy/ProxyExecutionService.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ export class ProxyExecutionService extends AbstractExecutionService<string> {
7171
* @returns An object with the worker ID and stream.
7272
*/
7373
protected async initEnvStream(snapId: string) {
74+
this.setSnapStatus(snapId, 'initializing');
75+
7476
const stream = new ProxyPostMessageStream({
7577
stream: this.#stream,
7678
jobId: snapId,

packages/snaps-controllers/src/services/webview/WebViewExecutionService.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ export class WebViewExecutionService extends AbstractExecutionService<WebViewInt
4040
* @returns An object with the webview and stream.
4141
*/
4242
protected async initEnvStream(snapId: string) {
43+
this.setSnapStatus(snapId, 'initializing');
44+
4345
const webView = await this.#createWebView(snapId);
4446

4547
const stream = new WebViewMessageStream({

0 commit comments

Comments
 (0)