From 54f688323815f0102861e703c9100e613273dc80 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Tue, 15 Oct 2024 11:43:10 -0500 Subject: [PATCH 01/12] refactor: init, delete -> launch, terminate --- .../extension/src/VatWorkerClient.test.ts | 10 +++--- packages/extension/src/VatWorkerClient.ts | 22 ++++++------- .../extension/src/VatWorkerServer.test.ts | 2 +- packages/extension/src/VatWorkerServer.ts | 18 +++++------ packages/extension/src/iframe-vat-worker.ts | 4 +-- .../extension/src/vat-worker-service.test.ts | 32 +++++++++---------- packages/extension/src/vat-worker-service.ts | 12 +++---- packages/extension/test/vat-worker-service.ts | 4 +-- packages/kernel/src/Kernel.test.ts | 20 ++++++------ packages/kernel/src/Kernel.ts | 4 +-- packages/kernel/src/types.ts | 4 +-- 11 files changed, 65 insertions(+), 67 deletions(-) diff --git a/packages/extension/src/VatWorkerClient.test.ts b/packages/extension/src/VatWorkerClient.test.ts index 1751be368..314db09e5 100644 --- a/packages/extension/src/VatWorkerClient.test.ts +++ b/packages/extension/src/VatWorkerClient.test.ts @@ -40,8 +40,8 @@ describe('ExtensionVatWorkerClient', () => { it.each` method - ${VatWorkerServiceMethod.Init} - ${VatWorkerServiceMethod.Delete} + ${VatWorkerServiceMethod.Launch} + ${VatWorkerServiceMethod.Terminate} `( "calls logger.error when receiving a $method reply it wasn't waiting for", async ({ method }) => { @@ -61,13 +61,13 @@ describe('ExtensionVatWorkerClient', () => { }, ); - it(`calls logger.error when receiving a ${VatWorkerServiceMethod.Init} reply without a port`, async () => { + it(`calls logger.error when receiving a ${VatWorkerServiceMethod.Launch} reply without a port`, async () => { const errorSpy = vi.spyOn(clientLogger, 'error'); const vatId: VatId = 'v0'; // eslint-disable-next-line @typescript-eslint/no-floating-promises - client.initWorker(vatId); + client.launch(vatId); const reply = { - method: VatWorkerServiceMethod.Init, + method: VatWorkerServiceMethod.Launch, id: 1, vatId: 'v0', }; diff --git a/packages/extension/src/VatWorkerClient.ts b/packages/extension/src/VatWorkerClient.ts index a37dcc689..1ecc57e4b 100644 --- a/packages/extension/src/VatWorkerClient.ts +++ b/packages/extension/src/VatWorkerClient.ts @@ -33,8 +33,8 @@ export class ExtensionVatWorkerClient implements VatWorkerService { /** * The client end of the vat worker service, intended to be constructed in - * the kernel worker. Sends initWorker and deleteWorker requests to the - * server and wraps the initWorker response in a DuplexStream for consumption + * the kernel worker. Sends launch and terminate worker requests to the + * server and wraps the launch response in a DuplexStream for consumption * by the kernel. * * @see {@link ExtensionVatWorkerServer} for the other end of the service. @@ -55,8 +55,8 @@ export class ExtensionVatWorkerClient implements VatWorkerService { async #sendMessage( method: - | typeof VatWorkerServiceMethod.Init - | typeof VatWorkerServiceMethod.Delete, + | typeof VatWorkerServiceMethod.Launch + | typeof VatWorkerServiceMethod.Terminate, vatId: VatId, ): Promise { const message = { @@ -73,14 +73,12 @@ export class ExtensionVatWorkerClient implements VatWorkerService { return promise; } - async initWorker( - vatId: VatId, - ): Promise> { - return this.#sendMessage(VatWorkerServiceMethod.Init, vatId); + async launch(vatId: VatId): Promise>{ + return this.#sendMessage(VatWorkerServiceMethod.Launch, vatId); } - async deleteWorker(vatId: VatId): Promise { - return this.#sendMessage(VatWorkerServiceMethod.Delete, vatId); + async terminate(vatId: VatId): Promise { + return this.#sendMessage(VatWorkerServiceMethod.Terminate, vatId); } async #handleMessage(event: MessageEvent): Promise { @@ -106,7 +104,7 @@ export class ExtensionVatWorkerClient implements VatWorkerService { } switch (method) { - case VatWorkerServiceMethod.Init: + case VatWorkerServiceMethod.Launch: if (!port) { this.#logger.error('Expected a port with message reply', event); return; @@ -117,7 +115,7 @@ export class ExtensionVatWorkerClient implements VatWorkerService { ), ); break; - case VatWorkerServiceMethod.Delete: + case VatWorkerServiceMethod.Terminate: // If we were caching streams on the client this would be a good place // to remove them. promise.resolve(undefined); diff --git a/packages/extension/src/VatWorkerServer.test.ts b/packages/extension/src/VatWorkerServer.test.ts index 24fd14748..f2805081e 100644 --- a/packages/extension/src/VatWorkerServer.test.ts +++ b/packages/extension/src/VatWorkerServer.test.ts @@ -7,7 +7,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import type { ExtensionVatWorkerServer } from './VatWorkerServer.js'; import { makeTestServer } from '../test/vat-worker-service.js'; -describe('VatWorker', () => { +describe('ExtensionVatWorkerServer', () => { let serverPort: MessagePort; let clientPort: MessagePort; diff --git a/packages/extension/src/VatWorkerServer.ts b/packages/extension/src/VatWorkerServer.ts index 4809c6ab4..619c2302a 100644 --- a/packages/extension/src/VatWorkerServer.ts +++ b/packages/extension/src/VatWorkerServer.ts @@ -30,7 +30,7 @@ export class ExtensionVatWorkerServer { /** * The server end of the vat worker service, intended to be constructed in - * the offscreen document. Listens for initWorker and deleteWorker requests + * the offscreen document. Listens for launch and terminate worker requests * from the client and uses the {@link VatWorker} methods to effect those * requests. * @@ -79,13 +79,13 @@ export class ExtensionVatWorkerServer { }; switch (method) { - case VatWorkerServiceMethod.Init: - await this.#initVatWorker(vatId) + case VatWorkerServiceMethod.Launch: + await this.#launch(vatId) .then((port) => this.#postMessage({ method, id, vatId }, [port])) .catch(handleProblem); break; - case VatWorkerServiceMethod.Delete: - await this.#deleteVatWorker(vatId) + case VatWorkerServiceMethod.Terminate: + await this.#terminate(vatId) .then(() => this.#postMessage({ method, id, vatId })) .catch(handleProblem); break; @@ -99,22 +99,22 @@ export class ExtensionVatWorkerServer { } } - async #initVatWorker(vatId: VatId): Promise { + async #launch(vatId: VatId): Promise { if (this.#vatWorkers.has(vatId)) { throw new Error(`Worker for vat ${vatId} already exists.`); } const vatWorker = this.#makeWorker(vatId); - const [port] = await vatWorker.init(); + const [port] = await vatWorker.launch(); this.#vatWorkers.set(vatId, vatWorker); return port; } - async #deleteVatWorker(vatId: VatId): Promise { + async #terminate(vatId: VatId): Promise { const vatWorker = this.#vatWorkers.get(vatId); if (!vatWorker) { throw new Error(`Worker for vat ${vatId} does not exist.`); } - await vatWorker.delete(); + await vatWorker.terminate(); this.#vatWorkers.delete(vatId); } } diff --git a/packages/extension/src/iframe-vat-worker.ts b/packages/extension/src/iframe-vat-worker.ts index 400e149e6..ad1278631 100644 --- a/packages/extension/src/iframe-vat-worker.ts +++ b/packages/extension/src/iframe-vat-worker.ts @@ -12,7 +12,7 @@ export const makeIframeVatWorker = ( ): VatWorker => { const vatHtmlId = `ocap-iframe-${id}`; return { - init: async () => { + launch: async () => { const newWindow = await createWindow({ uri: IFRAME_URI, id: vatHtmlId, @@ -24,7 +24,7 @@ export const makeIframeVatWorker = ( return [port, newWindow]; }, - delete: async (): Promise => { + terminate: async (): Promise => { const iframe = document.getElementById(vatHtmlId); /* v8 ignore next 6: Not known to be possible. */ if (iframe === null) { diff --git a/packages/extension/src/vat-worker-service.test.ts b/packages/extension/src/vat-worker-service.test.ts index 96ae4fe1f..579d6a1bd 100644 --- a/packages/extension/src/vat-worker-service.test.ts +++ b/packages/extension/src/vat-worker-service.test.ts @@ -26,8 +26,8 @@ describe('VatWorker', () => { let mockWorker: VatWorker; let mockMakeWorker: (vatId: VatId) => VatWorker; - let mockInitWorker: MockInstance; - let mockDeleteWorker: MockInstance; + let mockLaunchWorker: MockInstance; + let mockTerminateWorker: MockInstance; beforeEach(() => { const serviceMessageChannel = new MessageChannel(); @@ -40,8 +40,8 @@ describe('VatWorker', () => { [mockWorker, mockMakeWorker] = getMockMakeWorker(kernelPort); - mockInitWorker = vi.spyOn(mockWorker, 'init'); - mockDeleteWorker = vi.spyOn(mockWorker, 'delete'); + mockLaunchWorker = vi.spyOn(mockWorker, 'launch'); + mockTerminateWorker = vi.spyOn(mockWorker, 'terminate'); }); // low key integration test @@ -52,28 +52,28 @@ describe('VatWorker', () => { server.start(); }); - it('initializes and deletes a worker', async () => { + it('launches and terminates a worker', async () => { const vatId: VatId = 'v0'; - const stream = await client.initWorker(vatId); + const stream = await client.launch(vatId); expect(stream).toBeInstanceOf(MessagePortDuplexStream); - expect(mockInitWorker).toHaveBeenCalledOnce(); - expect(mockDeleteWorker).not.toHaveBeenCalled(); + expect(mockLaunchWorker).toHaveBeenCalledOnce(); + expect(mockTerminateWorker).not.toHaveBeenCalled(); - await client.deleteWorker(vatId); - expect(mockInitWorker).toHaveBeenCalledOnce(); - expect(mockDeleteWorker).toHaveBeenCalledOnce(); + await client.terminate(vatId); + expect(mockLaunchWorker).toHaveBeenCalledOnce(); + expect(mockTerminateWorker).toHaveBeenCalledOnce(); }); - it('throws when deleting a nonexistent worker', async () => { - await expect(async () => await client.deleteWorker('v0')).rejects.toThrow( + it('throws when terminating a nonexistent worker', async () => { + await expect(async () => await client.terminate('v0')).rejects.toThrow( /vat v0 does not exist/u, ); }); - it('throws when initializing the same worker twice', async () => { + it('throws when launching the same worker twice', async () => { const vatId: VatId = 'v0'; - await client.initWorker(vatId); - await expect(async () => await client.initWorker(vatId)).rejects.toThrow( + await client.launch(vatId); + await expect(async () => await client.launch(vatId)).rejects.toThrow( /vat v0 already exists/u, ); }); diff --git a/packages/extension/src/vat-worker-service.ts b/packages/extension/src/vat-worker-service.ts index 6e5a6c329..aaa9e3d68 100644 --- a/packages/extension/src/vat-worker-service.ts +++ b/packages/extension/src/vat-worker-service.ts @@ -2,21 +2,21 @@ import { isObject } from '@metamask/utils'; import type { VatId } from '@ocap/kernel'; export enum VatWorkerServiceMethod { - Init = 'iframe-vat-worker-init', - Delete = 'iframe-vat-worker-delete', + Launch = 'iframe-vat-worker-launch', + Terminate = 'iframe-vat-worker-terminate', } type MessageId = number; export type VatWorker = { - init: () => Promise<[MessagePort, unknown]>; - delete: () => Promise; + launch: () => Promise<[MessagePort, unknown]>; + terminate: () => Promise; }; export type VatWorkerServiceMessage = { method: - | typeof VatWorkerServiceMethod.Init - | typeof VatWorkerServiceMethod.Delete; + | typeof VatWorkerServiceMethod.Launch + | typeof VatWorkerServiceMethod.Terminate; id: MessageId; vatId: VatId; error?: Error; diff --git a/packages/extension/test/vat-worker-service.ts b/packages/extension/test/vat-worker-service.ts index 434244957..36e5f110d 100644 --- a/packages/extension/test/vat-worker-service.ts +++ b/packages/extension/test/vat-worker-service.ts @@ -12,8 +12,8 @@ export const getMockMakeWorker = ( kernelPort: MessagePort, ): [VatWorker, MakeVatWorker] => { const mockWorker = { - init: vi.fn().mockResolvedValue([kernelPort, {}]), - delete: vi.fn().mockResolvedValue(undefined), + launch: vi.fn().mockResolvedValue([kernelPort, {}]), + terminate: vi.fn().mockResolvedValue(undefined), }; return [mockWorker, vi.fn().mockReturnValue(mockWorker)]; diff --git a/packages/kernel/src/Kernel.test.ts b/packages/kernel/src/Kernel.test.ts index 18080d4d7..12136d1b2 100644 --- a/packages/kernel/src/Kernel.test.ts +++ b/packages/kernel/src/Kernel.test.ts @@ -20,8 +20,8 @@ import { makeMapKernelStore } from '../test/storage.js'; describe('Kernel', () => { let mockStream: DuplexStream; let mockWorkerService: VatWorkerService; - let mockGetWorkerStreams: MockInstance; - let mockDeleteWorker: MockInstance; + let launchWorkerMock: MockInstance; + let terminateWorkerMock: MockInstance; let initMock: MockInstance; let terminateMock: MockInstance; @@ -38,17 +38,17 @@ describe('Kernel', () => { } as unknown as MessagePortDuplexStream; mockWorkerService = { - initWorker: async () => ({}), - deleteWorker: async () => undefined, + launch: async () => ({}), + terminate: async () => undefined, } as unknown as VatWorkerService; - mockGetWorkerStreams = vi - .spyOn(mockWorkerService, 'initWorker') + launchWorkerMock = vi + .spyOn(mockWorkerService, 'launch') .mockResolvedValue( {} as DuplexStream, ); - mockDeleteWorker = vi - .spyOn(mockWorkerService, 'deleteWorker') + terminateWorkerMock = vi + .spyOn(mockWorkerService, 'terminate') .mockResolvedValue(undefined); initMock = vi.spyOn(Vat.prototype, 'init').mockImplementation(vi.fn()); @@ -84,7 +84,7 @@ describe('Kernel', () => { const kernel = new Kernel(mockStream, mockWorkerService, mockKernelStore); await kernel.launchVat({ id: 'v0' }); expect(initMock).toHaveBeenCalledOnce(); - expect(mockGetWorkerStreams).toHaveBeenCalled(); + expect(launchWorkerMock).toHaveBeenCalled(); expect(kernel.getVatIds()).toStrictEqual(['v0']); }); @@ -108,7 +108,7 @@ describe('Kernel', () => { expect(kernel.getVatIds()).toStrictEqual(['v0']); await kernel.deleteVat('v0'); expect(terminateMock).toHaveBeenCalledOnce(); - expect(mockDeleteWorker).toHaveBeenCalledOnce(); + expect(terminateWorkerMock).toHaveBeenCalledOnce(); expect(kernel.getVatIds()).toStrictEqual([]); }); diff --git a/packages/kernel/src/Kernel.ts b/packages/kernel/src/Kernel.ts index 1839222a4..418b9be97 100644 --- a/packages/kernel/src/Kernel.ts +++ b/packages/kernel/src/Kernel.ts @@ -181,7 +181,7 @@ export class Kernel { if (this.#vats.has(id)) { throw new VatAlreadyExistsError(id); } - const stream = await this.#vatWorkerService.initWorker(id); + const stream = await this.#vatWorkerService.launch(id); const vat = new Vat({ id, stream }); this.#vats.set(vat.id, vat); await vat.init(); @@ -196,7 +196,7 @@ export class Kernel { async deleteVat(id: VatId): Promise { const vat = this.#getVat(id); await vat.terminate(); - await this.#vatWorkerService.deleteWorker(id).catch(console.error); + await this.#vatWorkerService.terminate(id).catch(console.error); this.#vats.delete(id); } diff --git a/packages/kernel/src/types.ts b/packages/kernel/src/types.ts index e25a8e894..f489749e6 100644 --- a/packages/kernel/src/types.ts +++ b/packages/kernel/src/types.ts @@ -16,8 +16,8 @@ export type PromiseCallbacks = Omit< >; export type VatWorkerService = { - initWorker: ( + launch: ( vatId: VatId, ) => Promise>; - deleteWorker: (vatId: VatId) => Promise; + terminate: (vatId: VatId) => Promise; }; From 0fe442626f75ba5fdc497ddb1fa3c4d58047d232 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Wed, 16 Oct 2024 14:17:55 -0500 Subject: [PATCH 02/12] feat(kernel): Add generic identified message kit. --- packages/kernel/src/Supervisor.ts | 9 +--- packages/kernel/src/Vat.ts | 6 +-- packages/kernel/src/messages.ts | 6 +-- packages/kernel/src/messages/message-kit.ts | 47 +++++++++++++++++-- .../kernel/src/messages/vat-message.test.ts | 18 ------- packages/kernel/src/messages/vat-message.ts | 23 --------- packages/kernel/src/messages/vat.test.ts | 17 +++++++ packages/kernel/src/messages/vat.ts | 31 ++++++++---- 8 files changed, 88 insertions(+), 69 deletions(-) delete mode 100644 packages/kernel/src/messages/vat-message.test.ts delete mode 100644 packages/kernel/src/messages/vat-message.ts create mode 100644 packages/kernel/src/messages/vat.test.ts diff --git a/packages/kernel/src/Supervisor.ts b/packages/kernel/src/Supervisor.ts index f50039a88..768853f2e 100644 --- a/packages/kernel/src/Supervisor.ts +++ b/packages/kernel/src/Supervisor.ts @@ -3,12 +3,7 @@ import { StreamReadError } from '@ocap/errors'; import type { DuplexStream } from '@ocap/streams'; import { stringify } from '@ocap/utils'; -import type { - CapTpMessage, - VatCommand, - VatCommandReply, - VatMessageId, -} from './messages.js'; +import type { CapTpMessage, VatCommand, VatCommandReply } from './messages.js'; import { VatCommandMethod } from './messages.js'; import type { StreamEnvelope, StreamEnvelopeReply } from './stream-envelope.js'; import { @@ -127,7 +122,7 @@ export class Supervisor { * @param payload - The payload to reply with. */ async replyToMessage( - id: VatMessageId, + id: VatCommand['id'], payload: VatCommandReply['payload'], ): Promise { await this.#stream.write(wrapStreamCommandReply({ id, payload })); diff --git a/packages/kernel/src/Vat.ts b/packages/kernel/src/Vat.ts index 2514036e3..e4185c752 100644 --- a/packages/kernel/src/Vat.ts +++ b/packages/kernel/src/Vat.ts @@ -17,7 +17,6 @@ import type { CapTpPayload, VatCommandReply, VatCommand, - VatMessageId, } from './messages.js'; import type { StreamEnvelope, @@ -45,7 +44,8 @@ export class Vat { readonly #messageCounter: () => number; - readonly unresolvedMessages: Map = new Map(); + readonly unresolvedMessages: Map = + new Map(); readonly streamEnvelopeReplyHandler: StreamEnvelopeReplyHandler; @@ -185,7 +185,7 @@ export class Vat { * * @returns The message ID. */ - readonly #nextMessageId = (): VatMessageId => { + readonly #nextMessageId = (): VatCommand['id'] => { return `${this.id}:${this.#messageCounter()}`; }; } diff --git a/packages/kernel/src/messages.ts b/packages/kernel/src/messages.ts index d32528ae3..867201811 100644 --- a/packages/kernel/src/messages.ts +++ b/packages/kernel/src/messages.ts @@ -1,8 +1,4 @@ -// Base messages. - -export type { VatMessageId } from './messages/vat-message.js'; - -// CapTP. +// CapTp. export { isCapTpPayload, isCapTpMessage } from './messages/captp.js'; export type { CapTpPayload, CapTpMessage } from './messages/captp.js'; diff --git a/packages/kernel/src/messages/message-kit.ts b/packages/kernel/src/messages/message-kit.ts index 1e631d408..81b83fa8d 100644 --- a/packages/kernel/src/messages/message-kit.ts +++ b/packages/kernel/src/messages/message-kit.ts @@ -1,6 +1,6 @@ import '@ocap/shims/endoify'; -import type { Json } from '@metamask/utils'; +import { isObject, type Json } from '@metamask/utils'; import type { ExtractGuardType, TypeGuard } from '@ocap/utils'; import { isMessageLike } from './message.js'; @@ -8,9 +8,9 @@ import { uncapitalize } from './utils.js'; // Message kit. -type BoolExpr = (value: unknown) => boolean; +export type BoolExpr = (value: unknown) => boolean; -type SourceLike = Record; +export type SourceLike = Record; type MessageUnion = { [Key in keyof Source]: Key extends string @@ -95,3 +95,44 @@ export const makeMessageKit = ( replyGuard: makeGuard(source, methods, 1), } as MessageKit; }; + +const makeIsIdentified = + ( + isId: TypeGuard, + isPayload: TypeGuard, + ) => + (value: unknown): value is { id: Identifier; payload: Payload } => + isObject(value) && isId(value.id) && isPayload(value.payload); + +/** + * An object type encapsulating all of the schematics that define a functional + * group of messages as a payload wrapped with a message id. + */ +type IdentifiedMessageKit< + Source extends SourceLike, + MessageId extends string, +> = { + source: Source; + methods: Methods; + send: { id: MessageId; payload: Send }; + sendGuard: TypeGuard<{ id: MessageId; payload: Send }>; + reply: { id: MessageId; payload: Reply }; + replyGuard: TypeGuard<{ id: MessageId; payload: Reply }>; +}; + +export const makeIdentifiedMessageKit = < + Source extends SourceLike, + MessageId extends string, +>( + source: Source, + isMessageId: TypeGuard, +): IdentifiedMessageKit => { + const messageKit = makeMessageKit(source); + + return { + source: messageKit.source, + methods: messageKit.methods, + sendGuard: makeIsIdentified(isMessageId, messageKit.sendGuard), + replyGuard: makeIsIdentified(isMessageId, messageKit.replyGuard), + } as IdentifiedMessageKit; +}; diff --git a/packages/kernel/src/messages/vat-message.test.ts b/packages/kernel/src/messages/vat-message.test.ts deleted file mode 100644 index 4c3d85810..000000000 --- a/packages/kernel/src/messages/vat-message.test.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { describe, expect, it } from 'vitest'; - -import { isVatMessage } from './vat-message.js'; -import { VatCommandMethod } from './vat.js'; - -describe('isVatMessage', () => { - const validPayload = { method: VatCommandMethod.Evaluate, params: '3 + 3' }; - - it.each` - value | expectedResult | description - ${{ id: 'v0:1', payload: validPayload }} | ${true} | ${'valid message id with valid payload'} - ${{ id: 'vat-message-id', payload: validPayload }} | ${false} | ${'invalid id'} - ${{ id: 1, payload: validPayload }} | ${false} | ${'numerical id'} - ${{ id: 'v0:1' }} | ${false} | ${'missing payload'} - `('returns $expectedResult for $description', ({ value, expectedResult }) => { - expect(isVatMessage(value)).toBe(expectedResult); - }); -}); diff --git a/packages/kernel/src/messages/vat-message.ts b/packages/kernel/src/messages/vat-message.ts deleted file mode 100644 index 6e3562ecc..000000000 --- a/packages/kernel/src/messages/vat-message.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { hasProperty, isObject } from '@metamask/utils'; - -import type { VatId } from '../types.js'; -import { isVatId } from '../types.js'; - -export type VatMessageId = `${VatId}:${number}`; - -export const isVatMessageId = (value: unknown): value is VatMessageId => { - if (typeof value !== 'string') { - return false; - } - const parts = value.split(':'); - return ( - parts.length === 2 && - isVatId(parts[0]) && - parts[1] === String(Number(parts[1])) - ); -}; - -export type VatMessage = { id: VatMessageId; payload: Payload }; - -export const isVatMessage = (value: unknown): value is VatMessage => - isObject(value) && isVatMessageId(value.id) && hasProperty(value, 'payload'); diff --git a/packages/kernel/src/messages/vat.test.ts b/packages/kernel/src/messages/vat.test.ts new file mode 100644 index 000000000..8b28c63df --- /dev/null +++ b/packages/kernel/src/messages/vat.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from 'vitest'; + +import { isVatCommand, VatCommandMethod } from './vat.js'; + +describe('isVatCommand', () => { + const payload = { method: VatCommandMethod.Evaluate, params: '3 + 3' }; + + it.each` + value | expectedResult | description + ${{ id: 'v0:1', payload }} | ${true} | ${'valid message id with valid payload'} + ${{ id: 'vat-message-id', payload }} | ${false} | ${'invalid id'} + ${{ id: 1, payload }} | ${false} | ${'numerical id'} + ${{ id: 'v0:1' }} | ${false} | ${'missing payload'} + `('returns $expectedResult for $description', ({ value, expectedResult }) => { + expect(isVatCommand(value)).toBe(expectedResult); + }); +}); diff --git a/packages/kernel/src/messages/vat.ts b/packages/kernel/src/messages/vat.ts index 77eb99866..5611941e9 100644 --- a/packages/kernel/src/messages/vat.ts +++ b/packages/kernel/src/messages/vat.ts @@ -1,7 +1,7 @@ -import { makeMessageKit, messageType } from './message-kit.js'; -import type { VatMessage } from './vat-message.js'; -import { isVatMessage } from './vat-message.js'; +import { makeIdentifiedMessageKit, messageType } from './message-kit.js'; import { vatTestCommand } from './vat-test.js'; +import { isVatId } from '../types.js'; +import type { VatId } from '../types.js'; export const vatCommand = { CapTpInit: messageType( @@ -12,14 +12,25 @@ export const vatCommand = { ...vatTestCommand, }; -const vatMessageKit = makeMessageKit(vatCommand); +const vatMessageKit = makeIdentifiedMessageKit( + vatCommand, + (value: unknown): value is `${VatId}:${number}` => { + if (typeof value !== 'string') { + return false; + } + const parts = value.split(':'); + return ( + parts.length === 2 && + isVatId(parts[0]) && + parts[1] === String(Number(parts[1])) + ); + }, +); export const VatCommandMethod = vatMessageKit.methods; -export type VatCommand = VatMessage; -export const isVatCommand = (value: unknown): value is VatCommand => - isVatMessage(value) && vatMessageKit.sendGuard(value.payload); +export type VatCommand = typeof vatMessageKit.send; +export const isVatCommand = vatMessageKit.sendGuard; -export type VatCommandReply = VatMessage; -export const isVatCommandReply = (value: unknown): value is VatCommandReply => - isVatMessage(value) && vatMessageKit.replyGuard(value.payload); +export type VatCommandReply = typeof vatMessageKit.reply; +export const isVatCommandReply = vatMessageKit.replyGuard; From 02d5c200e3e2c155baf98c80a5e59d90e606c9cd Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Wed, 16 Oct 2024 14:20:11 -0500 Subject: [PATCH 03/12] feat(kernel): Add VatWorkerService terminateAll method. --- packages/extension/package.json | 1 + .../extension/src/VatWorkerClient.test.ts | 28 ++-- packages/extension/src/VatWorkerClient.ts | 68 +++++---- .../extension/src/VatWorkerServer.test.ts | 91 ++++++++--- packages/extension/src/VatWorkerServer.ts | 68 ++++++--- .../extension/src/vat-worker-service.test.ts | 112 +++++++++----- packages/extension/src/vat-worker-service.ts | 34 +---- packages/extension/test/vat-worker-service.ts | 31 +++- packages/kernel/src/index.test.ts | 4 +- packages/kernel/src/messages.ts | 12 ++ packages/kernel/src/messages/utils.ts | 20 +++ .../src/messages/vat-worker-service.test.ts | 141 ++++++++++++++++++ .../kernel/src/messages/vat-worker-service.ts | 71 +++++++++ packages/kernel/src/types.ts | 20 +++ yarn.lock | 1 + 15 files changed, 544 insertions(+), 158 deletions(-) create mode 100644 packages/kernel/src/messages/vat-worker-service.test.ts create mode 100644 packages/kernel/src/messages/vat-worker-service.ts diff --git a/packages/extension/package.json b/packages/extension/package.json index 8347bf599..d96b8de5f 100644 --- a/packages/extension/package.json +++ b/packages/extension/package.json @@ -39,6 +39,7 @@ "@endo/promise-kit": "^1.1.6", "@metamask/snaps-utils": "^8.3.0", "@metamask/utils": "^9.3.0", + "@ocap/errors": "workspace:^", "@ocap/kernel": "workspace:^", "@ocap/shims": "workspace:^", "@ocap/streams": "workspace:^", diff --git a/packages/extension/src/VatWorkerClient.test.ts b/packages/extension/src/VatWorkerClient.test.ts index 314db09e5..431e63301 100644 --- a/packages/extension/src/VatWorkerClient.test.ts +++ b/packages/extension/src/VatWorkerClient.test.ts @@ -1,11 +1,11 @@ import '@ocap/shims/endoify'; -import type { VatId } from '@ocap/kernel'; +import type { VatId, VatWorkerServiceCommandReply } from '@ocap/kernel'; +import { VatWorkerServiceCommandMethod } from '@ocap/kernel'; import { delay } from '@ocap/test-utils'; import type { Logger } from '@ocap/utils'; import { makeLogger } from '@ocap/utils'; import { describe, it, expect, beforeEach, vi } from 'vitest'; -import { VatWorkerServiceMethod } from './vat-worker-service.js'; import type { ExtensionVatWorkerClient } from './VatWorkerClient.js'; import { makeTestClient } from '../test/vat-worker-service.js'; @@ -40,16 +40,18 @@ describe('ExtensionVatWorkerClient', () => { it.each` method - ${VatWorkerServiceMethod.Launch} - ${VatWorkerServiceMethod.Terminate} + ${VatWorkerServiceCommandMethod.Launch} + ${VatWorkerServiceCommandMethod.Terminate} `( "calls logger.error when receiving a $method reply it wasn't waiting for", async ({ method }) => { const errorSpy = vi.spyOn(clientLogger, 'error'); - const unexpectedReply = { - method, - id: 9, - vatId: 'v0', + const unexpectedReply: VatWorkerServiceCommandReply = { + id: 'm9', + payload: { + method, + params: { vatId: 'v0' }, + }, }; serverPort.postMessage(unexpectedReply); await delay(100); @@ -61,15 +63,17 @@ describe('ExtensionVatWorkerClient', () => { }, ); - it(`calls logger.error when receiving a ${VatWorkerServiceMethod.Launch} reply without a port`, async () => { + it(`calls logger.error when receiving a ${VatWorkerServiceCommandMethod.Launch} reply without a port`, async () => { const errorSpy = vi.spyOn(clientLogger, 'error'); const vatId: VatId = 'v0'; // eslint-disable-next-line @typescript-eslint/no-floating-promises client.launch(vatId); const reply = { - method: VatWorkerServiceMethod.Launch, - id: 1, - vatId: 'v0', + id: 'm1', + payload: { + method: VatWorkerServiceCommandMethod.Launch, + params: { vatId: 'v0' }, + }, }; serverPort.postMessage(reply); await delay(100); diff --git a/packages/extension/src/VatWorkerClient.ts b/packages/extension/src/VatWorkerClient.ts index 1ecc57e4b..70cb751c1 100644 --- a/packages/extension/src/VatWorkerClient.ts +++ b/packages/extension/src/VatWorkerClient.ts @@ -1,21 +1,23 @@ import { makePromiseKit } from '@endo/promise-kit'; import type { PromiseKit } from '@endo/promise-kit'; +import { isObject } from '@metamask/utils'; +import { + VatWorkerServiceCommandMethod, + isVatWorkerServiceCommandReply, +} from '@ocap/kernel'; import type { StreamEnvelope, StreamEnvelopeReply, VatWorkerService, VatId, + VatWorkerServiceCommand, } from '@ocap/kernel'; import type { DuplexStream } from '@ocap/streams'; import { MessagePortDuplexStream } from '@ocap/streams'; import type { Logger } from '@ocap/utils'; import { makeCounter, makeHandledCallback, makeLogger } from '@ocap/utils'; -import type { AddListener } from './vat-worker-service.js'; -import { - isVatWorkerServiceMessage, - VatWorkerServiceMethod, -} from './vat-worker-service.js'; +import type { AddListener, PostMessage } from './vat-worker-service.js'; // Appears in the docs. // eslint-disable-next-line @typescript-eslint/no-unused-vars import type { ExtensionVatWorkerServer } from './VatWorkerServer.js'; @@ -25,11 +27,14 @@ type PromiseCallbacks = Omit, 'promise'>; export class ExtensionVatWorkerClient implements VatWorkerService { readonly #logger: Logger; - readonly #unresolvedMessages: Map = new Map(); + readonly #unresolvedMessages: Map< + VatWorkerServiceCommand['id'], + PromiseCallbacks + > = new Map(); readonly #messageCounter = makeCounter(); - readonly #postMessage: (message: unknown) => void; + readonly #postMessage: PostMessage; /** * The client end of the vat worker service, intended to be constructed in @@ -44,7 +49,7 @@ export class ExtensionVatWorkerClient implements VatWorkerService { * @param logger - An optional {@link Logger}. Defaults to a new logger labeled '[vat worker client]'. */ constructor( - postMessage: (message: unknown) => void, + postMessage: PostMessage, addListener: AddListener, logger?: Logger, ) { @@ -54,15 +59,11 @@ export class ExtensionVatWorkerClient implements VatWorkerService { } async #sendMessage( - method: - | typeof VatWorkerServiceMethod.Launch - | typeof VatWorkerServiceMethod.Terminate, - vatId: VatId, + payload: VatWorkerServiceCommand['payload'], ): Promise { - const message = { - id: this.#messageCounter(), - method, - vatId, + const message: VatWorkerServiceCommand = { + id: `m${this.#messageCounter()}`, + payload, }; const { promise, resolve, reject } = makePromiseKit(); this.#unresolvedMessages.set(message.id, { @@ -73,22 +74,38 @@ export class ExtensionVatWorkerClient implements VatWorkerService { return promise; } - async launch(vatId: VatId): Promise>{ - return this.#sendMessage(VatWorkerServiceMethod.Launch, vatId); + async launch( + vatId: VatId, + ): Promise> { + return this.#sendMessage({ + method: VatWorkerServiceCommandMethod.Launch, + params: { vatId }, + }); } async terminate(vatId: VatId): Promise { - return this.#sendMessage(VatWorkerServiceMethod.Terminate, vatId); + return this.#sendMessage({ + method: VatWorkerServiceCommandMethod.Terminate, + params: { vatId }, + }); + } + + async terminateAll(): Promise { + return this.#sendMessage({ + method: VatWorkerServiceCommandMethod.TerminateAll, + params: null, + }); } async #handleMessage(event: MessageEvent): Promise { - if (!isVatWorkerServiceMessage(event.data)) { + if (!isVatWorkerServiceCommandReply(event.data)) { // This happens when other messages pass through the same channel. this.#logger.debug('Received unexpected message', event.data); return; } - const { id, method, error } = event.data; + const { id, payload } = event.data; + const { method } = payload; const port = event.ports.at(0); const promise = this.#unresolvedMessages.get(id); @@ -98,13 +115,13 @@ export class ExtensionVatWorkerClient implements VatWorkerService { return; } - if (error) { - promise.reject(error); + if (isObject(payload.params) && payload.params.error) { + promise.reject(new Error(payload.params.error)); return; } switch (method) { - case VatWorkerServiceMethod.Launch: + case VatWorkerServiceCommandMethod.Launch: if (!port) { this.#logger.error('Expected a port with message reply', event); return; @@ -115,7 +132,8 @@ export class ExtensionVatWorkerClient implements VatWorkerService { ), ); break; - case VatWorkerServiceMethod.Terminate: + case VatWorkerServiceCommandMethod.Terminate: + case VatWorkerServiceCommandMethod.TerminateAll: // If we were caching streams on the client this would be a good place // to remove them. promise.resolve(undefined); diff --git a/packages/extension/src/VatWorkerServer.test.ts b/packages/extension/src/VatWorkerServer.test.ts index f2805081e..f14dcc5f2 100644 --- a/packages/extension/src/VatWorkerServer.test.ts +++ b/packages/extension/src/VatWorkerServer.test.ts @@ -1,11 +1,16 @@ import '@ocap/shims/endoify'; +import { VatWorkerServiceCommandMethod } from '@ocap/kernel'; import { delay } from '@ocap/test-utils'; import type { Logger } from '@ocap/utils'; import { makeLogger } from '@ocap/utils'; import { describe, it, expect, beforeEach, vi } from 'vitest'; +import type { VatWorker } from './vat-worker-service.js'; import type { ExtensionVatWorkerServer } from './VatWorkerServer.js'; -import { makeTestServer } from '../test/vat-worker-service.js'; +import { + getMockMakeWorker, + makeTestServer, +} from '../test/vat-worker-service.js'; describe('ExtensionVatWorkerServer', () => { let serverPort: MessagePort; @@ -28,30 +33,74 @@ describe('ExtensionVatWorkerServer', () => { const deliveredMessageChannel = new MessageChannel(); // vatPort = deliveredMessageChannel.port1; kernelPort = deliveredMessageChannel.port2; - - server = makeTestServer({ serverPort, logger, kernelPort }); }); - it('starts', () => { - server.start(); - expect(serverPort.onmessage).toBeDefined(); - }); + describe('Misc', () => { + beforeEach(() => { + server = makeTestServer({ serverPort, logger, kernelPort }); + }); + + it('starts', () => { + server.start(); + expect(serverPort.onmessage).toBeDefined(); + }); + + it('throws if started twice', () => { + server.start(); + expect(() => server.start()).toThrow(/already running/u); + }); - it('throws if started twice', () => { - server.start(); - expect(() => server.start()).toThrow(/already running/u); + it('calls logger.debug when receiving an unexpected message', async () => { + const debugSpy = vi.spyOn(logger, 'debug'); + const unexpectedMessage = 'foobar'; + server.start(); + clientPort.postMessage(unexpectedMessage); + await delay(100); + expect(debugSpy).toHaveBeenCalledOnce(); + expect(debugSpy).toHaveBeenLastCalledWith( + 'Received unexpected message', + unexpectedMessage, + ); + }); }); - it('calls logger.debug when receiving an unexpected message', async () => { - const debugSpy = vi.spyOn(logger, 'debug'); - const unexpectedMessage = 'foobar'; - server.start(); - clientPort.postMessage(unexpectedMessage); - await delay(100); - expect(debugSpy).toHaveBeenCalledOnce(); - expect(debugSpy).toHaveBeenLastCalledWith( - 'Received unexpected message', - unexpectedMessage, - ); + describe('terminateAll', () => { + let workers: VatWorker[]; + let makeWorker: (vatId: VatId) => VatWorker; + + beforeEach(() => { + [makeWorker, ...workers] = getMockMakeWorker(3); + server = makeTestServer({ serverPort, logger, makeWorker }); + }); + + it('calls logger.error when a vat fails to terminate', async () => { + const errorSpy = vi.spyOn(logger, 'error'); + const vatId = 'v0'; + const vatNotFoundError = new Error(`vat not found: ${vatId}`); + vi.spyOn(workers[0], 'terminate').mockRejectedValue(vatNotFoundError); + server.start(); + clientPort.postMessage({ + id: 'm0', + payload: { + method: VatWorkerServiceCommandMethod.Launch, + params: { vatId }, + }, + }); + clientPort.postMessage({ + id: 'm1', + payload: { + method: VatWorkerServiceCommandMethod.TerminateAll, + params: null, + }, + }); + + await delay(100); + + expect(errorSpy).toHaveBeenCalledOnce(); + expect(errorSpy.mock.lastCall[0]).toBe( + `Error handling ${VatWorkerServiceCommandMethod.TerminateAll} for vatId ${vatId}`, + ); + expect(errorSpy.mock.lastCall[1]).toBe(vatNotFoundError); + }); }); }); diff --git a/packages/extension/src/VatWorkerServer.ts b/packages/extension/src/VatWorkerServer.ts index 619c2302a..e2cf90504 100644 --- a/packages/extension/src/VatWorkerServer.ts +++ b/packages/extension/src/VatWorkerServer.ts @@ -1,11 +1,13 @@ -import type { VatId } from '@ocap/kernel'; +import type { OcapError } from '@ocap/errors'; +import { VatAlreadyExistsError, VatDeletedError } from '@ocap/errors'; +import { + isVatWorkerServiceCommand, + VatWorkerServiceCommandMethod, +} from '@ocap/kernel'; +import type { VatWorkerServiceCommandReply, VatId } from '@ocap/kernel'; import type { Logger } from '@ocap/utils'; import { makeHandledCallback, makeLogger } from '@ocap/utils'; -import { - isVatWorkerServiceMessage, - VatWorkerServiceMethod, -} from './vat-worker-service.js'; import type { AddListener, PostMessage, @@ -20,7 +22,7 @@ export class ExtensionVatWorkerServer { readonly #vatWorkers: Map = new Map(); - readonly #postMessage: PostMessage; + readonly #postMessage: PostMessage; readonly #addListener: AddListener; @@ -42,7 +44,7 @@ export class ExtensionVatWorkerServer { * @param logger - An optional {@link Logger}. Defaults to a new logger labeled '[vat worker server]'. */ constructor( - postMessage: PostMessage, + postMessage: PostMessage, addListener: (listener: (event: MessageEvent) => void) => void, makeWorker: (vatId: VatId) => VatWorker, logger?: Logger, @@ -62,32 +64,56 @@ export class ExtensionVatWorkerServer { } async #handleMessage(event: MessageEvent): Promise { - if (!isVatWorkerServiceMessage(event.data)) { + if (!isVatWorkerServiceCommand(event.data)) { // This happens when other messages pass through the same channel. this.#logger.debug('Received unexpected message', event.data); return; } - const { method, id, vatId } = event.data; + const { id, payload } = event.data; + const { method, params } = payload; - const handleProblem = async (problem: Error): Promise => { + const handleProblem = ({ + problem, + vatId, + }: { + problem: OcapError; + vatId: VatId; + }): void => { this.#logger.error( `Error handling ${method} for vatId ${vatId}`, problem, ); - this.#postMessage({ method, id, vatId, error: problem }); + this.#postMessage({ + id, + // TODO(#170): use @ocap/errors marshaling. + payload: { + method, + params: { vatId, error: `${problem.code}: ${vatId}` }, + }, + }); }; switch (method) { - case VatWorkerServiceMethod.Launch: - await this.#launch(vatId) - .then((port) => this.#postMessage({ method, id, vatId }, [port])) - .catch(handleProblem); + case VatWorkerServiceCommandMethod.Launch: + await this.#launch(params.vatId) + .then((port) => this.#postMessage({ id, payload }, [port])) + .catch(async (problem) => handleProblem({ problem, ...params })); + break; + case VatWorkerServiceCommandMethod.Terminate: + await this.#terminate(params.vatId) + .then(() => this.#postMessage({ id, payload })) + .catch(async (problem) => handleProblem({ problem, ...params })); break; - case VatWorkerServiceMethod.Terminate: - await this.#terminate(vatId) - .then(() => this.#postMessage({ method, id, vatId })) - .catch(handleProblem); + case VatWorkerServiceCommandMethod.TerminateAll: + await Promise.all( + Array.from(this.#vatWorkers.keys()).map(async (vatId) => + this.#terminate(vatId).catch((problem) => { + // eslint-disable-next-line @typescript-eslint/only-throw-error + throw { problem, vatId }; + }), + ), + ).then(() => this.#postMessage({ id, payload }), handleProblem); break; /* v8 ignore next 6: Not known to be possible. */ default: @@ -101,7 +127,7 @@ export class ExtensionVatWorkerServer { async #launch(vatId: VatId): Promise { if (this.#vatWorkers.has(vatId)) { - throw new Error(`Worker for vat ${vatId} already exists.`); + throw new VatAlreadyExistsError(vatId); } const vatWorker = this.#makeWorker(vatId); const [port] = await vatWorker.launch(); @@ -112,7 +138,7 @@ export class ExtensionVatWorkerServer { async #terminate(vatId: VatId): Promise { const vatWorker = this.#vatWorkers.get(vatId); if (!vatWorker) { - throw new Error(`Worker for vat ${vatId} does not exist.`); + throw new VatDeletedError(vatId); } await vatWorker.terminate(); this.#vatWorkers.delete(vatId); diff --git a/packages/extension/src/vat-worker-service.test.ts b/packages/extension/src/vat-worker-service.test.ts index 579d6a1bd..db556a61d 100644 --- a/packages/extension/src/vat-worker-service.test.ts +++ b/packages/extension/src/vat-worker-service.test.ts @@ -1,4 +1,12 @@ import '@ocap/shims/endoify'; +// VatAleadyExistsError and VatDeletedError appears in TODO(#170) comments. +import { + ErrorCode, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + VatAlreadyExistsError, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + VatDeletedError, +} from '@ocap/errors'; import type { VatId } from '@ocap/kernel'; import { MessagePortDuplexStream } from '@ocap/streams'; import type { MockInstance } from 'vitest'; @@ -13,17 +21,16 @@ import { makeTestServer, } from '../test/vat-worker-service.js'; -describe('VatWorker', () => { +// low key integration test +describe('VatWorkerService', () => { let serverPort: MessagePort; let clientPort: MessagePort; let server: ExtensionVatWorkerServer; let client: ExtensionVatWorkerClient; - // let vatPort: MessagePort; - let kernelPort: MessagePort; - let mockWorker: VatWorker; + let mockWorkers: VatWorker[]; let mockMakeWorker: (vatId: VatId) => VatWorker; let mockLaunchWorker: MockInstance; @@ -34,48 +41,73 @@ describe('VatWorker', () => { serverPort = serviceMessageChannel.port1; clientPort = serviceMessageChannel.port2; - const deliveredMessageChannel = new MessageChannel(); - // vatPort = deliveredMessageChannel.port1; - kernelPort = deliveredMessageChannel.port2; + [mockMakeWorker, ...mockWorkers] = getMockMakeWorker(3); - [mockWorker, mockMakeWorker] = getMockMakeWorker(kernelPort); + client = makeTestClient(clientPort); + server = makeTestServer({ serverPort, makeWorker: mockMakeWorker }); + server.start(); + }); + it('launches and terminates a worker', async () => { + mockWorker = mockWorkers[0]; mockLaunchWorker = vi.spyOn(mockWorker, 'launch'); mockTerminateWorker = vi.spyOn(mockWorker, 'terminate'); + + const vatId: VatId = 'v0'; + const stream = await client.launch(vatId); + expect(stream).toBeInstanceOf(MessagePortDuplexStream); + expect(mockLaunchWorker).toHaveBeenCalledOnce(); + expect(mockTerminateWorker).not.toHaveBeenCalled(); + + await client.terminate(vatId); + expect(mockLaunchWorker).toHaveBeenCalledOnce(); + expect(mockTerminateWorker).toHaveBeenCalledOnce(); }); - // low key integration test - describe('Service', () => { - beforeEach(() => { - client = makeTestClient(clientPort); - server = makeTestServer({ serverPort, makeWorker: mockMakeWorker }); - server.start(); - }); - - it('launches and terminates a worker', async () => { - const vatId: VatId = 'v0'; - const stream = await client.launch(vatId); + it('terminates all workers', async () => { + const mockLaunches = mockWorkers.map((worker) => + vi.spyOn(worker, 'launch'), + ); + const mockTerminates = mockWorkers.map((worker) => + vi.spyOn(worker, 'terminate'), + ); + + // launch many workers + for (let i = 0; i < mockWorkers.length; i++) { + const stream = await client.launch(`v${i}`); expect(stream).toBeInstanceOf(MessagePortDuplexStream); - expect(mockLaunchWorker).toHaveBeenCalledOnce(); - expect(mockTerminateWorker).not.toHaveBeenCalled(); - - await client.terminate(vatId); - expect(mockLaunchWorker).toHaveBeenCalledOnce(); - expect(mockTerminateWorker).toHaveBeenCalledOnce(); - }); - - it('throws when terminating a nonexistent worker', async () => { - await expect(async () => await client.terminate('v0')).rejects.toThrow( - /vat v0 does not exist/u, - ); - }); - - it('throws when launching the same worker twice', async () => { - const vatId: VatId = 'v0'; - await client.launch(vatId); - await expect(async () => await client.launch(vatId)).rejects.toThrow( - /vat v0 already exists/u, - ); - }); + } + + // each worker had its launch method called + for (let i = 0; i < mockWorkers.length; i++) { + expect(mockLaunches[i]).toHaveBeenCalledOnce(); + expect(mockTerminates[i]).not.toHaveBeenCalled(); + } + + // terminate all workers + await client.terminateAll(); + + // each worker had its terminate method called + for (let i = 0; i < mockWorkers.length; i++) { + expect(mockLaunches[i]).toHaveBeenCalledOnce(); + expect(mockTerminates[i]).toHaveBeenCalledOnce(); + } + }); + + it('throws when terminating a nonexistent worker', async () => { + const vatId: VatId = 'v0'; + await expect(async () => await client.terminate(vatId)).rejects.toThrow( + /** TODO(#170): use @ocap/errors marshaling. {@link VatDeletedError} */ + RegExp(`${ErrorCode.VatDeleted}.*${vatId}`, 'u'), + ); + }); + + it('throws when launching the same worker twice', async () => { + const vatId: VatId = 'v0'; + await client.launch(vatId); + await expect(async () => await client.launch(vatId)).rejects.toThrow( + /** TODO(#170): use @ocap/errors marshaling. {@link VatAlreadyExistsError} */ + RegExp(`${ErrorCode.VatAlreadyExists}.*${vatId}`, 'u'), + ); }); }); diff --git a/packages/extension/src/vat-worker-service.ts b/packages/extension/src/vat-worker-service.ts index aaa9e3d68..c042c296a 100644 --- a/packages/extension/src/vat-worker-service.ts +++ b/packages/extension/src/vat-worker-service.ts @@ -1,38 +1,12 @@ -import { isObject } from '@metamask/utils'; -import type { VatId } from '@ocap/kernel'; - -export enum VatWorkerServiceMethod { - Launch = 'iframe-vat-worker-launch', - Terminate = 'iframe-vat-worker-terminate', -} - -type MessageId = number; - export type VatWorker = { launch: () => Promise<[MessagePort, unknown]>; terminate: () => Promise; }; -export type VatWorkerServiceMessage = { - method: - | typeof VatWorkerServiceMethod.Launch - | typeof VatWorkerServiceMethod.Terminate; - id: MessageId; - vatId: VatId; - error?: Error; -}; - -export const isVatWorkerServiceMessage = ( - value: unknown, -): value is VatWorkerServiceMessage => - isObject(value) && - typeof value.id === 'number' && - Object.values(VatWorkerServiceMethod).includes( - value.method as VatWorkerServiceMethod, - ) && - typeof value.vatId === 'string'; - -export type PostMessage = (message: unknown, transfer?: Transferable[]) => void; +export type PostMessage = ( + message: Message, + transfer?: Transferable[], +) => void; export type AddListener = ( listener: (event: MessageEvent) => void, ) => void; diff --git a/packages/extension/test/vat-worker-service.ts b/packages/extension/test/vat-worker-service.ts index 36e5f110d..61a7d3308 100644 --- a/packages/extension/test/vat-worker-service.ts +++ b/packages/extension/test/vat-worker-service.ts @@ -1,4 +1,5 @@ import type { VatId } from '@ocap/kernel'; +import { makeCounter } from '@ocap/utils'; import type { Logger } from '@ocap/utils'; import { vi } from 'vitest'; @@ -6,17 +7,31 @@ import type { VatWorker } from '../src/vat-worker-service.js'; import { ExtensionVatWorkerClient } from '../src/VatWorkerClient.js'; import { ExtensionVatWorkerServer } from '../src/VatWorkerServer.js'; -type MakeVatWorker = (vatId: VatId) => VatWorker; +type MakeVatWorker = (vatId: VatId) => VatWorker & { kernelPort: MessagePort }; export const getMockMakeWorker = ( - kernelPort: MessagePort, -): [VatWorker, MakeVatWorker] => { - const mockWorker = { - launch: vi.fn().mockResolvedValue([kernelPort, {}]), - terminate: vi.fn().mockResolvedValue(undefined), - }; + nWorkers: number = 1, +): [MakeVatWorker, ...VatWorker[]] => { + const counter = makeCounter(-1); + const mockWorkers = Array(nWorkers) + .fill(0) + .map(() => { + const { + // port1: vatPort, + port2: kernelPort, + } = new MessageChannel(); + return { + launch: vi.fn().mockResolvedValue([kernelPort, {}]), + terminate: vi.fn().mockResolvedValue(undefined), + // vatPort, + kernelPort, + }; + }); - return [mockWorker, vi.fn().mockReturnValue(mockWorker)]; + return [ + vi.fn().mockImplementation(() => mockWorkers[counter()]), + ...mockWorkers, + ]; }; export const makeTestClient = ( diff --git a/packages/kernel/src/index.test.ts b/packages/kernel/src/index.test.ts index 5e7a1d2ca..c11c0bd83 100644 --- a/packages/kernel/src/index.test.ts +++ b/packages/kernel/src/index.test.ts @@ -1,3 +1,4 @@ +import '@ocap/shims/endoify'; import { describe, it, expect } from 'vitest'; import * as indexModule from './index.js'; @@ -7,7 +8,8 @@ describe('index', () => { expect(Object.keys(indexModule)).toStrictEqual( expect.arrayContaining( ['Kernel', 'Vat'].concat( - ['Cluster', 'Kernel', 'Vat'].flatMap((value) => [ + ['Cluster', 'Kernel', 'Vat', 'VatWorkerService'].flatMap((value) => [ + `${value}CommandMethod`, `is${value}Command`, `is${value}CommandReply`, ]), diff --git a/packages/kernel/src/messages.ts b/packages/kernel/src/messages.ts index 867201811..4e10c4697 100644 --- a/packages/kernel/src/messages.ts +++ b/packages/kernel/src/messages.ts @@ -3,6 +3,18 @@ export { isCapTpPayload, isCapTpMessage } from './messages/captp.js'; export type { CapTpPayload, CapTpMessage } from './messages/captp.js'; +// Vat worker service commands. + +export { + VatWorkerServiceCommandMethod, + isVatWorkerServiceCommand, + isVatWorkerServiceCommandReply, +} from './messages/vat-worker-service.js'; +export type { + VatWorkerServiceCommand, + VatWorkerServiceCommandReply, +} from './messages/vat-worker-service.js'; + // Cluster commands. export { diff --git a/packages/kernel/src/messages/utils.ts b/packages/kernel/src/messages/utils.ts index ca9d7b510..0a68be2d4 100644 --- a/packages/kernel/src/messages/utils.ts +++ b/packages/kernel/src/messages/utils.ts @@ -1,4 +1,24 @@ // Uncapitalize. +import { hasProperty, isObject } from '@metamask/utils'; +import type { ErrorCode } from '@ocap/errors'; + export const uncapitalize = (value: string): Uncapitalize => (value.at(0)?.toLowerCase() + value.slice(1)) as Uncapitalize; + +// Marshaled error mock. +// TODO(#170): use @ocap/errors marshalling. Delete this. + +export type MarshaledError = string; + +export const hasMarshaledError = ( + mode: Mode, + value: object, + ...codes: ErrorCode[] +): value is Mode extends 'required' + ? { error: MarshaledError } + : { error?: MarshaledError } => + (mode === 'optional' && !hasProperty(value, 'error')) || + (isObject(value) && + typeof value.error === 'string' && + codes.some((code) => (value.error as string).startsWith(code))); diff --git a/packages/kernel/src/messages/vat-worker-service.test.ts b/packages/kernel/src/messages/vat-worker-service.test.ts new file mode 100644 index 000000000..ff7165303 --- /dev/null +++ b/packages/kernel/src/messages/vat-worker-service.test.ts @@ -0,0 +1,141 @@ +import '@ocap/shims/endoify'; +import { ErrorCode } from '@ocap/errors'; +import { describe, expect, it } from 'vitest'; + +import type { VatWorkerServiceCommandReply } from './vat-worker-service.js'; +import { + isVatWorkerServiceCommand, + isVatWorkerServiceCommandReply, + VatWorkerServiceCommandMethod, +} from './vat-worker-service.js'; +import type { VatId } from '../types.js'; + +const launch: VatWorkerServiceCommandReply['payload'] = { + method: VatWorkerServiceCommandMethod.Launch, + params: { vatId: 'v0' }, +}; +const terminate: VatWorkerServiceCommandReply['payload'] = { + method: VatWorkerServiceCommandMethod.Terminate, + params: { vatId: 'v0' }, +}; +const terminateAll: VatWorkerServiceCommandReply['payload'] = { + method: VatWorkerServiceCommandMethod.TerminateAll, + params: null, +}; + +const sharedCases = (payload: unknown): [boolean, string, unknown][] => [ + [true, 'valid message id with valid payload', { id: 'm0', payload }], + [false, 'invalid id', { id: 'vat-message-id', payload }], + [false, 'numerical id', { id: 1, payload }], + [false, 'missing payload', { id: 'm0' }], +]; + +describe('isVatWorkerServiceCommand', () => { + describe.each` + payload + ${launch} + ${terminate} + `('$payload.method', ({ payload }) => { + it.each(sharedCases(payload))( + 'returns %j for %j', + (expectedResult, _, value) => { + expect(isVatWorkerServiceCommand(value)).toBe(expectedResult); + }, + ); + }); +}); + +describe('isVatWorkerServiceCommandReply', () => { + const withError = ( + payload: VatWorkerServiceCommandReply['payload'], + problem: unknown, + ): unknown => ({ + method: payload.method, + params: { ...payload.params, error: problem }, + }); + + // TODO(#170): use @ocap/errors marshaling. + const withVatError = ( + payload: VatWorkerServiceCommandReply['payload'], + problem: ErrorCode, + vatId: VatId, + ): unknown => ({ + method: payload.method, + params: { ...payload.params, error: `${problem}: ${vatId}`, vatId }, + }); + + describe('launch', () => { + it.each([ + ...sharedCases(launch), + [ + true, + 'valid message id with valid error', + // TODO(#170): use @ocap/errors marshaling. + { + id: 'm0', + payload: withVatError(launch, ErrorCode.VatAlreadyExists, 'v0'), + }, + ], + [ + false, + 'valid message id with invalid error', + { id: 'm0', payload: withError(launch, 404) }, + ], + ])('returns %j for %j', (expectedResult, _, value) => { + expect(isVatWorkerServiceCommandReply(value)).toBe(expectedResult); + }); + }); + + describe('terminate', () => { + it.each([ + ...sharedCases(terminate), + [ + true, + 'valid message id with valid error', + // TODO(#170): use @ocap/errors marshaling. + { + id: 'm0', + payload: withVatError(terminate, ErrorCode.VatDeleted, 'v0'), + }, + ], + [ + false, + 'valid message id with invalid error', + { id: 'm0', payload: withError(terminate, 404) }, + ], + ])('returns %j for %j', (expectedResult, _, value) => { + expect(isVatWorkerServiceCommandReply(value)).toBe(expectedResult); + }); + }); + + describe('terminateAll', () => { + it.each([ + ...sharedCases(terminateAll), + [ + true, + 'valid message id with valid vat error', + // TODO(#170): use @ocap/errors marshaling. + { + id: 'm0', + payload: withVatError(terminateAll, ErrorCode.VatDeleted, 'v0'), + }, + ], + [ + true, + 'valid message id with valid error', + // TODO(#170): use @ocap/errors marshaling. + { + id: 'm0', + payload: withError(terminateAll, `${ErrorCode.VatNotFound}`), + }, + ], + [ + false, + 'valid message id with invalid error', + { id: 'm0', payload: withError(terminateAll, 404) }, + ], + ])('returns %j for %j', (expectedResult, _, value) => { + expect(isVatWorkerServiceCommandReply(value)).toBe(expectedResult); + }); + }); +}); diff --git a/packages/kernel/src/messages/vat-worker-service.ts b/packages/kernel/src/messages/vat-worker-service.ts new file mode 100644 index 000000000..681171472 --- /dev/null +++ b/packages/kernel/src/messages/vat-worker-service.ts @@ -0,0 +1,71 @@ +import { hasProperty, isObject } from '@metamask/utils'; +import { ErrorCode } from '@ocap/errors'; +import type { TypeGuard } from '@ocap/utils'; + +import { makeIdentifiedMessageKit, messageType } from './message-kit.js'; +import { hasMarshaledError } from './utils.js'; +import type { MarshaledError } from './utils.js'; +import type { VatId } from '../types.js'; +import { isVatId } from '../types.js'; +// TODO(#170): use @ocap/errors marshaling. + +export const vatWorkerServiceCommand = { + Launch: messageType< + { vatId: VatId }, + // Expect VatAlreadyExistsError. + { vatId: VatId; error?: MarshaledError } + >( + (send) => isObject(send) && isVatId(send.vatId), + (reply) => + isObject(reply) && + isVatId(reply.vatId) && + hasMarshaledError('optional', reply, ErrorCode.VatAlreadyExists), + ), + + Terminate: messageType< + { vatId: VatId }, + // Expect VatDeletedError. + { vatId: VatId; error?: MarshaledError } + >( + (send) => isObject(send) && isVatId(send.vatId), + (reply) => + isObject(reply) && + isVatId(reply.vatId) && + hasMarshaledError('optional', reply, ErrorCode.VatDeleted), + ), + + TerminateAll: messageType< + null, + null | { vatId?: VatId; error: MarshaledError } + >( + (send) => send === null, + (reply) => + reply === null || + (isObject(reply) && + hasMarshaledError( + 'required', + reply, + ErrorCode.VatDeleted, + ErrorCode.VatNotFound, + ) && + (!hasProperty(reply, 'vatId') || isVatId(reply.vatId))), + ), +}; + +const messageKit = makeIdentifiedMessageKit( + vatWorkerServiceCommand, + (value: unknown): value is `m${number}` => + typeof value === 'string' && + value.at(0) === 'm' && + value.slice(1) === String(Number(value.slice(1))), +); + +export const VatWorkerServiceCommandMethod = messageKit.methods; + +export type VatWorkerServiceCommand = typeof messageKit.send; +export const isVatWorkerServiceCommand: TypeGuard = + messageKit.sendGuard; + +export type VatWorkerServiceCommandReply = typeof messageKit.reply; +export const isVatWorkerServiceCommandReply: TypeGuard = + messageKit.replyGuard; diff --git a/packages/kernel/src/types.ts b/packages/kernel/src/types.ts index f489749e6..308fe1298 100644 --- a/packages/kernel/src/types.ts +++ b/packages/kernel/src/types.ts @@ -16,8 +16,28 @@ export type PromiseCallbacks = Omit< >; export type VatWorkerService = { + /** + * Launch a new worker with a specific vat id. + * + * @param vatId - The vat id of the worker to launch. + * @returns A promise for a duplex stream connected to the worker + * which rejects if a worker with the given vat id already exists. + */ launch: ( vatId: VatId, ) => Promise>; + /** + * Terminate a worker identified by its vat id. + * + * @param vatId - The vat id of the worker to terminate. + * @returns A promise that resolves when the worker has terminated + * or rejects if that worker does not exist. + */ terminate: (vatId: VatId) => Promise; + /** + * Terminate all workers known to the service. + * + * @returns A promise for the number of workers deleted. + */ + terminateAll: () => Promise; }; diff --git a/yarn.lock b/yarn.lock index 8312a704f..08b901b01 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1514,6 +1514,7 @@ __metadata: "@metamask/eslint-config-typescript": "npm:^14.0.0" "@metamask/snaps-utils": "npm:^8.3.0" "@metamask/utils": "npm:^9.3.0" + "@ocap/errors": "workspace:^" "@ocap/kernel": "workspace:^" "@ocap/shims": "workspace:^" "@ocap/streams": "workspace:^" From a60ec03b544498151a5ae30dbf8e1dea0af4ce5a Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Thu, 17 Oct 2024 16:11:04 -0500 Subject: [PATCH 04/12] use @ocap/errors --- packages/extension/src/VatWorkerClient.ts | 3 +- packages/extension/src/VatWorkerServer.ts | 12 ++-- .../extension/src/vat-worker-service.test.ts | 18 ++---- packages/kernel/src/messages/message-kit.ts | 3 +- packages/kernel/src/messages/utils.ts | 20 ------ .../src/messages/vat-worker-service.test.ts | 63 ++++++++++--------- .../kernel/src/messages/vat-worker-service.ts | 21 +++---- 7 files changed, 55 insertions(+), 85 deletions(-) diff --git a/packages/extension/src/VatWorkerClient.ts b/packages/extension/src/VatWorkerClient.ts index 70cb751c1..2139bcb9d 100644 --- a/packages/extension/src/VatWorkerClient.ts +++ b/packages/extension/src/VatWorkerClient.ts @@ -1,6 +1,7 @@ import { makePromiseKit } from '@endo/promise-kit'; import type { PromiseKit } from '@endo/promise-kit'; import { isObject } from '@metamask/utils'; +import { unmarshalError } from '@ocap/errors'; import { VatWorkerServiceCommandMethod, isVatWorkerServiceCommandReply, @@ -116,7 +117,7 @@ export class ExtensionVatWorkerClient implements VatWorkerService { } if (isObject(payload.params) && payload.params.error) { - promise.reject(new Error(payload.params.error)); + promise.reject(unmarshalError(payload.params.error)); return; } diff --git a/packages/extension/src/VatWorkerServer.ts b/packages/extension/src/VatWorkerServer.ts index e2cf90504..941536910 100644 --- a/packages/extension/src/VatWorkerServer.ts +++ b/packages/extension/src/VatWorkerServer.ts @@ -1,5 +1,9 @@ import type { OcapError } from '@ocap/errors'; -import { VatAlreadyExistsError, VatDeletedError } from '@ocap/errors'; +import { + VatAlreadyExistsError, + VatDeletedError, + marshalError, +} from '@ocap/errors'; import { isVatWorkerServiceCommand, VatWorkerServiceCommandMethod, @@ -86,11 +90,7 @@ export class ExtensionVatWorkerServer { ); this.#postMessage({ id, - // TODO(#170): use @ocap/errors marshaling. - payload: { - method, - params: { vatId, error: `${problem.code}: ${vatId}` }, - }, + payload: { method, params: { vatId, error: marshalError(problem) } }, }); }; diff --git a/packages/extension/src/vat-worker-service.test.ts b/packages/extension/src/vat-worker-service.test.ts index db556a61d..f96ef9261 100644 --- a/packages/extension/src/vat-worker-service.test.ts +++ b/packages/extension/src/vat-worker-service.test.ts @@ -1,12 +1,5 @@ import '@ocap/shims/endoify'; -// VatAleadyExistsError and VatDeletedError appears in TODO(#170) comments. -import { - ErrorCode, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - VatAlreadyExistsError, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - VatDeletedError, -} from '@ocap/errors'; +import { VatAlreadyExistsError, VatDeletedError } from '@ocap/errors'; import type { VatId } from '@ocap/kernel'; import { MessagePortDuplexStream } from '@ocap/streams'; import type { MockInstance } from 'vitest'; @@ -95,10 +88,8 @@ describe('VatWorkerService', () => { }); it('throws when terminating a nonexistent worker', async () => { - const vatId: VatId = 'v0'; - await expect(async () => await client.terminate(vatId)).rejects.toThrow( - /** TODO(#170): use @ocap/errors marshaling. {@link VatDeletedError} */ - RegExp(`${ErrorCode.VatDeleted}.*${vatId}`, 'u'), + await expect(async () => await client.terminate('v0')).rejects.toThrow( + VatDeletedError, ); }); @@ -106,8 +97,7 @@ describe('VatWorkerService', () => { const vatId: VatId = 'v0'; await client.launch(vatId); await expect(async () => await client.launch(vatId)).rejects.toThrow( - /** TODO(#170): use @ocap/errors marshaling. {@link VatAlreadyExistsError} */ - RegExp(`${ErrorCode.VatAlreadyExists}.*${vatId}`, 'u'), + VatAlreadyExistsError, ); }); }); diff --git a/packages/kernel/src/messages/message-kit.ts b/packages/kernel/src/messages/message-kit.ts index 81b83fa8d..971d746b3 100644 --- a/packages/kernel/src/messages/message-kit.ts +++ b/packages/kernel/src/messages/message-kit.ts @@ -1,6 +1,7 @@ import '@ocap/shims/endoify'; -import { isObject, type Json } from '@metamask/utils'; +import { isObject } from '@metamask/utils'; +import type { Json } from '@metamask/utils'; import type { ExtractGuardType, TypeGuard } from '@ocap/utils'; import { isMessageLike } from './message.js'; diff --git a/packages/kernel/src/messages/utils.ts b/packages/kernel/src/messages/utils.ts index 0a68be2d4..ca9d7b510 100644 --- a/packages/kernel/src/messages/utils.ts +++ b/packages/kernel/src/messages/utils.ts @@ -1,24 +1,4 @@ // Uncapitalize. -import { hasProperty, isObject } from '@metamask/utils'; -import type { ErrorCode } from '@ocap/errors'; - export const uncapitalize = (value: string): Uncapitalize => (value.at(0)?.toLowerCase() + value.slice(1)) as Uncapitalize; - -// Marshaled error mock. -// TODO(#170): use @ocap/errors marshalling. Delete this. - -export type MarshaledError = string; - -export const hasMarshaledError = ( - mode: Mode, - value: object, - ...codes: ErrorCode[] -): value is Mode extends 'required' - ? { error: MarshaledError } - : { error?: MarshaledError } => - (mode === 'optional' && !hasProperty(value, 'error')) || - (isObject(value) && - typeof value.error === 'string' && - codes.some((code) => (value.error as string).startsWith(code))); diff --git a/packages/kernel/src/messages/vat-worker-service.test.ts b/packages/kernel/src/messages/vat-worker-service.test.ts index ff7165303..7f0756300 100644 --- a/packages/kernel/src/messages/vat-worker-service.test.ts +++ b/packages/kernel/src/messages/vat-worker-service.test.ts @@ -1,5 +1,9 @@ import '@ocap/shims/endoify'; -import { ErrorCode } from '@ocap/errors'; +import { + marshalError, + VatAlreadyExistsError, + VatDeletedError, +} from '@ocap/errors'; import { describe, expect, it } from 'vitest'; import type { VatWorkerServiceCommandReply } from './vat-worker-service.js'; @@ -35,6 +39,7 @@ describe('isVatWorkerServiceCommand', () => { payload ${launch} ${terminate} + ${terminateAll} `('$payload.method', ({ payload }) => { it.each(sharedCases(payload))( 'returns %j for %j', @@ -54,27 +59,20 @@ describe('isVatWorkerServiceCommandReply', () => { params: { ...payload.params, error: problem }, }); - // TODO(#170): use @ocap/errors marshaling. - const withVatError = ( - payload: VatWorkerServiceCommandReply['payload'], - problem: ErrorCode, - vatId: VatId, - ): unknown => ({ - method: payload.method, - params: { ...payload.params, error: `${problem}: ${vatId}`, vatId }, - }); - describe('launch', () => { + const withMarshaledError = (vatId: VatId): unknown => ({ + method: launch.method, + params: { + ...launch.params, + error: marshalError(new VatAlreadyExistsError(vatId)), + }, + }); it.each([ ...sharedCases(launch), [ true, 'valid message id with valid error', - // TODO(#170): use @ocap/errors marshaling. - { - id: 'm0', - payload: withVatError(launch, ErrorCode.VatAlreadyExists, 'v0'), - }, + { id: 'm0', payload: withMarshaledError('v0') }, ], [ false, @@ -87,16 +85,19 @@ describe('isVatWorkerServiceCommandReply', () => { }); describe('terminate', () => { + const withMarshaledError = (vatId: VatId): unknown => ({ + method: terminate.method, + params: { + ...terminate.params, + error: marshalError(new VatDeletedError(vatId)), + }, + }); it.each([ ...sharedCases(terminate), [ true, 'valid message id with valid error', - // TODO(#170): use @ocap/errors marshaling. - { - id: 'm0', - payload: withVatError(terminate, ErrorCode.VatDeleted, 'v0'), - }, + { id: 'm0', payload: withMarshaledError('v0') }, ], [ false, @@ -109,25 +110,25 @@ describe('isVatWorkerServiceCommandReply', () => { }); describe('terminateAll', () => { + const withValidVatError = (vatId: VatId): unknown => ({ + method: terminateAll.method, + params: { vatId, error: marshalError(new VatDeletedError(vatId)) }, + }); + const withMarshaledError = (): unknown => ({ + method: terminateAll.method, + params: { error: marshalError(new Error('code: foobar')) }, + }); it.each([ ...sharedCases(terminateAll), [ true, 'valid message id with valid vat error', - // TODO(#170): use @ocap/errors marshaling. - { - id: 'm0', - payload: withVatError(terminateAll, ErrorCode.VatDeleted, 'v0'), - }, + { id: 'm0', payload: withValidVatError('v0') }, ], [ true, 'valid message id with valid error', - // TODO(#170): use @ocap/errors marshaling. - { - id: 'm0', - payload: withError(terminateAll, `${ErrorCode.VatNotFound}`), - }, + { id: 'm0', payload: withMarshaledError() }, ], [ false, diff --git a/packages/kernel/src/messages/vat-worker-service.ts b/packages/kernel/src/messages/vat-worker-service.ts index 681171472..cfe621ee4 100644 --- a/packages/kernel/src/messages/vat-worker-service.ts +++ b/packages/kernel/src/messages/vat-worker-service.ts @@ -1,13 +1,15 @@ import { hasProperty, isObject } from '@metamask/utils'; -import { ErrorCode } from '@ocap/errors'; +import { ErrorCode, isMarshaledError } from '@ocap/errors'; +import type { MarshaledError } from '@ocap/errors'; import type { TypeGuard } from '@ocap/utils'; import { makeIdentifiedMessageKit, messageType } from './message-kit.js'; -import { hasMarshaledError } from './utils.js'; -import type { MarshaledError } from './utils.js'; import type { VatId } from '../types.js'; import { isVatId } from '../types.js'; -// TODO(#170): use @ocap/errors marshaling. + +const hasOptionalMarshaledError = (value: object, code: ErrorCode): boolean => + !hasProperty(value, 'error') || + (isMarshaledError(value.error) && value.error.code === code); export const vatWorkerServiceCommand = { Launch: messageType< @@ -19,7 +21,7 @@ export const vatWorkerServiceCommand = { (reply) => isObject(reply) && isVatId(reply.vatId) && - hasMarshaledError('optional', reply, ErrorCode.VatAlreadyExists), + hasOptionalMarshaledError(reply, ErrorCode.VatAlreadyExists), ), Terminate: messageType< @@ -31,7 +33,7 @@ export const vatWorkerServiceCommand = { (reply) => isObject(reply) && isVatId(reply.vatId) && - hasMarshaledError('optional', reply, ErrorCode.VatDeleted), + hasOptionalMarshaledError(reply, ErrorCode.VatDeleted), ), TerminateAll: messageType< @@ -42,12 +44,7 @@ export const vatWorkerServiceCommand = { (reply) => reply === null || (isObject(reply) && - hasMarshaledError( - 'required', - reply, - ErrorCode.VatDeleted, - ErrorCode.VatNotFound, - ) && + isMarshaledError(reply.error) && (!hasProperty(reply, 'vatId') || isVatId(reply.vatId))), ), }; From 2b49540242d76b526e92094c6b52231a4e04f135 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Tue, 22 Oct 2024 15:52:30 -0500 Subject: [PATCH 05/12] explicate and harden tests --- .../src/messages/vat-worker-service.test.ts | 92 ++++++++++++------- 1 file changed, 58 insertions(+), 34 deletions(-) diff --git a/packages/kernel/src/messages/vat-worker-service.test.ts b/packages/kernel/src/messages/vat-worker-service.test.ts index 7f0756300..0989dd5b2 100644 --- a/packages/kernel/src/messages/vat-worker-service.test.ts +++ b/packages/kernel/src/messages/vat-worker-service.test.ts @@ -14,39 +14,34 @@ import { } from './vat-worker-service.js'; import type { VatId } from '../types.js'; -const launch: VatWorkerServiceCommandReply['payload'] = { +const launchPayload: VatWorkerServiceCommandReply['payload'] = harden({ method: VatWorkerServiceCommandMethod.Launch, params: { vatId: 'v0' }, -}; -const terminate: VatWorkerServiceCommandReply['payload'] = { +}); +const terminatePayload: VatWorkerServiceCommandReply['payload'] = harden({ method: VatWorkerServiceCommandMethod.Terminate, params: { vatId: 'v0' }, -}; -const terminateAll: VatWorkerServiceCommandReply['payload'] = { +}); +const terminateAllPayload: VatWorkerServiceCommandReply['payload'] = harden({ method: VatWorkerServiceCommandMethod.TerminateAll, params: null, -}; - -const sharedCases = (payload: unknown): [boolean, string, unknown][] => [ - [true, 'valid message id with valid payload', { id: 'm0', payload }], - [false, 'invalid id', { id: 'vat-message-id', payload }], - [false, 'numerical id', { id: 1, payload }], - [false, 'missing payload', { id: 'm0' }], -]; +}); describe('isVatWorkerServiceCommand', () => { describe.each` payload - ${launch} - ${terminate} - ${terminateAll} + ${launchPayload} + ${terminatePayload} + ${terminateAllPayload} `('$payload.method', ({ payload }) => { - it.each(sharedCases(payload))( - 'returns %j for %j', - (expectedResult, _, value) => { - expect(isVatWorkerServiceCommand(value)).toBe(expectedResult); - }, - ); + it.each([ + [true, 'valid message id with valid payload', { id: 'm0', payload }], + [false, 'invalid id', { id: 'vat-message-id', payload }], + [false, 'numerical id', { id: 1, payload }], + [false, 'missing payload', { id: 'm0' }], + ])('returns %j for %j', (expectedResult, _, value) => { + expect(isVatWorkerServiceCommand(value)).toBe(expectedResult); + }); }); }); @@ -61,14 +56,21 @@ describe('isVatWorkerServiceCommandReply', () => { describe('launch', () => { const withMarshaledError = (vatId: VatId): unknown => ({ - method: launch.method, + method: launchPayload.method, params: { - ...launch.params, + ...launchPayload.params, error: marshalError(new VatAlreadyExistsError(vatId)), }, }); it.each([ - ...sharedCases(launch), + [ + true, + 'valid message id with valid payload', + { id: 'm0', payload: launchPayload }, + ], + [false, 'invalid id', { id: 'vat-message-id', payload: launchPayload }], + [false, 'numerical id', { id: 1, payload: launchPayload }], + [false, 'missing payload', { id: 'm0' }], [ true, 'valid message id with valid error', @@ -77,7 +79,7 @@ describe('isVatWorkerServiceCommandReply', () => { [ false, 'valid message id with invalid error', - { id: 'm0', payload: withError(launch, 404) }, + { id: 'm0', payload: withError(launchPayload, 404) }, ], ])('returns %j for %j', (expectedResult, _, value) => { expect(isVatWorkerServiceCommandReply(value)).toBe(expectedResult); @@ -86,14 +88,25 @@ describe('isVatWorkerServiceCommandReply', () => { describe('terminate', () => { const withMarshaledError = (vatId: VatId): unknown => ({ - method: terminate.method, + method: terminatePayload.method, params: { - ...terminate.params, + ...terminatePayload.params, error: marshalError(new VatDeletedError(vatId)), }, }); it.each([ - ...sharedCases(terminate), + [ + true, + 'valid message id with valid payload', + { id: 'm0', payload: terminatePayload }, + ], + [ + false, + 'invalid id', + { id: 'vat-message-id', payload: terminatePayload }, + ], + [false, 'numerical id', { id: 1, payload: terminatePayload }], + [false, 'missing payload', { id: 'm0' }], [ true, 'valid message id with valid error', @@ -102,7 +115,7 @@ describe('isVatWorkerServiceCommandReply', () => { [ false, 'valid message id with invalid error', - { id: 'm0', payload: withError(terminate, 404) }, + { id: 'm0', payload: withError(terminatePayload, 404) }, ], ])('returns %j for %j', (expectedResult, _, value) => { expect(isVatWorkerServiceCommandReply(value)).toBe(expectedResult); @@ -111,15 +124,26 @@ describe('isVatWorkerServiceCommandReply', () => { describe('terminateAll', () => { const withValidVatError = (vatId: VatId): unknown => ({ - method: terminateAll.method, + method: terminateAllPayload.method, params: { vatId, error: marshalError(new VatDeletedError(vatId)) }, }); const withMarshaledError = (): unknown => ({ - method: terminateAll.method, + method: terminateAllPayload.method, params: { error: marshalError(new Error('code: foobar')) }, }); it.each([ - ...sharedCases(terminateAll), + [ + true, + 'valid message id with valid payload', + { id: 'm0', payload: terminateAllPayload }, + ], + [ + false, + 'invalid id', + { id: 'vat-message-id', payload: terminateAllPayload }, + ], + [false, 'numerical id', { id: 1, payload: terminateAllPayload }], + [false, 'missing payload', { id: 'm0' }], [ true, 'valid message id with valid vat error', @@ -133,7 +157,7 @@ describe('isVatWorkerServiceCommandReply', () => { [ false, 'valid message id with invalid error', - { id: 'm0', payload: withError(terminateAll, 404) }, + { id: 'm0', payload: withError(terminateAllPayload, 404) }, ], ])('returns %j for %j', (expectedResult, _, value) => { expect(isVatWorkerServiceCommandReply(value)).toBe(expectedResult); From b063c073059b24603f4600942f662189812f411b Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Tue, 22 Oct 2024 16:06:33 -0500 Subject: [PATCH 06/12] remove error code checking --- packages/kernel/src/messages/vat-worker-service.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/kernel/src/messages/vat-worker-service.ts b/packages/kernel/src/messages/vat-worker-service.ts index cfe621ee4..c797ddeca 100644 --- a/packages/kernel/src/messages/vat-worker-service.ts +++ b/packages/kernel/src/messages/vat-worker-service.ts @@ -1,5 +1,5 @@ import { hasProperty, isObject } from '@metamask/utils'; -import { ErrorCode, isMarshaledError } from '@ocap/errors'; +import { isMarshaledError } from '@ocap/errors'; import type { MarshaledError } from '@ocap/errors'; import type { TypeGuard } from '@ocap/utils'; @@ -7,33 +7,30 @@ import { makeIdentifiedMessageKit, messageType } from './message-kit.js'; import type { VatId } from '../types.js'; import { isVatId } from '../types.js'; -const hasOptionalMarshaledError = (value: object, code: ErrorCode): boolean => - !hasProperty(value, 'error') || - (isMarshaledError(value.error) && value.error.code === code); +const hasOptionalMarshaledError = (value: object): boolean => + !hasProperty(value, 'error') || isMarshaledError(value.error); export const vatWorkerServiceCommand = { Launch: messageType< { vatId: VatId }, - // Expect VatAlreadyExistsError. { vatId: VatId; error?: MarshaledError } >( (send) => isObject(send) && isVatId(send.vatId), (reply) => isObject(reply) && isVatId(reply.vatId) && - hasOptionalMarshaledError(reply, ErrorCode.VatAlreadyExists), + hasOptionalMarshaledError(reply), ), Terminate: messageType< { vatId: VatId }, - // Expect VatDeletedError. { vatId: VatId; error?: MarshaledError } >( (send) => isObject(send) && isVatId(send.vatId), (reply) => isObject(reply) && isVatId(reply.vatId) && - hasOptionalMarshaledError(reply, ErrorCode.VatDeleted), + hasOptionalMarshaledError(reply), ), TerminateAll: messageType< From 47cec523ca08191c84c6d42a2b4f57e2fe58b7a4 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Tue, 22 Oct 2024 16:08:18 -0500 Subject: [PATCH 07/12] shorten vat message id guard --- packages/kernel/src/messages/vat.ts | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/packages/kernel/src/messages/vat.ts b/packages/kernel/src/messages/vat.ts index 5611941e9..e20c2e32b 100644 --- a/packages/kernel/src/messages/vat.ts +++ b/packages/kernel/src/messages/vat.ts @@ -14,17 +14,10 @@ export const vatCommand = { const vatMessageKit = makeIdentifiedMessageKit( vatCommand, - (value: unknown): value is `${VatId}:${number}` => { - if (typeof value !== 'string') { - return false; - } - const parts = value.split(':'); - return ( - parts.length === 2 && - isVatId(parts[0]) && - parts[1] === String(Number(parts[1])) - ); - }, + (value: unknown): value is `${VatId}:${number}` => + typeof value === 'string' && + /^\w+:\d+$/u.test(value) && + isVatId(value.split(':')[0]), ); export const VatCommandMethod = vatMessageKit.methods; From b5430a51be983a5afd5cbb2d7efa43a68fddc526 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Tue, 22 Oct 2024 16:12:29 -0500 Subject: [PATCH 08/12] fix terminateAll docstring --- packages/kernel/src/types.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/kernel/src/types.ts b/packages/kernel/src/types.ts index 308fe1298..bdf120041 100644 --- a/packages/kernel/src/types.ts +++ b/packages/kernel/src/types.ts @@ -35,9 +35,10 @@ export type VatWorkerService = { */ terminate: (vatId: VatId) => Promise; /** - * Terminate all workers known to the service. + * Terminate all workers managed by the service. * - * @returns A promise for the number of workers deleted. + * @returns A promise that resolves after all workers have terminated + * or rejects if there was an error during termination. */ terminateAll: () => Promise; }; From 3dca364a7f07cbe652e646feb3d4956648ceb884 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Tue, 22 Oct 2024 16:58:44 -0500 Subject: [PATCH 09/12] remove makeIsIdentified --- packages/kernel/src/messages/message-kit.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/kernel/src/messages/message-kit.ts b/packages/kernel/src/messages/message-kit.ts index 971d746b3..c191cf1cc 100644 --- a/packages/kernel/src/messages/message-kit.ts +++ b/packages/kernel/src/messages/message-kit.ts @@ -97,14 +97,6 @@ export const makeMessageKit = ( } as MessageKit; }; -const makeIsIdentified = - ( - isId: TypeGuard, - isPayload: TypeGuard, - ) => - (value: unknown): value is { id: Identifier; payload: Payload } => - isObject(value) && isId(value.id) && isPayload(value.payload); - /** * An object type encapsulating all of the schematics that define a functional * group of messages as a payload wrapped with a message id. @@ -133,7 +125,13 @@ export const makeIdentifiedMessageKit = < return { source: messageKit.source, methods: messageKit.methods, - sendGuard: makeIsIdentified(isMessageId, messageKit.sendGuard), - replyGuard: makeIsIdentified(isMessageId, messageKit.replyGuard), + sendGuard: (value: unknown) => + isObject(value) && + isMessageId(value.id) && + messageKit.sendGuard(value.payload), + replyGuard: (value: unknown) => + isObject(value) && + isMessageId(value.id) && + messageKit.replyGuard(value.payload), } as IdentifiedMessageKit; }; From 8c3f2175a4d3bec9bc71d27dfeae379fdd8819d4 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Wed, 23 Oct 2024 09:38:19 -0500 Subject: [PATCH 10/12] identified message kit accepts an object arg --- packages/kernel/src/messages/message-kit.ts | 11 +++++++---- packages/kernel/src/messages/vat-worker-service.ts | 8 ++++---- packages/kernel/src/messages/vat.ts | 8 ++++---- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/kernel/src/messages/message-kit.ts b/packages/kernel/src/messages/message-kit.ts index c191cf1cc..841b843f3 100644 --- a/packages/kernel/src/messages/message-kit.ts +++ b/packages/kernel/src/messages/message-kit.ts @@ -116,10 +116,13 @@ type IdentifiedMessageKit< export const makeIdentifiedMessageKit = < Source extends SourceLike, MessageId extends string, ->( - source: Source, - isMessageId: TypeGuard, -): IdentifiedMessageKit => { +>({ + source, + isMessageId, +}: { + source: Source; + isMessageId: TypeGuard; +}): IdentifiedMessageKit => { const messageKit = makeMessageKit(source); return { diff --git a/packages/kernel/src/messages/vat-worker-service.ts b/packages/kernel/src/messages/vat-worker-service.ts index c797ddeca..1d9f4355f 100644 --- a/packages/kernel/src/messages/vat-worker-service.ts +++ b/packages/kernel/src/messages/vat-worker-service.ts @@ -46,13 +46,13 @@ export const vatWorkerServiceCommand = { ), }; -const messageKit = makeIdentifiedMessageKit( - vatWorkerServiceCommand, - (value: unknown): value is `m${number}` => +const messageKit = makeIdentifiedMessageKit({ + source: vatWorkerServiceCommand, + isMessageId: (value: unknown): value is `m${number}` => typeof value === 'string' && value.at(0) === 'm' && value.slice(1) === String(Number(value.slice(1))), -); +}); export const VatWorkerServiceCommandMethod = messageKit.methods; diff --git a/packages/kernel/src/messages/vat.ts b/packages/kernel/src/messages/vat.ts index e20c2e32b..2ba698102 100644 --- a/packages/kernel/src/messages/vat.ts +++ b/packages/kernel/src/messages/vat.ts @@ -12,13 +12,13 @@ export const vatCommand = { ...vatTestCommand, }; -const vatMessageKit = makeIdentifiedMessageKit( - vatCommand, - (value: unknown): value is `${VatId}:${number}` => +const vatMessageKit = makeIdentifiedMessageKit({ + source: vatCommand, + isMessageId: (value: unknown): value is `${VatId}:${number}` => typeof value === 'string' && /^\w+:\d+$/u.test(value) && isVatId(value.split(':')[0]), -); +}); export const VatCommandMethod = vatMessageKit.methods; From b592ebc888f9a970e503eab507f948bd4633bca2 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Thu, 24 Oct 2024 10:31:39 -0500 Subject: [PATCH 11/12] git apply server-handle-error.patch --- packages/extension/src/VatWorkerServer.ts | 29 +++++++---------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/packages/extension/src/VatWorkerServer.ts b/packages/extension/src/VatWorkerServer.ts index 941536910..4abaa31dd 100644 --- a/packages/extension/src/VatWorkerServer.ts +++ b/packages/extension/src/VatWorkerServer.ts @@ -1,4 +1,3 @@ -import type { OcapError } from '@ocap/errors'; import { VatAlreadyExistsError, VatDeletedError, @@ -77,43 +76,33 @@ export class ExtensionVatWorkerServer { const { id, payload } = event.data; const { method, params } = payload; - const handleProblem = ({ - problem, - vatId, - }: { - problem: OcapError; - vatId: VatId; - }): void => { - this.#logger.error( - `Error handling ${method} for vatId ${vatId}`, - problem, - ); + const handleError = (error: Error, vatId: VatId): void => { + this.#logger.error(`Error handling ${method} for vatId ${vatId}`, error); this.#postMessage({ id, - payload: { method, params: { vatId, error: marshalError(problem) } }, + payload: { method, params: { vatId, error: marshalError(error) } }, }); + throw error; }; switch (method) { case VatWorkerServiceCommandMethod.Launch: await this.#launch(params.vatId) .then((port) => this.#postMessage({ id, payload }, [port])) - .catch(async (problem) => handleProblem({ problem, ...params })); + .catch(async (error) => handleError(error, params.vatId)); break; case VatWorkerServiceCommandMethod.Terminate: await this.#terminate(params.vatId) .then(() => this.#postMessage({ id, payload })) - .catch(async (problem) => handleProblem({ problem, ...params })); + .catch(async (error) => handleError(error, params.vatId)); break; case VatWorkerServiceCommandMethod.TerminateAll: await Promise.all( Array.from(this.#vatWorkers.keys()).map(async (vatId) => - this.#terminate(vatId).catch((problem) => { - // eslint-disable-next-line @typescript-eslint/only-throw-error - throw { problem, vatId }; - }), + this.#terminate(vatId).catch((error) => handleError(error, vatId)), ), - ).then(() => this.#postMessage({ id, payload }), handleProblem); + ); + this.#postMessage({ id, payload }); break; /* v8 ignore next 6: Not known to be possible. */ default: From 7b67fa8d748d599ac5f57531ed6bfa5f89e0bf36 Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Thu, 24 Oct 2024 11:16:03 -0500 Subject: [PATCH 12/12] cleanup vws extension tests --- .../extension/src/VatWorkerServer.test.ts | 32 ++++------ .../extension/src/vat-worker-service.test.ts | 14 ++--- packages/extension/test/vat-worker-service.ts | 60 +++++++++---------- 3 files changed, 46 insertions(+), 60 deletions(-) diff --git a/packages/extension/src/VatWorkerServer.test.ts b/packages/extension/src/VatWorkerServer.test.ts index f14dcc5f2..c5390b25a 100644 --- a/packages/extension/src/VatWorkerServer.test.ts +++ b/packages/extension/src/VatWorkerServer.test.ts @@ -1,4 +1,6 @@ import '@ocap/shims/endoify'; +import type { NonEmptyArray } from '@metamask/utils'; +import { VatNotFoundError } from '@ocap/errors'; import { VatWorkerServiceCommandMethod } from '@ocap/kernel'; import { delay } from '@ocap/test-utils'; import type { Logger } from '@ocap/utils'; @@ -7,10 +9,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import type { VatWorker } from './vat-worker-service.js'; import type { ExtensionVatWorkerServer } from './VatWorkerServer.js'; -import { - getMockMakeWorker, - makeTestServer, -} from '../test/vat-worker-service.js'; +import { makeTestServer } from '../test/vat-worker-service.js'; describe('ExtensionVatWorkerServer', () => { let serverPort: MessagePort; @@ -20,24 +19,17 @@ describe('ExtensionVatWorkerServer', () => { let server: ExtensionVatWorkerServer; - // let vatPort: MessagePort; - let kernelPort: MessagePort; - beforeEach(() => { const serviceMessageChannel = new MessageChannel(); serverPort = serviceMessageChannel.port1; clientPort = serviceMessageChannel.port2; logger = makeLogger('[test server]'); - - const deliveredMessageChannel = new MessageChannel(); - // vatPort = deliveredMessageChannel.port1; - kernelPort = deliveredMessageChannel.port2; }); describe('Misc', () => { beforeEach(() => { - server = makeTestServer({ serverPort, logger, kernelPort }); + [server] = makeTestServer({ serverPort, logger }); }); it('starts', () => { @@ -65,18 +57,20 @@ describe('ExtensionVatWorkerServer', () => { }); describe('terminateAll', () => { - let workers: VatWorker[]; - let makeWorker: (vatId: VatId) => VatWorker; + let workers: NonEmptyArray; beforeEach(() => { - [makeWorker, ...workers] = getMockMakeWorker(3); - server = makeTestServer({ serverPort, logger, makeWorker }); + [server, ...workers] = makeTestServer({ + serverPort, + logger, + nWorkers: 3, + }); }); it('calls logger.error when a vat fails to terminate', async () => { const errorSpy = vi.spyOn(logger, 'error'); const vatId = 'v0'; - const vatNotFoundError = new Error(`vat not found: ${vatId}`); + const vatNotFoundError = new VatNotFoundError(vatId); vi.spyOn(workers[0], 'terminate').mockRejectedValue(vatNotFoundError); server.start(); clientPort.postMessage({ @@ -97,10 +91,10 @@ describe('ExtensionVatWorkerServer', () => { await delay(100); expect(errorSpy).toHaveBeenCalledOnce(); - expect(errorSpy.mock.lastCall[0]).toBe( + expect(errorSpy.mock.lastCall?.[0]).toBe( `Error handling ${VatWorkerServiceCommandMethod.TerminateAll} for vatId ${vatId}`, ); - expect(errorSpy.mock.lastCall[1]).toBe(vatNotFoundError); + expect(errorSpy.mock.lastCall?.[1]).toBe(vatNotFoundError); }); }); }); diff --git a/packages/extension/src/vat-worker-service.test.ts b/packages/extension/src/vat-worker-service.test.ts index f96ef9261..aed974312 100644 --- a/packages/extension/src/vat-worker-service.test.ts +++ b/packages/extension/src/vat-worker-service.test.ts @@ -1,4 +1,5 @@ import '@ocap/shims/endoify'; +import type { NonEmptyArray } from '@metamask/utils'; import { VatAlreadyExistsError, VatDeletedError } from '@ocap/errors'; import type { VatId } from '@ocap/kernel'; import { MessagePortDuplexStream } from '@ocap/streams'; @@ -8,11 +9,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import type { VatWorker } from './vat-worker-service.js'; import type { ExtensionVatWorkerClient } from './VatWorkerClient.js'; import type { ExtensionVatWorkerServer } from './VatWorkerServer.js'; -import { - getMockMakeWorker, - makeTestClient, - makeTestServer, -} from '../test/vat-worker-service.js'; +import { makeTestClient, makeTestServer } from '../test/vat-worker-service.js'; // low key integration test describe('VatWorkerService', () => { @@ -23,9 +20,8 @@ describe('VatWorkerService', () => { let client: ExtensionVatWorkerClient; let mockWorker: VatWorker; - let mockWorkers: VatWorker[]; + let mockWorkers: NonEmptyArray; - let mockMakeWorker: (vatId: VatId) => VatWorker; let mockLaunchWorker: MockInstance; let mockTerminateWorker: MockInstance; @@ -34,10 +30,8 @@ describe('VatWorkerService', () => { serverPort = serviceMessageChannel.port1; clientPort = serviceMessageChannel.port2; - [mockMakeWorker, ...mockWorkers] = getMockMakeWorker(3); - client = makeTestClient(clientPort); - server = makeTestServer({ serverPort, makeWorker: mockMakeWorker }); + [server, ...mockWorkers] = makeTestServer({ serverPort, nWorkers: 3 }); server.start(); }); diff --git a/packages/extension/test/vat-worker-service.ts b/packages/extension/test/vat-worker-service.ts index 61a7d3308..541bffb1f 100644 --- a/packages/extension/test/vat-worker-service.ts +++ b/packages/extension/test/vat-worker-service.ts @@ -1,3 +1,4 @@ +import type { NonEmptyArray } from '@metamask/utils'; import type { VatId } from '@ocap/kernel'; import { makeCounter } from '@ocap/utils'; import type { Logger } from '@ocap/utils'; @@ -7,11 +8,15 @@ import type { VatWorker } from '../src/vat-worker-service.js'; import { ExtensionVatWorkerClient } from '../src/VatWorkerClient.js'; import { ExtensionVatWorkerServer } from '../src/VatWorkerServer.js'; -type MakeVatWorker = (vatId: VatId) => VatWorker & { kernelPort: MessagePort }; - -export const getMockMakeWorker = ( +const getMockMakeWorker = ( nWorkers: number = 1, -): [MakeVatWorker, ...VatWorker[]] => { +): [ + (vatId: VatId) => VatWorker & { kernelPort: MessagePort }, + ...NonEmptyArray, +] => { + if (typeof nWorkers !== 'number' || nWorkers < 1) { + throw new Error('invalid argument: nWorkers must be > 0'); + } const counter = makeCounter(-1); const mockWorkers = Array(nWorkers) .fill(0) @@ -26,7 +31,7 @@ export const getMockMakeWorker = ( // vatPort, kernelPort, }; - }); + }) as unknown as NonEmptyArray; return [ vi.fn().mockImplementation(() => mockWorkers[counter()]), @@ -46,31 +51,24 @@ export const makeTestClient = ( logger, ); -type MakeTestServerArgs = { +export const makeTestServer = (args: { serverPort: MessagePort; logger?: Logger; -} & ( - | { - makeWorker: MakeVatWorker; - kernelPort?: never; - } - | { - makeWorker?: never; - kernelPort: MessagePort; - } -); - -export const makeTestServer = ( - args: MakeTestServerArgs, -): ExtensionVatWorkerServer => - new ExtensionVatWorkerServer( - (message: unknown, transfer?: Transferable[]) => - transfer - ? args.serverPort.postMessage(message, transfer) - : args.serverPort.postMessage(message), - (listener) => { - args.serverPort.onmessage = listener; - }, - args.makeWorker ?? getMockMakeWorker(args.kernelPort)[1], - args.logger, - ); + nWorkers?: number; +}): [ExtensionVatWorkerServer, ...NonEmptyArray] => { + const [makeWorker, ...workers] = getMockMakeWorker(args.nWorkers); + return [ + new ExtensionVatWorkerServer( + (message: unknown, transfer?: Transferable[]) => + transfer + ? args.serverPort.postMessage(message, transfer) + : args.serverPort.postMessage(message), + (listener) => { + args.serverPort.onmessage = listener; + }, + makeWorker, + args.logger, + ), + ...workers, + ]; +};