Skip to content

Commit 5c21b34

Browse files
fix: Destroying execution environment streams should emit events (#3074)
Previously we would remove event listeners when calling `destroy`. This meant that consumers of our streams would not be notified that the stream was closed causing issues like `Cannot call write after a stream was destroyed`. Instead, we want consumers to be notified when a stream is torn down. One issue however is that it introduces new issues where some code paths throw `Premature close`. This seems to be an underlying problem in the MetaMask stream architecture and the error message is [ignored on the extension client](https://github.com/MetaMask/metamask-extension/blob/bb90c361992d0381ba1852143d6c08d2ccb2fce1/app/scripts/lib/stream-utils.js#L14-L16). We do the same here. This will eventually also allow us to remove `closeConnections` from the SnapController constructor.
1 parent 6cea1bf commit 5c21b34

File tree

6 files changed

+12
-11
lines changed

6 files changed

+12
-11
lines changed

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,9 @@ export abstract class AbstractExecutionService<WorkerType>
193193

194194
Object.values(job.streams).forEach((stream) => {
195195
try {
196-
!stream.destroyed && stream.destroy();
197-
stream.removeAllListeners();
196+
if (!stream.destroyed) {
197+
stream.destroy();
198+
}
198199
} catch (error) {
199200
logError('Error while destroying stream', error);
200201
}
@@ -226,7 +227,7 @@ export abstract class AbstractExecutionService<WorkerType>
226227
streams.command,
227228
jsonRpcConnection.stream,
228229
(error) => {
229-
if (error) {
230+
if (error && !error.message?.match('Premature close')) {
230231
logError(`Command stream failure.`, error);
231232
}
232233
},
@@ -496,7 +497,7 @@ export function setupMultiplex(
496497
): ObjectMultiplex {
497498
const mux = new ObjectMultiplex();
498499
pipeline(connectionStream, mux, connectionStream, (error) => {
499-
if (error) {
500+
if (error && !error.message?.match('Premature close')) {
500501
logError(`"${streamName}" stream failure.`, error);
501502
}
502503
});

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1661,7 +1661,7 @@ describe('SnapController', () => {
16611661
engine.push(middleware);
16621662
const providerStream = createEngineStream({ engine });
16631663
pipeline(stream, providerStream, stream, (error) => {
1664-
if (error) {
1664+
if (error && !error.message?.match('Premature close')) {
16651665
logError(`Provider stream failure.`, error);
16661666
}
16671667
});
@@ -1736,7 +1736,7 @@ describe('SnapController', () => {
17361736
engine.push(middleware);
17371737
const providerStream = createEngineStream({ engine });
17381738
pipeline(stream, providerStream, stream, (error) => {
1739-
if (error) {
1739+
if (error && !error.message?.match('Premature close')) {
17401740
logError(`Provider stream failure.`, error);
17411741
}
17421742
});

packages/snaps-controllers/src/test-utils/execution-environment.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export const getNodeEES = (
5656
});
5757
const providerStream = createEngineStream({ engine });
5858
pipeline(stream, providerStream, stream, (error) => {
59-
if (error) {
59+
if (error && !error.message?.match('Premature close')) {
6060
logError(`Provider stream failure.`, error);
6161
}
6262
});

packages/snaps-controllers/src/test-utils/service.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export const createService = <
4444
});
4545
const providerStream = createEngineStream({ engine });
4646
pipeline(stream, providerStream, stream, (error) => {
47-
if (error) {
47+
if (error && !error.message?.match('Premature close')) {
4848
logError(`Provider stream failure.`, error);
4949
}
5050
});

packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ export class BaseSnapExecutor {
403403

404404
const multiplex = new ObjectMultiplex();
405405
pipeline(this.rpcStream, multiplex, this.rpcStream, (error) => {
406-
if (error) {
406+
if (error && !error.message?.match('Premature close')) {
407407
logError(`Provider stream failure.`, error);
408408
}
409409
});

packages/snaps-simulation/src/simulation.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,8 +393,8 @@ export async function installSnap<
393393

394394
// Error function is difficult to test, so we ignore it.
395395
/* istanbul ignore next 2 */
396-
pipeline(stream, providerStream, stream, (error: unknown) => {
397-
if (error) {
396+
pipeline(stream, providerStream, stream, (error) => {
397+
if (error && !error.message?.match('Premature close')) {
398398
logError(`Provider stream failure.`, error);
399399
}
400400
});

0 commit comments

Comments
 (0)