diff --git a/packages/extension/src/kernel-integration/VatWorkerClient.ts b/packages/extension/src/kernel-integration/VatWorkerClient.ts index dc25c33af..3803d2d85 100644 --- a/packages/extension/src/kernel-integration/VatWorkerClient.ts +++ b/packages/extension/src/kernel-integration/VatWorkerClient.ts @@ -1,9 +1,5 @@ import { isJsonRpcResponse } from '@metamask/utils'; -import type { - JsonRpcId, - JsonRpcRequest, - JsonRpcResponse, -} from '@metamask/utils'; +import type { JsonRpcId, JsonRpcResponse } from '@metamask/utils'; import type { VatWorkerManager, VatId, VatConfig } from '@ocap/kernel'; import { vatWorkerServiceMethodSpecs } from '@ocap/kernel/rpc'; import { RpcClient } from '@ocap/rpc-methods'; @@ -16,7 +12,7 @@ import type { PostMessageEnvelope, PostMessageTarget, } from '@ocap/streams/browser'; -import type { JsonRpcMessage, Logger } from '@ocap/utils'; +import type { JsonRpcCall, JsonRpcMessage, Logger } from '@ocap/utils'; import { isJsonRpcMessage, makeLogger, stringify } from '@ocap/utils'; // Appears in the docs. @@ -25,7 +21,7 @@ import type { ExtensionVatWorkerService } from './VatWorkerServer.ts'; export type VatWorkerClientStream = PostMessageDuplexStream< MessageEvent, - PostMessageEnvelope + PostMessageEnvelope >; export class ExtensionVatWorkerClient implements VatWorkerManager { @@ -61,8 +57,10 @@ export class ExtensionVatWorkerClient implements VatWorkerManager { this.#rpcClient = new RpcClient( vatWorkerServiceMethodSpecs, async (request) => { - if (request.method === 'launch') { - this.#portMap.set(request.id, undefined); + if ('id' in request) { + if (request.method === 'launch') { + this.#portMap.set(request.id, undefined); + } } await this.#stream.write({ payload: request, transfer: [] }); }, diff --git a/packages/extension/src/kernel-integration/kernel-worker.ts b/packages/extension/src/kernel-integration/kernel-worker.ts index 58abf19c5..901764a15 100644 --- a/packages/extension/src/kernel-integration/kernel-worker.ts +++ b/packages/extension/src/kernel-integration/kernel-worker.ts @@ -1,6 +1,5 @@ import { JsonRpcEngine } from '@metamask/json-rpc-engine'; import type { JsonRpcRequest, JsonRpcResponse } from '@metamask/utils'; -import { isJsonRpcRequest } from '@metamask/utils'; import type { ClusterConfig } from '@ocap/kernel'; import { ClusterConfigStruct, Kernel } from '@ocap/kernel'; import { makeSQLKernelDatabase } from '@ocap/store/sqlite/wasm'; @@ -9,7 +8,8 @@ import { MessagePortDuplexStream, receiveMessagePort, } from '@ocap/streams/browser'; -import { fetchValidatedJson, Logger } from '@ocap/utils'; +import { fetchValidatedJson, isJsonRpcCall, Logger } from '@ocap/utils'; +import type { JsonRpcCall } from '@ocap/utils'; import { makeLoggingMiddleware } from './middleware/logging.ts'; import { createPanelMessageMiddleware } from './middleware/panel-message.ts'; @@ -39,9 +39,9 @@ async function main(): Promise { ); const kernelStream = await MessagePortDuplexStream.make< - JsonRpcRequest, + JsonRpcCall, JsonRpcResponse - >(port, isJsonRpcRequest); + >(port, isJsonRpcCall); // Initialize kernel dependencies const vatWorkerClient = ExtensionVatWorkerClient.make( @@ -63,7 +63,11 @@ async function main(): Promise { const kernelEngine = new JsonRpcEngine(); kernelEngine.push(makeLoggingMiddleware(logger.subLogger('kernel-command'))); kernelEngine.push(createPanelMessageMiddleware(kernel, kernelDatabase)); - receiveUiConnections(async (request) => kernelEngine.handle(request), logger); + // JsonRpcEngine type error: does not handle JSON-RPC notifications + receiveUiConnections( + async (request) => kernelEngine.handle(request as JsonRpcRequest), + logger, + ); const launchDefaultSubcluster = firstTime || ALWAYS_RESET_STORAGE; const defaultSubcluster = await fetchValidatedJson( diff --git a/packages/extension/src/kernel-integration/ui-connections.test.ts b/packages/extension/src/kernel-integration/ui-connections.test.ts index ac2d11b4a..c6262bbef 100644 --- a/packages/extension/src/kernel-integration/ui-connections.test.ts +++ b/packages/extension/src/kernel-integration/ui-connections.test.ts @@ -1,8 +1,8 @@ -import type { JsonRpcRequest, JsonRpcResponse } from '@metamask/utils'; +import type { JsonRpcResponse } from '@metamask/utils'; import type { PostMessageTarget } from '@ocap/streams/browser'; import { delay } from '@ocap/test-utils'; import { TestDuplexStream } from '@ocap/test-utils/streams'; -import type { Logger } from '@ocap/utils'; +import type { JsonRpcCall, Logger } from '@ocap/utils'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { @@ -161,7 +161,7 @@ describe('ui-connections', () => { const logger = makeMockLogger(); const mockHandleMessage = vi.fn( - async (_request: JsonRpcRequest): Promise => ({ + async (_request: JsonRpcCall): Promise => ({ id: 'foo', jsonrpc: '2.0' as const, result: { vats: [], clusterConfig }, diff --git a/packages/extension/src/kernel-integration/ui-connections.ts b/packages/extension/src/kernel-integration/ui-connections.ts index 1239282c2..bfe762505 100644 --- a/packages/extension/src/kernel-integration/ui-connections.ts +++ b/packages/extension/src/kernel-integration/ui-connections.ts @@ -1,8 +1,8 @@ import { isJsonRpcRequest, isJsonRpcResponse } from '@metamask/utils'; -import type { JsonRpcRequest, JsonRpcResponse } from '@metamask/utils'; +import type { JsonRpcResponse } from '@metamask/utils'; import { PostMessageDuplexStream } from '@ocap/streams/browser'; import { stringify } from '@ocap/utils'; -import type { Logger } from '@ocap/utils'; +import type { JsonRpcCall, Logger } from '@ocap/utils'; import { nanoid } from 'nanoid'; import { isUiControlCommand } from './ui-control-command.ts'; @@ -11,18 +11,16 @@ import type { UiControlCommand } from './ui-control-command.ts'; export const UI_CONTROL_CHANNEL_NAME = 'ui-control'; export type KernelControlStream = PostMessageDuplexStream< - JsonRpcRequest, + JsonRpcCall, JsonRpcResponse >; export type KernelControlReplyStream = PostMessageDuplexStream< JsonRpcResponse, - JsonRpcRequest + JsonRpcCall >; -type HandleInstanceMessage = ( - request: JsonRpcRequest, -) => Promise; +type HandleInstanceMessage = (request: JsonRpcCall) => Promise; /** * Establishes a connection between a UI instance and the kernel. Should be called @@ -45,7 +43,7 @@ export const establishKernelConnection = async ( const kernelStream = await PostMessageDuplexStream.make< JsonRpcResponse, - JsonRpcRequest + JsonRpcCall >({ validateInput: isJsonRpcResponse, messageTarget: instanceChannel, diff --git a/packages/extension/src/offscreen.ts b/packages/extension/src/offscreen.ts index b10e75f0f..d2547283a 100644 --- a/packages/extension/src/offscreen.ts +++ b/packages/extension/src/offscreen.ts @@ -1,5 +1,5 @@ -import { isJsonRpcRequest, isJsonRpcResponse } from '@metamask/utils'; -import type { JsonRpcRequest, JsonRpcResponse } from '@metamask/utils'; +import { isJsonRpcResponse } from '@metamask/utils'; +import type { JsonRpcResponse } from '@metamask/utils'; import type { DuplexStream } from '@ocap/streams'; import { initializeMessageChannel, @@ -7,7 +7,8 @@ import { MessagePortDuplexStream, } from '@ocap/streams/browser'; import type { PostMessageTarget } from '@ocap/streams/browser'; -import { delay, Logger } from '@ocap/utils'; +import { delay, isJsonRpcCall, Logger } from '@ocap/utils'; +import type { JsonRpcCall } from '@ocap/utils'; import { makeIframeVatWorker } from './kernel-integration/iframe-vat-worker.ts'; import { ExtensionVatWorkerService } from './kernel-integration/VatWorkerServer.ts'; @@ -25,9 +26,9 @@ async function main(): Promise { // Create stream for messages from the background script const backgroundStream = await ChromeRuntimeDuplexStream.make< - JsonRpcRequest, + JsonRpcCall, JsonRpcResponse - >(chrome.runtime, 'offscreen', 'background', isJsonRpcRequest); + >(chrome.runtime, 'offscreen', 'background', isJsonRpcCall); const { kernelStream, vatWorkerService } = await makeKernelWorker(); @@ -45,7 +46,7 @@ async function main(): Promise { * @returns The message port stream for worker communication */ async function makeKernelWorker(): Promise<{ - kernelStream: DuplexStream; + kernelStream: DuplexStream; vatWorkerService: ExtensionVatWorkerService; }> { const worker = new Worker('kernel-worker.js', { type: 'module' }); @@ -56,7 +57,7 @@ async function makeKernelWorker(): Promise<{ const kernelStream = await MessagePortDuplexStream.make< JsonRpcResponse, - JsonRpcRequest + JsonRpcCall >(port, isJsonRpcResponse); const vatWorkerService = ExtensionVatWorkerService.make( diff --git a/packages/kernel/src/Kernel.ts b/packages/kernel/src/Kernel.ts index 5dc9bcf50..4e3ac8793 100644 --- a/packages/kernel/src/Kernel.ts +++ b/packages/kernel/src/Kernel.ts @@ -1,6 +1,7 @@ import type { CapData } from '@endo/marshal'; import { serializeError } from '@metamask/rpc-errors'; -import type { JsonRpcRequest, JsonRpcResponse } from '@metamask/utils'; +import { hasProperty } from '@metamask/utils'; +import type { JsonRpcResponse } from '@metamask/utils'; import { StreamReadError, VatAlreadyExistsError, @@ -11,6 +12,7 @@ import type { ExtractParams, ExtractResult } from '@ocap/rpc-methods'; import type { KernelDatabase } from '@ocap/store'; import type { DuplexStream } from '@ocap/streams'; import { Logger } from '@ocap/utils'; +import type { JsonRpcCall } from '@ocap/utils'; import { KernelQueue } from './KernelQueue.ts'; import { KernelRouter } from './KernelRouter.ts'; @@ -33,7 +35,7 @@ import { VatHandle } from './VatHandle.ts'; export class Kernel { /** Command channel from the controlling console/browser extension/test driver */ - readonly #commandStream: DuplexStream; + readonly #commandStream: DuplexStream; readonly #rpcService: RpcService; @@ -70,7 +72,7 @@ export class Kernel { */ // eslint-disable-next-line no-restricted-syntax private constructor( - commandStream: DuplexStream, + commandStream: DuplexStream, vatWorkerService: VatWorkerManager, kernelDatabase: KernelDatabase, options: { @@ -108,7 +110,7 @@ export class Kernel { * @returns A promise for the new kernel instance. */ static async make( - commandStream: DuplexStream, + commandStream: DuplexStream, vatWorkerService: VatWorkerManager, kernelDatabase: KernelDatabase, options: { @@ -155,25 +157,29 @@ export class Kernel { * * @param message - The message to handle. */ - async #handleCommandMessage(message: JsonRpcRequest): Promise { + async #handleCommandMessage(message: JsonRpcCall): Promise { try { this.#rpcService.assertHasMethod(message.method); const result = await this.#rpcService.execute( message.method, message.params, ); - await this.#commandStream.write({ - id: message.id, - jsonrpc: '2.0', - result, - }); + if (hasProperty(message, 'id') && typeof message.id === 'string') { + await this.#commandStream.write({ + id: message.id, + jsonrpc: '2.0', + result, + }); + } } catch (error) { this.#logger.error('Error executing command', error); - await this.#commandStream.write({ - id: message.id, - jsonrpc: '2.0', - error: serializeError(error), - }); + if (hasProperty(message, 'id') && typeof message.id === 'string') { + await this.#commandStream.write({ + id: message.id, + jsonrpc: '2.0', + error: serializeError(error), + }); + } } } diff --git a/packages/kernel/src/rpc/kernel/index.ts b/packages/kernel/src/rpc/kernel/index.ts index 44e64d7f6..4b2b223fd 100644 --- a/packages/kernel/src/rpc/kernel/index.ts +++ b/packages/kernel/src/rpc/kernel/index.ts @@ -1,14 +1,18 @@ -import type { MethodRequest } from '@ocap/rpc-methods'; +import type { + HandlerRecord, + MethodRequest, + MethodSpecRecord, +} from '@ocap/rpc-methods'; import { pingHandler, pingSpec } from '../vat/ping.ts'; export const kernelHandlers = { ping: pingHandler, -} as const; +} as HandlerRecord; export const kernelMethodSpecs = { ping: pingSpec, -} as const; +} as MethodSpecRecord; type Handlers = (typeof kernelHandlers)[keyof typeof kernelHandlers]; diff --git a/packages/kernel/src/rpc/vat-syscall/index.ts b/packages/kernel/src/rpc/vat-syscall/index.ts index 2416dbf30..7551bfa4b 100644 --- a/packages/kernel/src/rpc/vat-syscall/index.ts +++ b/packages/kernel/src/rpc/vat-syscall/index.ts @@ -1,12 +1,14 @@ +import type { MethodSpecRecord, HandlerRecord } from '@ocap/rpc-methods'; + import { vatSyscallSpec, vatSyscallHandler } from './vat-syscall.ts'; export const vatSyscallHandlers = { syscall: vatSyscallHandler, -} as const; +} as HandlerRecord; export const vatSyscallMethodSpecs = { syscall: vatSyscallSpec, -} as const; +} as MethodSpecRecord; type Handlers = (typeof vatSyscallHandlers)[keyof typeof vatSyscallHandlers]; diff --git a/packages/rpc-methods/src/RpcClient.test.ts b/packages/rpc-methods/src/RpcClient.test.ts index 81d8ef829..e77e1e312 100644 --- a/packages/rpc-methods/src/RpcClient.test.ts +++ b/packages/rpc-methods/src/RpcClient.test.ts @@ -69,6 +69,33 @@ describe('RpcClient', () => { }); }); + describe('notify', () => { + it('should call a notification method', async () => { + const sendMessage = vi.fn(async () => Promise.resolve()); + const client = new RpcClient(getMethods(), sendMessage, 'test'); + await client.notify('method3', ['test']); + expect(sendMessage).toHaveBeenCalledWith({ + jsonrpc: jsonrpc2, + method: 'method3', + params: ['test'], + }); + }); + + it('should log an error if the message fails to send', async () => { + const logger = makeLogger('[test]'); + const sendMessage = vi.fn(async () => + Promise.reject(new Error('test error')), + ); + const client = new RpcClient(getMethods(), sendMessage, 'test', logger); + const logError = vi.spyOn(logger, 'error'); + await client.notify('method3', ['test']); + expect(logError).toHaveBeenCalledWith( + 'Failed to send notification', + new Error('test error'), + ); + }); + }); + describe('callAndGetId', () => { it('should call a method and return the id', async () => { const client = new RpcClient(getMethods(), vi.fn(), 'test'); diff --git a/packages/rpc-methods/src/RpcClient.ts b/packages/rpc-methods/src/RpcClient.ts index e9835bf88..182b9edaa 100644 --- a/packages/rpc-methods/src/RpcClient.ts +++ b/packages/rpc-methods/src/RpcClient.ts @@ -1,7 +1,11 @@ import { makePromiseKit } from '@endo/promise-kit'; import { assert as assertStruct } from '@metamask/superstruct'; import { isJsonRpcFailure, isJsonRpcSuccess } from '@metamask/utils'; -import type { JsonRpcRequest, JsonRpcSuccess } from '@metamask/utils'; +import type { + JsonRpcNotification, + JsonRpcRequest, + JsonRpcSuccess, +} from '@metamask/utils'; import { makeCounter, makeLogger, stringify } from '@ocap/utils'; import type { Logger, PromiseCallbacks } from '@ocap/utils'; @@ -9,11 +13,14 @@ import type { MethodSpec, ExtractParams, ExtractResult, - ExtractMethod, MethodSpecRecord, + ExtractNotification, + ExtractRequest, } from './types.ts'; -export type SendMessage = (payload: JsonRpcRequest) => Promise; +export type SendMessage = ( + payload: JsonRpcRequest | JsonRpcNotification, +) => Promise; export class RpcClient< // The class picks up its type from the `methods` argument, @@ -45,7 +52,7 @@ export class RpcClient< this.#logger = logger; } - async #call>( + async #call>( method: Method, params: ExtractParams, id: string, @@ -68,13 +75,33 @@ export class RpcClient< * @param params - The parameters to pass to the method. * @returns A promise that resolves to the result. */ - async call>( + async call>( method: Method, params: ExtractParams, ): Promise> { return await this.#call(method, params, this.#nextMessageId()); } + /** + * Sends a JSON-RPC notification. Recall that we do not receive responses to notifications + * for any reason. + * + * @param method - The method to notify. + * @param params - The parameters to pass to the method. + */ + async notify>( + method: Method, + params: ExtractParams, + ): Promise { + await this.#sendMessage({ + jsonrpc: '2.0', + method, + params, + }).catch((error) => + this.#logger.error(`Failed to send notification`, error), + ); + } + /** * Calls a JSON-RPC method and returns the message id and the result. * @@ -82,7 +109,7 @@ export class RpcClient< * @param params - The parameters to pass to the method. * @returns A promise that resolves to a tuple of the message id and the result. */ - async callAndGetId>( + async callAndGetId>( method: Method, params: ExtractParams, ): Promise<[string, ExtractResult]> { @@ -90,7 +117,7 @@ export class RpcClient< return [id, await this.#call(method, params, id)]; } - #assertResult>( + #assertResult>( method: Method, result: unknown, ): asserts result is ExtractResult { diff --git a/packages/rpc-methods/src/RpcService.test.ts b/packages/rpc-methods/src/RpcService.test.ts index a2d929461..3c37a9747 100644 --- a/packages/rpc-methods/src/RpcService.test.ts +++ b/packages/rpc-methods/src/RpcService.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { RpcService } from './RpcService.ts'; import { getHandlers, getHooks } from '../test/methods.ts'; @@ -20,7 +20,7 @@ describe('RpcService', () => { it('should throw if the method is not found', () => { const service = new RpcService(getHandlers(), getHooks()); - expect(() => service.assertHasMethod('method3')).toThrow( + expect(() => service.assertHasMethod('does-not-exist')).toThrow( 'The method does not exist / is not available.', ); }); @@ -28,8 +28,20 @@ describe('RpcService', () => { describe('execute', () => { it('should execute a method', async () => { - const service = new RpcService(getHandlers(), getHooks()); + const hooks = getHooks(); + const hookSpy = vi.spyOn(hooks, 'hook1'); + const service = new RpcService(getHandlers(), hooks); expect(await service.execute('method1', ['test'])).toBeNull(); + expect(hookSpy).toHaveBeenCalledOnce(); + }); + + it('should execute a notification method', async () => { + const hooks = getHooks(); + const hookSpy = vi.spyOn(hooks, 'hook4'); + const service = new RpcService(getHandlers(), hooks); + expect(await service.execute('method3', ['test'])).toBeUndefined(); + expect(hookSpy).toHaveBeenCalledOnce(); + expect(hookSpy).toHaveBeenCalledWith('test'); }); it('should be able to execute a method that uses a hook', async () => { @@ -40,7 +52,7 @@ describe('RpcService', () => { it('should throw an error if the method is not found', async () => { const service = new RpcService(getHandlers(), getHooks()); // @ts-expect-error Intentional destructive testing - await expect(service.execute('method3', [2])).rejects.toThrow( + await expect(service.execute('does-not-exist', [2])).rejects.toThrow( // This is not a _good_ error, but we only care about type safety in this instance. "Cannot read properties of undefined (reading 'params')", ); diff --git a/packages/rpc-methods/src/RpcService.ts b/packages/rpc-methods/src/RpcService.ts index c5336ec14..1073252c3 100644 --- a/packages/rpc-methods/src/RpcService.ts +++ b/packages/rpc-methods/src/RpcService.ts @@ -21,7 +21,7 @@ type HandlerRecord< Handlers extends Handler< string, JsonRpcParams, - Json, + Json | void, Record >, > = Record; @@ -83,11 +83,8 @@ export class RpcService< const handler = this.#getHandler(method); assertParams(params, handler.params); - // Select only the hooks that the handler needs - const selectedHooks = selectHooks(this.#hooks, handler.hooks); - - // Execute the handler with the selected hooks - return await handler.implementation(selectedHooks, params); + const expectedHooks = selectHooks(this.#hooks, handler.hooks); + return await handler.implementation(expectedHooks, params); } /** diff --git a/packages/rpc-methods/src/types.ts b/packages/rpc-methods/src/types.ts index 6a020e09f..9cd9125f5 100644 --- a/packages/rpc-methods/src/types.ts +++ b/packages/rpc-methods/src/types.ts @@ -12,12 +12,14 @@ export type MethodSignature< export type MethodSpec< Method extends string, Params extends JsonRpcParams, - Result extends Json | Promise, -> = { - method: Method; - params: Struct; - result: Struct>; -}; + Result extends Json | Promise | void, +> = Result extends void + ? { method: Method; params: Struct; result?: undefined } + : { + method: Method; + params: Struct; + result: Struct>; + }; // `any` can safely be used in constraints. // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -45,6 +47,16 @@ export type ExtractMethodSpec< export type ExtractMethod = ExtractMethodSpec['method']; +export type ExtractRequest = + // Safe to use `any` in constraints. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Extract, { result: Struct }>['method']; + +export type ExtractNotification = Extract< + ExtractMethodSpec, + { result?: undefined } +>['method']; + export type ExtractParams< Method extends string, Specs extends SpecRecordConstraint, @@ -53,11 +65,16 @@ export type ExtractParams< export type ExtractResult< Method extends string, Specs extends SpecRecordConstraint, -> = UnwrapPromise['result']>>; +> = + // Safe to use `any` in constraints. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ExtractMethodSpec extends MethodSpec + ? UnwrapPromise + : never; export type HandlerFunction< Params extends JsonRpcParams, - Result extends Json | Promise, + Result extends Json | Promise | void, Hooks extends Record, > = (hooks: Hooks, params: Params) => Result; @@ -66,7 +83,7 @@ export type HandlerFunction< export type Handler< Method extends string, Params extends JsonRpcParams, - Result extends Json | Promise, + Result extends Json | Promise | void, Hooks extends Record, > = MethodSpec & { hooks: { [Key in keyof Hooks]: true }; diff --git a/packages/rpc-methods/test/methods.ts b/packages/rpc-methods/test/methods.ts index 178deb995..d27ab7967 100644 --- a/packages/rpc-methods/test/methods.ts +++ b/packages/rpc-methods/test/methods.ts @@ -7,6 +7,7 @@ export const getHooks = () => hook1: () => undefined, hook2: () => undefined, hook3: () => undefined, + hook4: (value: string) => value, }) as const; export type Hooks = ReturnType; @@ -23,6 +24,10 @@ export const getMethods = () => params: tuple([number()]), result: number(), } as MethodSpec<'method2', [number], number>, + method3: { + method: 'method3', + params: tuple([string()]), + } as MethodSpec<'method3', [string], void>, }) as const; export const getHandlers = () => { @@ -49,6 +54,13 @@ export const getHandlers = () => { return value * 2; }, } as Handler<'method2', [number], number, Pick>, + method3: { + ...methods.method3, + hooks: { hook4: true } as const, + implementation: (hooks, [value]) => { + hooks.hook4(value); + }, + } as Handler<'method3', [string], void, Pick>, }; }; diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index f8d18bcf8..c2f213b70 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -3,6 +3,7 @@ export { delay, makeCounter } from './misc.ts'; export { stringify } from './stringify.ts'; export type { ExtractGuardType, + JsonRpcCall, JsonRpcMessage, PromiseCallbacks, TypeGuard, @@ -12,6 +13,7 @@ export { isPrimitive, isTypedArray, isTypedObject, + isJsonRpcCall, isJsonRpcMessage, } from './types.ts'; export { fetchValidatedJson } from './fetchValidatedJson.ts'; diff --git a/packages/utils/src/misc.test.ts b/packages/utils/src/misc.test.ts index e8ba50dac..b51e39ded 100644 --- a/packages/utils/src/misc.test.ts +++ b/packages/utils/src/misc.test.ts @@ -32,4 +32,10 @@ describe('delay', () => { vi.advanceTimersByTime(delayTime); expect(await delayP).toBeUndefined(); }); + + it('delays execution by the default number of milliseconds', async () => { + const delayP = delay(); + vi.advanceTimersByTime(1); + expect(await delayP).toBeUndefined(); + }); }); diff --git a/packages/utils/src/types.test.ts b/packages/utils/src/types.test.ts index 71ac8b623..3b61fefb7 100644 --- a/packages/utils/src/types.test.ts +++ b/packages/utils/src/types.test.ts @@ -1,7 +1,13 @@ import { isObject } from '@metamask/utils'; import { describe, it, expect } from 'vitest'; -import { isPrimitive, isTypedArray, isTypedObject } from './types.ts'; +import { + isJsonRpcCall, + isJsonRpcMessage, + isPrimitive, + isTypedArray, + isTypedObject, +} from './types.ts'; const isNumber = (value: unknown): value is number => typeof value === 'number'; const alwaysFalse = (): boolean => false; @@ -85,3 +91,65 @@ describe('isTypedObject', () => { expect(isTypedObject(value, guard)).toBe(false); }); }); + +describe('isJsonRpcCall', () => { + it.each` + value + ${{ jsonrpc: '2.0', id: '1', method: 'foo', params: [] }} + ${{ jsonrpc: '2.0', id: '1', method: 'foo', params: {} }} + ${{ jsonrpc: '2.0', method: 'foo', params: [] }} + ${{ jsonrpc: '2.0', method: 'foo', params: {} }} + `('returns true for valid JSON-RPC call $value', ({ value }) => { + expect(isJsonRpcCall(value)).toBe(true); + }); + + it.each` + value + ${null} + ${undefined} + ${'foo'} + ${[]} + ${{}} + ${{ jsonrpc: '2.0', id: '1', result: { foo: 'bar' } }} + ${{ jsonrpc: '2.0', id: '1', error: { code: 1, message: 'foo' } }} + ${{ id: '1', method: 'foo', params: [1, 2, 3] }} + ${{ jsonrpc: '2.0', id: '1', params: { foo: 'bar' } }} + ${{ jsonrpc: '2.0', result: 'foo', params: [1, 2, 3] }} + ${{ jsonrpc: '2.0', error: 'foo', params: { foo: 'bar' } }} + `('returns false for invalid values: $value', ({ value }) => { + expect(isJsonRpcCall(value)).toBe(false); + }); +}); + +describe('isJsonRpcMessage', () => { + it.each` + value + ${{ jsonrpc: '2.0', id: '1', method: 'foo', params: [] }} + ${{ jsonrpc: '2.0', id: '1', method: 'foo', params: {} }} + ${{ jsonrpc: '2.0', method: 'foo', params: [] }} + ${{ jsonrpc: '2.0', method: 'foo', params: {} }} + ${{ jsonrpc: '2.0', id: '1', method: 'foo', params: [] }} + ${{ jsonrpc: '2.0', id: '1', method: 'foo', params: {} }} + ${{ jsonrpc: '2.0', method: 'foo', params: [] }} + ${{ jsonrpc: '2.0', method: 'foo', params: {} }} + ${{ jsonrpc: '2.0', id: '1', result: { foo: 'bar' } }} + ${{ jsonrpc: '2.0', id: '1', error: { code: 1, message: 'foo' } }} + `('returns true for valid JSON-RPC message $value', ({ value }) => { + expect(isJsonRpcMessage(value)).toBe(true); + }); + + it.each` + value + ${null} + ${undefined} + ${'foo'} + ${[]} + ${{}} + ${{ id: '1', method: 'foo', params: [1, 2, 3] }} + ${{ jsonrpc: '2.0', id: '1', params: { foo: 'bar' } }} + ${{ jsonrpc: '2.0', result: 'foo', params: [1, 2, 3] }} + ${{ jsonrpc: '2.0', error: 'foo', params: { foo: 'bar' } }} + `('returns false for invalid values: $value', ({ value }) => { + expect(isJsonRpcMessage(value)).toBe(false); + }); +}); diff --git a/packages/utils/src/types.ts b/packages/utils/src/types.ts index bf3d88196..36fa619a0 100644 --- a/packages/utils/src/types.ts +++ b/packages/utils/src/types.ts @@ -7,8 +7,13 @@ import { UnsafeJsonStruct, JsonRpcRequestStruct, JsonRpcResponseStruct, + JsonRpcNotificationStruct, +} from '@metamask/utils'; +import type { + JsonRpcNotification, + JsonRpcRequest, + JsonRpcResponse, } from '@metamask/utils'; -import type { JsonRpcRequest, JsonRpcResponse } from '@metamask/utils'; export type TypeGuard = (value: unknown) => value is Type; @@ -54,9 +59,23 @@ export const EmptyJsonArray = empty(array(UnsafeJsonStruct)); export type EmptyJsonArray = Infer; -export type JsonRpcMessage = JsonRpcRequest | JsonRpcResponse; +export type JsonRpcCall = JsonRpcRequest | JsonRpcNotification; + +export const JsonRpcCallStruct: Struct = union([ + JsonRpcRequestStruct, + JsonRpcNotificationStruct, +]); + +export const isJsonRpcCall = (value: unknown): value is JsonRpcCall => + is(value, JsonRpcCallStruct); + +export type JsonRpcMessage = + | JsonRpcNotification + | JsonRpcRequest + | JsonRpcResponse; export const JsonRpcMessageStruct: Struct = union([ + JsonRpcNotificationStruct, JsonRpcRequestStruct, JsonRpcResponseStruct, ]); diff --git a/packages/utils/src/wait-quiescent.test.ts b/packages/utils/src/wait-quiescent.test.ts index 7433a22ce..1378e0445 100644 --- a/packages/utils/src/wait-quiescent.test.ts +++ b/packages/utils/src/wait-quiescent.test.ts @@ -3,129 +3,134 @@ import { describe, it, expect } from 'vitest'; import { waitUntilQuiescent } from './wait-quiescent.ts'; -describe('wait-quiescent', () => { - describe('waitUntilQuiescent', () => { - it('resolves after microtask queue is empty', async () => { - const { promise: p1, resolve: r1 } = makePromiseKit(); - const { promise: p2, resolve: r2 } = makePromiseKit(); - - // Start waiting for quiescence first - const quiescentPromise = waitUntilQuiescent(); - - // Create microtasks - Promise.resolve() - .then(() => { - r1(); - return undefined; - }) - .catch(() => undefined); - - Promise.resolve() - .then(() => { - r2(); - return undefined; - }) - .catch(() => undefined); - - // Wait for all promises - await p1; - await p2; - await quiescentPromise; - - expect(true).toBe(true); // If we got here, the test passed - }); +describe('waitUntilQuiescent', () => { + it('resolves after microtask queue is empty', async () => { + const { promise: p1, resolve: r1 } = makePromiseKit(); + const { promise: p2, resolve: r2 } = makePromiseKit(); + + // Start waiting for quiescence first + const quiescentPromise = waitUntilQuiescent(); + + // Create microtasks + Promise.resolve() + .then(() => { + r1(); + return undefined; + }) + .catch(() => undefined); + + Promise.resolve() + .then(() => { + r2(); + return undefined; + }) + .catch(() => undefined); + + // Wait for all promises + await p1; + await p2; + await quiescentPromise; + + expect(true).toBe(true); // If we got here, the test passed + }); + + it('waits for a delay', async () => { + const delayP = waitUntilQuiescent(1); + // Using fake timers messes up these tests, so we use a (very long) real timeout. + await new Promise((resolve) => setTimeout(resolve, 500)); + expect(await delayP).toBeUndefined(); + }); - it('waits for nested promise chains', async () => { - const results: number[] = []; + it('waits for nested promise chains', async () => { + const results: number[] = []; - // Create nested promise chains + // Create nested promise chains + await Promise.resolve().then(async () => { + results.push(1); + // eslint-disable-next-line promise/no-nesting await Promise.resolve().then(async () => { - results.push(1); + results.push(2); // eslint-disable-next-line promise/no-nesting - await Promise.resolve().then(async () => { - results.push(2); - // eslint-disable-next-line promise/no-nesting - await Promise.resolve().then(() => { - results.push(3); - return results; - }); + await Promise.resolve().then(() => { + results.push(3); return results; }); return results; }); + return results; + }); - await waitUntilQuiescent(); + await waitUntilQuiescent(); - expect(results).toStrictEqual([1, 2, 3]); - }); + expect(results).toStrictEqual([1, 2, 3]); + }); - it('waits for concurrent promises', async () => { - const results: number[] = []; - const promises = [ - Promise.resolve().then(() => results.push(1)), - Promise.resolve().then(() => results.push(2)), - Promise.resolve().then(() => results.push(3)), - ]; - - await Promise.all(promises); - await waitUntilQuiescent(); - - expect(results).toHaveLength(3); - expect(results).toContain(1); - expect(results).toContain(2); - expect(results).toContain(3); - }); + it('waits for concurrent promises', async () => { + const results: number[] = []; + const promises = [ + Promise.resolve().then(() => results.push(1)), + Promise.resolve().then(() => results.push(2)), + Promise.resolve().then(() => results.push(3)), + ]; + + await Promise.all(promises); + await waitUntilQuiescent(); + + expect(results).toHaveLength(3); + expect(results).toContain(1); + expect(results).toContain(2); + expect(results).toContain(3); + }); - it('handles rejected promises in the queue', async () => { - const results: string[] = []; + it('handles rejected promises in the queue', async () => { + const results: string[] = []; - // Create a mix of resolved and rejected promises - await Promise.resolve() - .then(() => { - results.push('success1'); - return results; - }) - .catch(() => { - results.push('caught1'); - return results; - }); + // Create a mix of resolved and rejected promises + await Promise.resolve() + .then(() => { + results.push('success1'); + return results; + }) + .catch(() => { + results.push('caught1'); + return results; + }); - await Promise.reject(new Error('test error')) - .then(() => { - results.push('success2'); - return results; - }) - .catch(() => { - results.push('caught2'); - return results; - }); + await Promise.reject(new Error('test error')) + .then(() => { + results.push('success2'); + return results; + }) + .catch(() => { + results.push('caught2'); + return results; + }); - await waitUntilQuiescent(); + await waitUntilQuiescent(); - expect(results).toContain('success1'); - expect(results).toContain('caught2'); - }); + expect(results).toContain('success1'); + expect(results).toContain('caught2'); + }); - it('resolves even with setImmediate callbacks', async () => { - const results: string[] = []; + it('resolves even with setImmediate callbacks', async () => { + const results: string[] = []; + setImmediate(() => { + results.push('immediate1'); setImmediate(() => { - results.push('immediate1'); - setImmediate(() => { - results.push('immediate2'); - }); + results.push('immediate2'); }); + }); - await Promise.resolve().then(() => { - results.push('promise'); - return results; - }); + await Promise.resolve().then(() => { + results.push('promise'); + return results; + }); - await waitUntilQuiescent(); + await waitUntilQuiescent(); - expect(results).toContain('promise'); - // Note: We don't check for immediate1/2 as they may execute after - // waitUntilQuiescent resolves, since they're lower priority - }); + expect(results).toContain('promise'); + // Note: We don't check for immediate1/2 as they may execute after + // waitUntilQuiescent resolves, since they're lower priority }); }); diff --git a/vitest.config.ts b/vitest.config.ts index 3086271fe..425559e6a 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -82,16 +82,16 @@ export default defineConfig({ lines: 98.63, }, 'packages/extension/**': { - statements: 78.71, + statements: 78.75, functions: 80.72, - branches: 74.82, - lines: 78.7, + branches: 74.46, + lines: 78.59, }, 'packages/kernel/**': { - statements: 90.68, + statements: 90.55, functions: 92.27, - branches: 80.71, - lines: 90.65, + branches: 79.57, + lines: 90.53, }, 'packages/nodejs/**': { statements: 72.91,