diff --git a/packages/json-rpc-engine/README.md b/packages/json-rpc-engine/README.md index a3d766dc79a..1113ede22df 100644 --- a/packages/json-rpc-engine/README.md +++ b/packages/json-rpc-engine/README.md @@ -17,9 +17,21 @@ or ```ts import { JsonRpcEngineV2 } from '@metamask/json-rpc-engine/v2'; +import type { + Json, + JsonRpcMiddleware, + MiddlewareContext, +} from '@metamask/json-rpc-engine/v2'; + +type Middleware = JsonRpcMiddleware< + JsonRpcRequest, + Json, + MiddlewareContext<{ hello: string }> +>; -const engine = new JsonRpcEngineV2({ - // Create a stack of middleware and pass it to the engine: +// Engines are instantiated using the `create()` factory method as opposed to +// the constructor, which is private. +const engine = JsonRpcEngineV2.create({ middleware: [ ({ request, next, context }) => { if (request.method === 'hello') { @@ -28,7 +40,7 @@ const engine = new JsonRpcEngineV2({ } return null; }, - ({ context }) => context.get('hello'), + ({ context }) => context.assertGet('hello'), ], }); ``` @@ -81,7 +93,7 @@ import { JsonRpcEngine } from '@metamask/json-rpc-engine'; const legacyEngine = new JsonRpcEngine(); -const v2Engine = new JsonRpcEngineV2({ +const v2Engine = JsonRpcEngineV2.create({ middleware: [ // ... ], @@ -91,8 +103,8 @@ legacyEngine.push(asLegacyMiddleware(v2Engine)); ``` In keeping with the conventions of the legacy engine, non-JSON-RPC string properties of the `context` will be -copied over to the request once the V2 engine is done with the request. _Note that any symbol keys of the `context` -will **not** be copied over._ +copied over to the request once the V2 engine is done with the request. _Note that **only `string` keys** of +the `context` will be copied over._ ### Middleware @@ -104,27 +116,51 @@ They receive a `MiddlewareParams` object containing: - `context` - An append-only `Map` for passing data between middleware - `next` - - Function to call the next middleware in the stack + - Function that calls the next middleware in the stack and returns its result (if any) Here's a basic example: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ ({ next, context }) => { context.set('foo', 'bar'); - // Proceed to the next middleware + // Proceed to the next middleware and return its result return next(); }, async ({ request, context }) => { await doSomething(request, context.get('foo')); - // Return a result to end the request + // Return a result wihout calling next() to end the request return 42; }, ], }); ``` +In practice, middleware functions are often defined apart from the engine in which +they are used. Middleware defined in this manner must use the `JsonRpcMiddleware` type: + +```ts +export const permissionMiddleware: JsonRpcMiddleware< + JsonRpcRequest, + Json, // The result + MiddlewareContext<{ user: User; permissions: Permissions }> +> = async ({ request, context, next }) => { + const user = context.assertGet('user'); + const permissions = await getUserPermissions(user.id); + context.set('permissions', permissions); + return next(); +}; +``` + +Middleware can specify a return type, however `next()` always returns the widest possible +type based on the type of the `request`. See [Requests vs. notifications](#requests-vs-notifications) +for more details. + +Creating a useful `JsonRpcEngineV2` requires composing differently typed middleware together. +See [Engine composition](#engine-composition) for how to +accomplish this in the same or a set of composed engines. + ### Requests vs. notifications JSON-RPC requests come in two flavors: @@ -132,11 +168,14 @@ JSON-RPC requests come in two flavors: - [Requests](https://www.jsonrpc.org/specification#request_object), i.e. request objects _with_ an `id` - [Notifications](https://www.jsonrpc.org/specification#notification), i.e. request objects _without_ an `id` +`next()` returns `Json` for requests, `void` for notifications, and `Json | void` if the type of the request +object is not known. + For requests, one of the engine's middleware must "end" the request by returning a non-`undefined` result, or `.handle()` will throw an error: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ () => { if (Math.random() > 0.5) { @@ -158,7 +197,7 @@ try { ``` For notifications, on the other hand, one of the engine's middleware must return `undefined` to end the request, -and any non-`undefined` return values will cause an error: +and any non-`undefined` return values will cause an error to be thrown: ```ts const notification = { jsonrpc: '2.0', method: 'hello' }; @@ -174,6 +213,12 @@ try { If your middleware may be passed both requests and notifications, use the `isRequest` or `isNotification` utilities to determine what to do: +> [!NOTE] +> Middleware that handle both requests and notifications—i.e. the `JsonRpcCall` type— +> must ensure that their return values are valid for incoming requests at runtime. +> There is no compile time type error if such a middleware returns e.g. a string +> for a notification. + ```ts import { isRequest, @@ -181,7 +226,7 @@ import { JsonRpcEngineV2, } from '@metamask/json-rpc-engine/v2'; -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ async ({ request, next }) => { if (isRequest(request) && request.method === 'everything') { @@ -208,7 +253,7 @@ Middleware can modify the `method` and `params` properties by passing a new request object to `next()`: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { // Modify the request for subsequent middleware @@ -231,12 +276,12 @@ Modifying the `jsonrpc` or `id` properties is not allowed, and will cause an error: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { - // Modifying either property will cause an error return next({ ...request, + // Modifying either property will cause an error jsonrpc: '3.0', id: 'foo', }); @@ -244,6 +289,9 @@ const engine = new JsonRpcEngineV2({ () => 42, ], }); + +// Error: Middleware attempted to modify readonly property... +await engine.handle(anyRequest); ``` ### Result handling @@ -251,7 +299,7 @@ const engine = new JsonRpcEngineV2({ Middleware can observe the result by awaiting `next()`: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ async ({ request, next }) => { const startTime = Date.now(); @@ -263,8 +311,8 @@ const engine = new JsonRpcEngineV2({ `Request ${request.method} producing ${result} took ${duration}ms`, ); - // By returning undefined, the same result will be forwarded to earlier - // middleware awaiting next() + // By returning `undefined`, the result will be forwarded unmodified to earlier + // middleware. }, ({ request }) => { return 'Hello, World!'; @@ -277,7 +325,7 @@ Like the `request`, the `result` is also immutable. Middleware can update the result by returning a new one. ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ async ({ request, next }) => { const result = await next(); @@ -294,6 +342,7 @@ const engine = new JsonRpcEngineV2({ }; } + // Returning the unmodified result is equivalent to returning `undefined` return result; }, ({ request }) => { @@ -318,12 +367,12 @@ console.log(result); // } ``` -### Context sharing +### The `MiddlewareContext` Use the `context` to share data between middleware: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ async ({ context, next }) => { context.set('user', { id: '123', name: 'Alice' }); @@ -331,8 +380,7 @@ const engine = new JsonRpcEngineV2({ }, async ({ context, next }) => { // context.assertGet() throws if the value does not exist - // Use with caution: it does not otherwise perform any type checks. - const user = context.assertGet<{ id: string; name: string }>('user'); + const user = context.assertGet('user') as { id: string; name: string }; context.set('permissions', await getUserPermissions(user.id)); return next(); }, @@ -345,12 +393,13 @@ const engine = new JsonRpcEngineV2({ }); ``` -The `context` accepts symbol and string keys. To prevent accidental naming collisions, -it is append-only with deletions. -If you need to modify a context value over multiple middleware, use an array or object: +The `context` supports `PropertyKey` keys, i.e. strings, numbers, and symbols. +To prevent accidental naming collisions, existing keys must be deleted before they can be +overwritten via `set()`. +Context values are not frozen, and objects can be mutated as normal: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ async ({ context, next }) => { context.set('user', { id: '123', name: 'Alice' }); @@ -366,12 +415,37 @@ const engine = new JsonRpcEngineV2({ }); ``` +#### Constraining context keys and values + +The context exposes a generic parameter `KeyValues`, which determines the keys and values +a context instance supports: + +```ts +const context = new MiddlewareContext(); +context.set('foo', 'bar'); +context.get('foo'); // 'bar' +context.get('fizz'); // undefined +``` + +By default, `KeyValues` is `Record`. However, any object type can be +specified, effectively turning the context into a strongly typed `Map`: + +```ts +const context = new MiddlewareContext<{ foo: string }>([['foo', 'bar']]); +context.get('foo'); // 'bar' +context.get('fizz'); // Type error +``` + +The context is itself exposed as the third generic parameter of the `JsonRpcMiddleware` type. +See [Instrumenting middleware pipelines](#instrumenting-middleware-pipelines) for how to +compose different context types together. + ### Error handling Errors in middleware are propagated up the call stack: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ ({ next }) => { return next(); @@ -395,7 +469,7 @@ try { If your middleware awaits `next()`, it can handle errors using `try`/`catch`: ```ts -const engine = new JsonRpcEngineV2({ +const engine = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { try { @@ -423,12 +497,102 @@ console.log('Result:', result); // Result: 42 ``` +#### Internal errors + +The engine throws `JsonRpcEngineError` values when its invariants are violated, e.g. a middleware returns +a result value for a notification. +If you want to reliably detect these cases, use `JsonRpcEngineError.isInstance(error)`, which works across +versions of this package in the same realm. + ### Engine composition +#### Instrumenting middleware pipelines + +As discussed in the [Middleware](#middleware) section, middleware are often defined apart from the +engine in which they are used. To be used within the same engine, a set of middleware must have +compatible types. Specifically, all middleware must: + +- Handle either `JsonRpcRequest`, `JsonRpcNotification`, or both (i.e. `JsonRpcCall`) + - It is okay to mix `JsonRpcCall` middleware with either `JsonRpcRequest` or `JsonRpcNotification` + middleware, as long as the latter two are not mixed together. +- Return valid results for the overall request type +- Specify mutually inclusive context types + - The context types may be the same, partially intersecting, or completely disjoint + so long as they are not mutually exclusive. + +For example, the following middleware are compatible: + +```ts +const middleware1: JsonRpcMiddleware< + JsonRpcRequest, + Json, + MiddlewareContext<{ foo: string }> +> = /* ... */; + +const middleware2: JsonRpcMiddleware< + JsonRpcRequest, + Json, + MiddlewareContext<{ bar: string }> +> = /* ... */; + +const middleware3: JsonRpcMiddleware< + JsonRpcRequest, + { foo: string; bar: string }, + MiddlewareContext<{ foo: string; bar: string; baz: number }> +> = /* ... */; + +// ✅ OK +const engine = JsonRpcEngineV2.create({ + middleware: [middleware1, middleware2, middleware3], +}); +``` + +The following middleware are incompatible due to mismatched request types: + +> [!WARNING] +> Providing `JsonRpcRequest`- and `JsonRpcNotification`-only middleware to the same engine is +> unsound and should be avoided. However, doing so will **not** cause a type error, and it +> is the programmer's responsibility to prevent it from happening. + +```ts +const middleware1: JsonRpcMiddleware = /* ... */; + +const middleware2: JsonRpcMiddleware = /* ... */; + +// ⚠️ Attempting to call engine.handle() will NOT cause a type error, but it +// may cause errors at runtime and should be avoided. +const engine = JsonRpcEngineV2.create({ + middleware: [middleware1, middleware2], +}); +``` + +Finally, these middleware are incompatible due to mismatched context types: + +```ts +const middleware1: JsonRpcMiddleware< + JsonRpcRequest, + Json, + MiddlewareContext<{ foo: string }> +> = /* ... */; + +const middleware2: JsonRpcMiddleware< + JsonRpcRequest, + Json, + MiddlewareContext<{ foo: number }> +> = /* ... */; + +// ❌ The type of the engine is `never`; accessing any property will cause a type error +const engine = JsonRpcEngineV2.create({ + middleware: [middleware1, middleware2], +}); +``` + +#### `asMiddleware()` + Engines can be nested by converting them to middleware using `asMiddleware()`: ```ts -const subEngine = new JsonRpcEngineV2({ +const subEngine = JsonRpcEngineV2.create({ middleware: [ ({ request }) => { return 'Sub-engine result'; @@ -436,7 +600,7 @@ const subEngine = new JsonRpcEngineV2({ ], }); -const mainEngine = new JsonRpcEngineV2({ +const mainEngine = JsonRpcEngineV2.create({ middleware: [ subEngine.asMiddleware(), ({ request, next }) => { @@ -451,7 +615,7 @@ Engines used as middleware may return `undefined` for requests, but only when used as middleware: ```ts -const loggingEngine = new JsonRpcEngineV2({ +const loggingEngine = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { console.log('Observed request:', request.method); @@ -459,7 +623,7 @@ const loggingEngine = new JsonRpcEngineV2({ ], }); -const mainEngine = new JsonRpcEngineV2({ +const mainEngine = JsonRpcEngineV2.create({ middleware: [ loggingEngine.asMiddleware(), ({ request }) => { @@ -478,6 +642,40 @@ console.log('Result:', result); const result2 = await loggingEngine.handle(request); ``` +#### Calling `handle()` in a middleware + +You can also compose different engines together by calling `handle(request, context)` +on a different engine in a middleware. Keep in mind that, unlike when using `asMiddleware()`, +these "sub"-engines must return results for requests. + +This method of composition can be useful to instrument request- and notification-only +middleware pipelines: + +```ts +const requestEngine = JsonRpcEngineV2.create({ + middleware: [ + /* Request-only middleware */ + ], +}); + +const notificationEngine = JsonRpcEngineV2.create({ + middleware: [ + /* Notification-only middleware */ + ], +}); + +const orchestratorEngine = JsonRpcEngineV2.create({ + middleware: [ + ({ request, context }) => + isRequest(request) + ? requestEngine.handle(request, { context }) + : notificationEngine.handle(request as JsonRpcNotification, { + context, + }), + ], +}); +``` + ### `JsonRpcServer` The `JsonRpcServer` wraps a `JsonRpcEngineV2` to provide JSON-RPC 2.0 compliance and error handling. It coerces raw request objects into well-formed requests and handles error serialization. @@ -505,17 +703,24 @@ if ('result' in response) { // Handle error response } -// Notifications return undefined +// Notifications always return undefined const notification = { jsonrpc: '2.0', method: 'hello' }; await server.handle(notification); // Returns undefined ``` -The server accepts any object with a `method` property and validates JSON-RPC 2.0 -compliance. -Response objects are returned for requests but not notifications, and contain +The server accepts any object with a `method` property, coercing it into a request or notification +depending on the presence or absence of the `id` property, respectively. +Except for the `id`, all present JSON-RPC 2.0 fields are validated for spec conformance. +The `id` is replaced during request processing with an internal, trusted value, although the +original `id` is attached to the response before it is returned. + +Response objects are returned for requests, and contain the `result` in case of success and `error` in case of failure. -Errors thrown by the underlying engine are passed to `onError` before being serialized -and attached to the response object via the `error` property. +`undefined` is always returned for notifications. + +Errors thrown by the underlying engine are always passed to `onError` unmodified. +If the request is not a notification, the error is subsequently serialized and attached +to the response object via the `error` property. ## Contributing diff --git a/packages/json-rpc-engine/src/README.md b/packages/json-rpc-engine/src/README.md index 61fe01051f2..a83af82fbc2 100644 --- a/packages/json-rpc-engine/src/README.md +++ b/packages/json-rpc-engine/src/README.md @@ -33,7 +33,7 @@ import { asV2Middleware, JsonRpcEngine } from '@metamask/json-rpc-engine'; const legacyEngine = new JsonRpcEngine(); legacyEngine.push(/* ... */); -const v2Engine = new JsonRpcEngineV2({ +const v2Engine = JsonRpcEngineV2.create({ middleware: [asV2Middleware(legacyEngine)], }); ``` diff --git a/packages/json-rpc-engine/src/asV2Middleware.test.ts b/packages/json-rpc-engine/src/asV2Middleware.test.ts index 5d317e83d9d..81d06190e93 100644 --- a/packages/json-rpc-engine/src/asV2Middleware.test.ts +++ b/packages/json-rpc-engine/src/asV2Middleware.test.ts @@ -1,10 +1,16 @@ import { rpcErrors } from '@metamask/rpc-errors'; -import type { JsonRpcRequest } from '@metamask/utils'; +import type { Json, JsonRpcRequest } from '@metamask/utils'; import { JsonRpcEngine } from '.'; import { asV2Middleware } from './asV2Middleware'; import { JsonRpcEngineV2 } from './v2/JsonRpcEngineV2'; -import { getExtraneousKeys, makeRequest } from '../tests/utils'; +import type { JsonRpcMiddleware as V2Middleware } from './v2/JsonRpcEngineV2'; +import type { MiddlewareContext } from './v2/MiddlewareContext'; +import { + getExtraneousKeys, + makeNullMiddleware, + makeRequest, +} from '../tests/utils'; describe('asV2Middleware', () => { it('converts a legacy engine to a v2 middleware', () => { @@ -20,7 +26,7 @@ describe('asV2Middleware', () => { end(); }); - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [asV2Middleware(legacyEngine)], }); @@ -35,7 +41,7 @@ describe('asV2Middleware', () => { end(); }); - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [asV2Middleware(legacyEngine)], }); @@ -51,7 +57,7 @@ describe('asV2Middleware', () => { end(); }); - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [asV2Middleware(legacyEngine)], }); @@ -67,8 +73,8 @@ describe('asV2Middleware', () => { }); legacyEngine.push(legacyMiddleware); - const v2Engine = new JsonRpcEngineV2({ - middleware: [asV2Middleware(legacyEngine), () => null], + const v2Engine = JsonRpcEngineV2.create({ + middleware: [asV2Middleware(legacyEngine), makeNullMiddleware()], }); const result = await v2Engine.handle(makeRequest()); @@ -90,24 +96,22 @@ describe('asV2Middleware', () => { }); legacyEngine.push(legacyMiddleware); - const v2Engine = new JsonRpcEngineV2({ - middleware: [ - ({ context, next }) => { - context.set('value', 1); - return next(); - }, - asV2Middleware(legacyEngine), - ({ context }) => { - observedContextValues.push(context.assertGet('newValue')); - - expect(Array.from(context.keys())).toStrictEqual([ - 'value', - 'newValue', - ]); - - return null; - }, - ], + type Context = MiddlewareContext>; + const middleware1: V2Middleware = ({ + context, + next, + }) => { + context.set('value', 1); + return next(); + }; + const middleware2: V2Middleware = ({ + context, + }) => { + observedContextValues.push(context.assertGet('newValue')); + return null; + }; + const v2Engine = JsonRpcEngineV2.create({ + middleware: [middleware1, asV2Middleware(legacyEngine), middleware2], }); await v2Engine.handle(makeRequest()); diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts index 328909cf41a..45e30a6afb8 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.test.ts @@ -1,9 +1,10 @@ /* eslint-disable n/callback-return */ // next() is not a Node.js callback. -import type { JsonRpcId, NonEmptyArray } from '@metamask/utils'; +import type { Json, JsonRpcId } from '@metamask/utils'; import { createDeferredPromise } from '@metamask/utils'; -import type { JsonRpcMiddleware } from './JsonRpcEngineV2'; +import type { JsonRpcMiddleware, ResultConstraint } from './JsonRpcEngineV2'; import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; +import type { EmptyContext } from './MiddlewareContext'; import { MiddlewareContext } from './MiddlewareContext'; import { isRequest, @@ -13,16 +14,73 @@ import { type JsonRpcNotification, type JsonRpcRequest, } from './utils'; -import { makeNotification, makeRequest } from '../../tests/utils'; +import { + makeNotification, + makeNotificationMiddleware, + makeNullMiddleware, + makeRequest, + makeRequestMiddleware, +} from '../../tests/utils'; const jsonrpc = '2.0' as const; describe('JsonRpcEngineV2', () => { + describe('create', () => { + it('throws if the middleware array is empty', () => { + expect(() => JsonRpcEngineV2.create({ middleware: [] })).toThrow( + new JsonRpcEngineError('Middleware array cannot be empty'), + ); + }); + + it('type errors if passed middleware with incompatible context types', async () => { + const middleware1: JsonRpcMiddleware< + JsonRpcCall, + ResultConstraint, + MiddlewareContext<{ foo: string }> + > = ({ next, context }) => { + context.set('foo', 'bar'); + return next(); + }; + + const middleware2: JsonRpcMiddleware< + JsonRpcCall, + ResultConstraint, + MiddlewareContext<{ foo: number }> + > = ({ context }) => context.assertGet('foo'); + const engine = JsonRpcEngineV2.create({ + middleware: [middleware1, middleware2], + }); + + // @ts-expect-error - The engine is `InvalidEngine`. + expect(await engine.handle(makeRequest())).toBe('bar'); + }); + + // Keeping this here for documentation purposes. + // eslint-disable-next-line jest/no-disabled-tests + it.skip('type errors if passed middleware with incompatible request types', async () => { + const middleware1: JsonRpcMiddleware = ({ next }) => + next(); + const middleware2: JsonRpcMiddleware = () => { + return 'foo'; + }; + const engine = JsonRpcEngineV2.create({ + middleware: [middleware1, middleware2], + }); + + // TODO: We want this to cause a type error, but it's unclear if it can be + // made to work due to the difficulty (impossibility?) of distinguishing + // between these two cases: + // - JsonRpcMiddleware | JsonRpcMiddleware (invalid) + // - JsonRpcMiddleware | JsonRpcMiddleware (valid) + expect(await engine.handle(makeRequest() as JsonRpcRequest)).toBe('foo'); + }); + }); + describe('handle', () => { describe('notifications', () => { it('passes the notification through a middleware', async () => { const middleware: JsonRpcMiddleware = jest.fn(); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); const notification = { jsonrpc, method: 'test_request' }; @@ -38,7 +96,7 @@ describe('JsonRpcEngineV2', () => { }); it('returns no result', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [jest.fn()], }); const notification = { jsonrpc, method: 'test_request' }; @@ -51,7 +109,7 @@ describe('JsonRpcEngineV2', () => { it('returns no result, with multiple middleware', async () => { const middleware1 = jest.fn(({ next }) => next()); const middleware2 = jest.fn(); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2], }); const notification = { jsonrpc, method: 'test_request' }; @@ -64,7 +122,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware throws', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(() => { throw new Error('test'); @@ -79,7 +137,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware throws, with multiple middleware', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(({ next }) => next()), jest.fn(() => { @@ -95,8 +153,8 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a result is returned, from the first middleware', async () => { - const engine = new JsonRpcEngineV2({ - middleware: [jest.fn(() => 'foo')], + const engine = JsonRpcEngineV2.create({ + middleware: [() => 'foo'], }); const notification = { jsonrpc, method: 'test_request' }; @@ -108,13 +166,13 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a result is returned, from a later middleware', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ - jest.fn(async ({ next }) => { + async ({ next }) => { await next(); return undefined; - }), - jest.fn(() => null), + }, + makeNullMiddleware(), ], }); const notification = { jsonrpc, method: 'test_request' }; @@ -127,7 +185,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware calls next() multiple times', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ next }) => { await next(); @@ -148,10 +206,8 @@ describe('JsonRpcEngineV2', () => { describe('requests', () => { it('returns a result from the middleware', async () => { - const middleware: JsonRpcMiddleware = jest.fn( - () => null, - ); - const engine = new JsonRpcEngineV2({ + const middleware = jest.fn(() => null); + const engine = JsonRpcEngineV2.create({ middleware: [middleware], }); const request = makeRequest(); @@ -168,9 +224,9 @@ describe('JsonRpcEngineV2', () => { }); it('returns a result from the middleware, with multiple middleware', async () => { - const middleware1 = jest.fn(({ next }) => next()); - const middleware2 = jest.fn(() => null); - const engine = new JsonRpcEngineV2({ + const middleware1: JsonRpcMiddleware = jest.fn(({ next }) => next()); + const middleware2: JsonRpcMiddleware = jest.fn(() => null); + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2], }); const request = makeRequest(); @@ -193,7 +249,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware throws', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(() => { throw new Error('test'); @@ -207,7 +263,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware throws, with multiple middleware', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(({ next }) => next()), jest.fn(() => { @@ -222,7 +278,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if no middleware returns a result', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [jest.fn(({ next }) => next()), jest.fn()], }); const request = makeRequest(); @@ -235,13 +291,13 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware calls next() multiple times', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ next }) => { await next(); await next(); }), - jest.fn(), + makeNullMiddleware(), ], }); const request = makeRequest(); @@ -256,26 +312,38 @@ describe('JsonRpcEngineV2', () => { describe('context', () => { it('passes the context to the middleware', async () => { - const middleware = jest.fn(({ context }) => { - expect(context).toBeInstanceOf(Map); - return null; - }); - const engine = new JsonRpcEngineV2({ - middleware: [middleware], + const engine = JsonRpcEngineV2.create< + JsonRpcMiddleware + >({ + middleware: [ + ({ context }) => { + expect(context).toBeInstanceOf(Map); + return null; + }, + ], }); await engine.handle(makeRequest()); }); it('propagates context changes to subsequent middleware', async () => { - const middleware1 = jest.fn(async ({ context, next }) => { + type Context = MiddlewareContext<{ foo: string }>; + const middleware1: JsonRpcMiddleware< + JsonRpcCall, + Json | void, + Context + > = async ({ context, next }) => { context.set('foo', 'bar'); return next(); - }); - const middleware2 = jest.fn(({ context }) => { + }; + const middleware2: JsonRpcMiddleware< + JsonRpcCall, + string | undefined, + Context + > = ({ context }) => { return context.get('foo') as string | undefined; - }); - const engine = new JsonRpcEngineV2({ + }; + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2], }); @@ -285,10 +353,15 @@ describe('JsonRpcEngineV2', () => { }); it('accepts an initial context', async () => { - const initialContext = new MiddlewareContext(); + const initialContext = new MiddlewareContext>(); initialContext.set('foo', 'bar'); - const engine = new JsonRpcEngineV2({ - middleware: [({ context }) => context.assertGet('foo')], + const middleware: JsonRpcMiddleware< + JsonRpcRequest, + string, + MiddlewareContext> + > = ({ context }) => context.assertGet('foo'); + const engine = JsonRpcEngineV2.create({ + middleware: [middleware], }); const result = await engine.handle(makeRequest(), { @@ -298,12 +371,49 @@ describe('JsonRpcEngineV2', () => { expect(result).toBe('bar'); }); + it('accepts middleware with different context types', async () => { + const middleware1: JsonRpcMiddleware< + JsonRpcCall, + ResultConstraint, + MiddlewareContext<{ foo: string }> + > = ({ context, next }) => { + context.set('foo', 'bar'); + return next(); + }; + + const middleware2: JsonRpcMiddleware< + JsonRpcCall, + ResultConstraint + > = ({ next }) => next(); + + const middleware3: JsonRpcMiddleware< + JsonRpcCall, + ResultConstraint, + EmptyContext + > = ({ next }) => next(); + + const middleware4: JsonRpcMiddleware< + JsonRpcCall, + string, + MiddlewareContext<{ foo: string; bar: number }> + > = ({ context }) => context.assertGet('foo'); + + const engine = JsonRpcEngineV2.create({ + middleware: [middleware1, middleware2, middleware3, middleware4], + }); + + const result = await engine.handle(makeRequest()); + + expect(result).toBe('bar'); + }); + it('throws if a middleware attempts to modify properties of the context', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ - jest.fn(({ context }) => { + ({ context }) => { + // @ts-expect-error - Destructive testing. context.set = () => undefined; - }), + }, ], }); @@ -315,9 +425,8 @@ describe('JsonRpcEngineV2', () => { describe('asynchrony', () => { it('handles asynchronous middleware', async () => { - const middleware = jest.fn(async () => null); - const engine = new JsonRpcEngineV2({ - middleware: [middleware], + const engine = JsonRpcEngineV2.create({ + middleware: [async () => null], }); const result = await engine.handle(makeRequest()); @@ -326,26 +435,28 @@ describe('JsonRpcEngineV2', () => { }); it('handles mixed synchronous and asynchronous middleware', async () => { - const middleware1: JsonRpcMiddleware> = - jest.fn(async ({ context, next }) => { - context.set('foo', [1]); - return next(); - }); - const middleware2: JsonRpcMiddleware> = - jest.fn(({ context, next }) => { - const nums = context.assertGet('foo'); - nums.push(2); - return next(); - }); - const middleware3: JsonRpcMiddleware< + type Middleware = JsonRpcMiddleware< JsonRpcRequest, - number[] - > = jest.fn(async ({ context }) => { - const nums = context.assertGet('foo'); - return [...nums, 3]; - }); - const engine = new JsonRpcEngineV2({ - middleware: [middleware1, middleware2, middleware3], + Json, + MiddlewareContext> + >; + + const engine = JsonRpcEngineV2.create({ + middleware: [ + async ({ context, next }) => { + context.set('foo', [1]); + return next(); + }, + ({ context, next }) => { + const nums = context.assertGet('foo'); + nums.push(2); + return next(); + }, + async ({ context }) => { + const nums = context.assertGet('foo'); + return [...nums, 3]; + }, + ], }); const result = await engine.handle(makeRequest()); @@ -378,7 +489,7 @@ describe('JsonRpcEngineV2', () => { observedMethod = request.method; return null; }); - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware1, middleware2, middleware3], }); const request = makeRequest({ params: [1] }); @@ -393,10 +504,10 @@ describe('JsonRpcEngineV2', () => { }); it('throws if directly modifying the request', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(({ request }) => { - // @ts-expect-error Destructive testing. + // @ts-expect-error - Destructive testing. request.params = [2]; }) as JsonRpcMiddleware, ], @@ -410,7 +521,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware attempts to modify the request "id" property', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ request, next }) => { return await next({ @@ -418,7 +529,7 @@ describe('JsonRpcEngineV2', () => { id: '2', }); }), - jest.fn(() => null), + makeNullMiddleware(), ], }); const request = makeRequest(); @@ -431,15 +542,16 @@ describe('JsonRpcEngineV2', () => { }); it('throws if a middleware attempts to modify the request "jsonrpc" property', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ - jest.fn(async ({ request, next }) => { + async ({ request, next }) => { return await next({ ...request, + // @ts-expect-error - Destructive testing. jsonrpc: '3.0', }); - }), - jest.fn(() => null), + }, + makeNullMiddleware(), ], }); const request = makeRequest(); @@ -454,13 +566,15 @@ describe('JsonRpcEngineV2', () => { describe('result handling', () => { it('updates the result after next() is called', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create< + JsonRpcMiddleware + >({ middleware: [ - jest.fn(async ({ next }) => { - const result = await next(); + async ({ next }) => { + const result = (await next()) as number; return result + 1; - }), - jest.fn(() => 1), + }, + () => 1, ], }); @@ -470,13 +584,13 @@ describe('JsonRpcEngineV2', () => { }); it('updates an undefined result with a new value', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ - jest.fn(async ({ next }) => { + async ({ next }) => { await next(); return null; - }), - jest.fn(() => undefined), + }, + makeNotificationMiddleware(), ], }); @@ -486,12 +600,12 @@ describe('JsonRpcEngineV2', () => { }); it('returning undefined propagates previously defined result', async () => { - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ next }) => { await next(); }), - jest.fn(() => null), + makeNullMiddleware(), ], }); @@ -502,7 +616,7 @@ describe('JsonRpcEngineV2', () => { it('catches errors thrown by later middleware', async () => { let observedError: Error | undefined; - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ jest.fn(async ({ next }) => { try { @@ -526,21 +640,22 @@ describe('JsonRpcEngineV2', () => { it('handles returned results in reverse middleware order', async () => { const returnHandlerResults: number[] = []; - const middleware1 = jest.fn(async ({ next }) => { - await next(); - returnHandlerResults.push(1); - }); - const middleware2 = jest.fn(async ({ next }) => { - await next(); - returnHandlerResults.push(2); - }); - const middleware3 = jest.fn(async ({ next }) => { - await next(); - returnHandlerResults.push(3); - }); - const middleware4 = jest.fn(() => null); - const engine = new JsonRpcEngineV2({ - middleware: [middleware1, middleware2, middleware3, middleware4], + const engine = JsonRpcEngineV2.create({ + middleware: [ + async ({ next }) => { + await next(); + returnHandlerResults.push(1); + }, + async ({ next }) => { + await next(); + returnHandlerResults.push(2); + }, + async ({ next }) => { + await next(); + returnHandlerResults.push(3); + }, + makeNullMiddleware(), + ], }); await engine.handle(makeRequest()); @@ -549,14 +664,15 @@ describe('JsonRpcEngineV2', () => { }); it('throws if directly modifying the result', async () => { - const middleware1 = jest.fn(async ({ next }) => { - const result = await next(); - result.foo = 'baz'; - return result; - }); - const middleware2 = jest.fn(() => ({ foo: 'bar' })); - const engine = new JsonRpcEngineV2({ - middleware: [middleware1, middleware2], + const engine = JsonRpcEngineV2.create({ + middleware: [ + async ({ next }) => { + const result = (await next()) as { foo: string }; + result.foo = 'baz'; + return result; + }, + () => ({ foo: 'bar' }), + ], }); await expect(engine.handle(makeRequest())).rejects.toThrow( @@ -625,25 +741,33 @@ describe('JsonRpcEngineV2', () => { let inFlight = 0; let maxInFlight = 0; - const engine = new JsonRpcEngineV2({ - middleware: [ - async ({ context, next, request }) => { - // eslint-disable-next-line jest/no-conditional-in-test - context.set('id', context.get('id') ?? request.id); + type Context = MiddlewareContext<{ id: JsonRpcId }>; + const inflightMiddleware: JsonRpcMiddleware< + JsonRpcRequest, + Json, + Context + > = async ({ context, next, request }) => { + // eslint-disable-next-line jest/no-conditional-in-test + context.set('id', context.get('id') ?? request.id); - inFlight += 1; - maxInFlight = Math.max(maxInFlight, inFlight); - latch.increment(); + inFlight += 1; + maxInFlight = Math.max(maxInFlight, inFlight); + latch.increment(); - await gate; + await gate; - inFlight -= 1; - return next(); - }, - ({ context, request }) => { - return `result:${request.id}:${context.get('id') as JsonRpcId}`; - }, - ], + inFlight -= 1; + return next(); + }; + const resultMiddleware: JsonRpcMiddleware< + JsonRpcRequest, + string, + Context + > = ({ context, request }) => { + return `result:${request.id}:${context.assertGet('id')}`; + }; + const engine = JsonRpcEngineV2.create({ + middleware: [inflightMiddleware, resultMiddleware], }); // eslint-disable-next-line @typescript-eslint/ban-ts-comment @@ -672,13 +796,14 @@ describe('JsonRpcEngineV2', () => { it('eagerly processes requests in parallel, i.e. without queueing them', async () => { const queue = makeArbitraryQueue(3); - const engine = new JsonRpcEngineV2({ - middleware: [ - async ({ request }) => { - await queue.enqueue(request.id); - return null; - }, - ], + const middleware: JsonRpcMiddleware< + JsonRpcRequest & { id: number } + > = async ({ request }) => { + await queue.enqueue(request.id); + return null; + }; + const engine = JsonRpcEngineV2.create({ + middleware: [middleware], }); const p0 = engine.handle(makeRequest({ id: 0 })); @@ -700,11 +825,10 @@ describe('JsonRpcEngineV2', () => { describe('composition', () => { describe('asMiddleware', () => { it('ends a request if it returns a value', async () => { - // TODO: We may have to do a lot of these casts? - const engine1 = new JsonRpcEngineV2({ - middleware: [() => null], + const engine1 = JsonRpcEngineV2.create({ + middleware: [makeNullMiddleware()], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [engine1.asMiddleware(), jest.fn(() => 'foo')], }); @@ -714,11 +838,11 @@ describe('JsonRpcEngineV2', () => { }); it('permits returning undefined if a later middleware ends the request', async () => { - const engine1 = new JsonRpcEngineV2({ - middleware: [() => undefined], + const engine1 = JsonRpcEngineV2.create({ + middleware: [makeNotificationMiddleware()], }); - const engine2 = new JsonRpcEngineV2({ - middleware: [engine1.asMiddleware(), () => null], + const engine2 = JsonRpcEngineV2.create({ + middleware: [engine1.asMiddleware(), makeNullMiddleware()], }); const result = await engine2.handle(makeRequest()); @@ -729,13 +853,13 @@ describe('JsonRpcEngineV2', () => { it('composes nested engines', async () => { const middleware1 = jest.fn(async ({ next }) => next()); const middleware2 = jest.fn(async ({ next }) => next()); - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [middleware1], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [engine1.asMiddleware(), middleware2], }); - const engine3 = new JsonRpcEngineV2({ + const engine3 = JsonRpcEngineV2.create({ middleware: [engine2.asMiddleware(), () => null], }); @@ -747,7 +871,7 @@ describe('JsonRpcEngineV2', () => { }); it('propagates request mutation', async () => { - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { return next({ @@ -759,21 +883,19 @@ describe('JsonRpcEngineV2', () => { return next({ ...request, method: 'test_request_2', - // @ts-expect-error Will obviously work. - params: [request.params[0] * 2], + params: [(request.params as [number])[0] * 2], }); }, ], }); let observedMethod: string | undefined; - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ engine1.asMiddleware(), ({ request }) => { observedMethod = request.method; - // @ts-expect-error Will obviously work. - return request.params[0] * 2; + return (request.params as [number])[0] * 2; }, ], }); @@ -785,18 +907,17 @@ describe('JsonRpcEngineV2', () => { }); it('propagates context changes', async () => { - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ async ({ context, next }) => { - const nums = context.assertGet('foo'); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - nums[0]! *= 2; + const nums = context.assertGet('foo') as [number]; + nums[0] *= 2; return next(); }, ], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ async ({ context, next }) => { context.set('foo', [2]); @@ -804,8 +925,8 @@ describe('JsonRpcEngineV2', () => { }, engine1.asMiddleware(), async ({ context }) => { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return context.assertGet('foo')[0]! * 2; + const nums = context.assertGet('foo') as [number]; + return nums[0] * 2; }, ], }); @@ -817,7 +938,7 @@ describe('JsonRpcEngineV2', () => { it('observes results in expected order', async () => { const returnHandlerResults: string[] = []; - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ async ({ next }) => { await next(); @@ -830,7 +951,7 @@ describe('JsonRpcEngineV2', () => { ], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ engine1.asMiddleware(), async ({ next }) => { @@ -862,17 +983,23 @@ describe('JsonRpcEngineV2', () => { it('composes nested engines', async () => { const earlierMiddleware = jest.fn(async ({ next }) => next()); - const engine1 = new JsonRpcEngineV2({ - middleware: [() => null], + const engine1Middleware: JsonRpcMiddleware = () => null; + const engine1 = JsonRpcEngineV2.create({ + middleware: [engine1Middleware], }); - const laterMiddleware = jest.fn(() => 'foo'); - const engine2 = new JsonRpcEngineV2({ + const engine1ProxyMiddleware: JsonRpcMiddleware< + JsonRpcRequest + > = async ({ request }) => { + return engine1.handle(request); + }; + const laterMiddleware: JsonRpcMiddleware = jest.fn( + () => 'foo', + ); + const engine2 = JsonRpcEngineV2.create({ middleware: [ earlierMiddleware, - async ({ request }) => { - return engine1.handle(request as JsonRpcRequest); - }, + engine1ProxyMiddleware, laterMiddleware, ], }); @@ -887,7 +1014,7 @@ describe('JsonRpcEngineV2', () => { it('does not propagate request mutation', async () => { // Unlike asMiddleware(), although the inner engine mutates request, // those mutations do not propagate when using engine.handle(). - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ ({ request, next }) => { return next({ @@ -899,26 +1026,28 @@ describe('JsonRpcEngineV2', () => { return next({ ...request, method: 'test_request_2', - // @ts-expect-error Will obviously work at runtime - params: [request.params[0] * 2], + params: [(request.params as [number])[0] * 2], }); }, - () => null, + makeNullMiddleware(), ], }); let observedMethod: string | undefined; - const engine2 = new JsonRpcEngineV2({ + const observedMethodMiddleware: JsonRpcMiddleware< + JsonRpcRequest, + number + > = ({ request }) => { + observedMethod = request.method; + return (request.params as [number])[0] * 2; + }; + const engine2 = JsonRpcEngineV2.create({ middleware: [ async ({ request, next, context }) => { - await engine1.handle(request as JsonRpcRequest, { context }); + await engine1.handle(request, { context }); return next(); }, - ({ request }) => { - observedMethod = request.method; - // @ts-expect-error Will obviously work at runtime - return request.params[0] * 2; - }, + observedMethodMiddleware, ], }); @@ -931,30 +1060,29 @@ describe('JsonRpcEngineV2', () => { }); it('propagates context changes', async () => { - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ async ({ context }) => { - const nums = context.assertGet('foo'); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - nums[0]! *= 2; + const nums = context.assertGet('foo') as [number]; + nums[0] *= 2; return null; }, ], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ async ({ context, next }) => { context.set('foo', [2]); return next(); }, async ({ request, next, context }) => { - await engine1.handle(request as JsonRpcRequest, { context }); + await engine1.handle(request, { context }); return next(); }, async ({ context }) => { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return context.assertGet('foo')[0]! * 2; + const nums = context.assertGet('foo') as [number]; + return nums[0] * 2; }, ], }); @@ -966,7 +1094,7 @@ describe('JsonRpcEngineV2', () => { it('observes results in expected order', async () => { const returnHandlerResults: string[] = []; - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ async ({ next }) => { await next(); @@ -976,11 +1104,11 @@ describe('JsonRpcEngineV2', () => { await next(); returnHandlerResults.push('1:b'); }, - () => null, + makeNullMiddleware(), ], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ async ({ request, next, context }) => { await engine1.handle(request as JsonRpcRequest, { context }); @@ -994,7 +1122,7 @@ describe('JsonRpcEngineV2', () => { await next(); returnHandlerResults.push('2:b'); }, - () => null, + makeNullMiddleware(), ], }); @@ -1011,7 +1139,7 @@ describe('JsonRpcEngineV2', () => { }); it('throws if the inner engine throws', async () => { - const engine1 = new JsonRpcEngineV2({ + const engine1 = JsonRpcEngineV2.create({ middleware: [ () => { throw new Error('test'); @@ -1019,7 +1147,7 @@ describe('JsonRpcEngineV2', () => { ], }); - const engine2 = new JsonRpcEngineV2({ + const engine2 = JsonRpcEngineV2.create({ middleware: [ async ({ request }) => { await engine1.handle(request as JsonRpcRequest); @@ -1036,14 +1164,14 @@ describe('JsonRpcEngineV2', () => { describe('request- and notification-only engines', () => { it('constructs a request-only engine', async () => { - const engine = new JsonRpcEngineV2({ - middleware: [() => null], + const engine = JsonRpcEngineV2.create({ + middleware: [makeRequestMiddleware()], }); expect(await engine.handle(makeRequest())).toBeNull(); - // @ts-expect-error Valid at runtime, but should cause a type error + // @ts-expect-error - Valid at runtime, but should cause a type error expect(await engine.handle(makeRequest() as JsonRpcCall)).toBeNull(); - // @ts-expect-error Invalid at runtime and should cause a type error + // @ts-expect-error - Invalid at runtime and should cause a type error await expect(engine.handle(makeNotification())).rejects.toThrow( new JsonRpcEngineError( `Result returned for notification: ${stringify(makeNotification())}`, @@ -1052,13 +1180,13 @@ describe('JsonRpcEngineV2', () => { }); it('constructs a notification-only engine', async () => { - const engine = new JsonRpcEngineV2({ - middleware: [() => undefined], + const engine = JsonRpcEngineV2.create({ + middleware: [makeNotificationMiddleware()], }); expect(await engine.handle(makeNotification())).toBeUndefined(); await expect( - // @ts-expect-error Invalid at runtime and should cause a type error + // @ts-expect-error - Invalid at runtime and should cause a type error engine.handle({ id: '1', jsonrpc, method: 'test_request' }), ).rejects.toThrow( new JsonRpcEngineError( @@ -1066,7 +1194,7 @@ describe('JsonRpcEngineV2', () => { ), ); await expect( - // @ts-expect-error Invalid at runtime and should cause a type error + // @ts-expect-error - Invalid at runtime and should cause a type error engine.handle(makeRequest() as JsonRpcRequest), ).rejects.toThrow( new JsonRpcEngineError( @@ -1076,11 +1204,12 @@ describe('JsonRpcEngineV2', () => { }); it('constructs a mixed engine', async () => { - const engine = new JsonRpcEngineV2({ - middleware: [ - // eslint-disable-next-line jest/no-conditional-in-test - ({ request }) => (isRequest(request) ? null : undefined), - ], + const mixedMiddleware: JsonRpcMiddleware = ({ request }) => { + // eslint-disable-next-line jest/no-conditional-in-test + return isRequest(request) ? null : undefined; + }; + const engine = JsonRpcEngineV2.create({ + middleware: [mixedMiddleware], }); expect(await engine.handle(makeRequest())).toBeNull(); @@ -1089,15 +1218,15 @@ describe('JsonRpcEngineV2', () => { }); it('composes a pipeline of request- and notification-only engines', async () => { - const requestEngine = new JsonRpcEngineV2({ - middleware: [() => null], + const requestEngine = JsonRpcEngineV2.create({ + middleware: [makeRequestMiddleware()], }); - const notificationEngine = new JsonRpcEngineV2({ - middleware: [() => undefined], + const notificationEngine = JsonRpcEngineV2.create({ + middleware: [makeNotificationMiddleware()], }); - const orchestratorEngine = new JsonRpcEngineV2({ + const orchestratorEngine = JsonRpcEngineV2.create({ middleware: [ ({ request, context }) => // eslint-disable-next-line jest/no-conditional-in-test @@ -1123,7 +1252,7 @@ describe('JsonRpcEngineV2', () => { const middleware = { destroy: jest.fn(), }; - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware as unknown as JsonRpcMiddleware], }); @@ -1137,7 +1266,7 @@ describe('JsonRpcEngineV2', () => { destroy: jest.fn(), }; - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware as unknown as JsonRpcMiddleware], }); @@ -1148,8 +1277,8 @@ describe('JsonRpcEngineV2', () => { }); it('causes handle() to throw after destroying the engine', async () => { - const engine = new JsonRpcEngineV2({ - middleware: [() => null], + const engine = JsonRpcEngineV2.create({ + middleware: [makeNullMiddleware()], }); await engine.destroy(); @@ -1160,8 +1289,8 @@ describe('JsonRpcEngineV2', () => { }); it('causes asMiddleware() to throw after destroying the engine', async () => { - const engine = new JsonRpcEngineV2({ - middleware: [() => null], + const engine = JsonRpcEngineV2.create({ + middleware: [makeNullMiddleware()], }); await engine.destroy(); @@ -1176,7 +1305,7 @@ describe('JsonRpcEngineV2', () => { throw new Error('test'); }), }; - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [middleware as unknown as JsonRpcMiddleware], }); @@ -1192,11 +1321,11 @@ describe('JsonRpcEngineV2', () => { const middleware2 = { destroy: jest.fn(), }; - const engine = new JsonRpcEngineV2({ + const engine = JsonRpcEngineV2.create({ middleware: [ middleware1, middleware2, - ] as unknown as NonEmptyArray, + ] as unknown as JsonRpcMiddleware[], }); await expect(engine.destroy()).rejects.toThrow(new Error('test')); diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index dd94aaf21ad..4f55df4a382 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -7,6 +7,7 @@ import { } from '@metamask/utils'; import deepFreeze from 'deep-freeze-strict'; +import type { ContextConstraint, MergeContexts } from './MiddlewareContext'; import { MiddlewareContext } from './MiddlewareContext'; import { isNotification, @@ -37,17 +38,21 @@ export type Next = ( request?: Readonly, ) => Promise> | undefined>; -export type MiddlewareParams = { +export type MiddlewareParams< + Request extends JsonRpcCall, + Context extends MiddlewareContext, +> = { request: Readonly; - context: MiddlewareContext; + context: Context; next: Next; }; export type JsonRpcMiddleware< Request extends JsonRpcCall = JsonRpcCall, Result extends ResultConstraint = ResultConstraint, + Context extends ContextConstraint = MiddlewareContext, > = ( - params: MiddlewareParams, + params: MiddlewareParams, ) => Readonly | undefined | Promise | undefined>; type RequestState = { @@ -55,14 +60,52 @@ type RequestState = { result: Readonly> | undefined; }; -type Options = { - middleware: NonEmptyArray>; +type HandleOptions = { + context?: Context; }; -type HandleOptions = { - context?: MiddlewareContext; +type ConstructorOptions< + Request extends JsonRpcCall, + Context extends MiddlewareContext, +> = { + middleware: NonEmptyArray< + JsonRpcMiddleware, Context> + >; }; +type RequestOf = + Middleware extends JsonRpcMiddleware< + infer Request, + ResultConstraint, + // Non-polluting `any` constraint. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + any + > + ? Request + : never; + +type ContextOf = + // Non-polluting `any` constraint. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Middleware extends JsonRpcMiddleware, infer C> + ? C + : never; + +type MergedContextOf< + // Non-polluting `any` constraint. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Middleware extends JsonRpcMiddleware, +> = MergeContexts>; + +const INVALID_ENGINE = Symbol('Invalid engine'); + +/** + * An internal type for invalid engines that explains why the engine is invalid. + * + * @template Message - The message explaining why the engine is invalid. + */ +type InvalidEngine = { [INVALID_ENGINE]: Message }; + /** * A JSON-RPC request and response processor. * @@ -86,7 +129,7 @@ type HandleOptions = { * * @example * ```ts - * const engine = new JsonRpcEngineV2({ + * const engine = JsonRpcEngineV2.create({ * middleware, * }); * @@ -98,15 +141,65 @@ type HandleOptions = { * } * ``` */ -export class JsonRpcEngineV2 { - #middleware: Readonly>>; +export class JsonRpcEngineV2< + Request extends JsonRpcCall = JsonRpcCall, + Context extends ContextConstraint = MiddlewareContext, +> { + #middleware: Readonly< + NonEmptyArray< + JsonRpcMiddleware, Context> + > + >; #isDestroyed = false; - constructor({ middleware }: Options) { + // See .create() for why this is private. + private constructor({ middleware }: ConstructorOptions) { this.#middleware = [...middleware]; } + // We use a static factory method in order to construct a supertype of all middleware contexts, + // which enables us to instantiate an engine despite different middleware expecting different + // context types. + /** + * Create a new JSON-RPC engine. + * + * @throws If the middleware array is empty. + * @param options - The options for the engine. + * @param options.middleware - The middleware to use. + * @returns The JSON-RPC engine. + */ + static create< + Middleware extends JsonRpcMiddleware< + // Non-polluting `any` constraint. + /* eslint-disable @typescript-eslint/no-explicit-any */ + any, + ResultConstraint, + any + /* eslint-enable @typescript-eslint/no-explicit-any */ + > = JsonRpcMiddleware, + >({ middleware }: { middleware: Middleware[] }) { + // We can't use NonEmptyArray for the params because it ruins type inference. + if (middleware.length === 0) { + throw new JsonRpcEngineError('Middleware array cannot be empty'); + } + + type MergedContext = MergedContextOf; + type InputRequest = RequestOf; + const mw = middleware as unknown as NonEmptyArray< + JsonRpcMiddleware< + InputRequest, + ResultConstraint, + MergedContext + > + >; + return new JsonRpcEngineV2({ + middleware: mw, + }) as MergedContext extends never + ? InvalidEngine<'Some middleware have incompatible context types'> + : JsonRpcEngineV2; + } + /** * Handle a JSON-RPC request. * @@ -119,7 +212,7 @@ export class JsonRpcEngineV2 { request: Extract extends never ? never : Extract, - options?: HandleOptions, + options?: HandleOptions, ): Promise< Extract extends never ? never @@ -137,7 +230,7 @@ export class JsonRpcEngineV2 { notification: Extract extends never ? never : WithoutId>, - options?: HandleOptions, + options?: HandleOptions, ): Promise< Extract extends never ? never @@ -155,12 +248,12 @@ export class JsonRpcEngineV2 { */ async handle( call: MixedParam, - options?: HandleOptions, + options?: HandleOptions, ): Promise | void>; async handle( request: Request, - { context }: HandleOptions = {}, + { context }: HandleOptions = {}, ): Promise> | void> { const isReq = isRequest(request); const { result } = await this.#handle(request, context); @@ -183,7 +276,7 @@ export class JsonRpcEngineV2 { */ async #handle( originalRequest: Request, - context: MiddlewareContext = new MiddlewareContext(), + context: Context = new MiddlewareContext() as Context, ): Promise> { this.#assertIsNotDestroyed(); @@ -220,9 +313,11 @@ export class JsonRpcEngineV2 { * @returns The `next()` function factory. */ #makeNextFactory( - middlewareIterator: Iterator>, + middlewareIterator: Iterator< + JsonRpcMiddleware, Context> + >, state: RequestState, - context: MiddlewareContext, + context: Context, ): () => Next { const makeNext = (): Next => { let wasCalled = false; @@ -264,7 +359,9 @@ export class JsonRpcEngineV2 { return makeNext; } - #makeMiddlewareIterator(): Iterator> { + #makeMiddlewareIterator(): Iterator< + JsonRpcMiddleware, Context> + > { return this.#middleware[Symbol.iterator](); } @@ -325,7 +422,11 @@ export class JsonRpcEngineV2 { * * @returns The JSON-RPC middleware. */ - asMiddleware(): JsonRpcMiddleware { + asMiddleware(): JsonRpcMiddleware< + Request, + ResultConstraint, + Context + > { this.#assertIsNotDestroyed(); return async ({ request, context, next }) => { diff --git a/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts b/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts index 5b6ebac47b4..88d90d2e7d3 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcServer.test.ts @@ -1,5 +1,6 @@ import { rpcErrors } from '@metamask/rpc-errors'; +import type { JsonRpcMiddleware } from './JsonRpcEngineV2'; import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; import { JsonRpcServer } from './JsonRpcServer'; import { isRequest } from './utils'; @@ -7,7 +8,7 @@ import { isRequest } from './utils'; const jsonrpc = '2.0' as const; const makeEngine = () => { - return new JsonRpcEngineV2({ + return JsonRpcEngineV2.create({ middleware: [ ({ request }) => { if (request.method !== 'hello') { diff --git a/packages/json-rpc-engine/src/v2/JsonRpcServer.ts b/packages/json-rpc-engine/src/v2/JsonRpcServer.ts index 66a8475d4ac..bd10a67bf4a 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcServer.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcServer.ts @@ -74,7 +74,7 @@ export class JsonRpcServer { // @ts-expect-error - hasProperty fails to narrow the type. this.#engine = options.engine; } else { - this.#engine = new JsonRpcEngineV2({ middleware: options.middleware }); + this.#engine = JsonRpcEngineV2.create({ middleware: options.middleware }); } } @@ -201,7 +201,8 @@ function isMinimalRequest(rawRequest: unknown): rawRequest is MinimalRequest { } /** - * Check if a request has valid params. + * Check if a request has valid params, i.e. an array or object. + * The contents of the params are not inspected. * * @param rawRequest - The request to check. * @returns `true` if the request has valid params, `false` otherwise. diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.test.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.test.ts index afec7d8112c..8d755ddb485 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.test.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.test.ts @@ -1,14 +1,9 @@ import { MiddlewareContext } from './MiddlewareContext'; describe('MiddlewareContext', () => { - it('is a map', () => { - const context = new MiddlewareContext(); - expect(context).toBeInstanceOf(Map); - }); - it('can be constructed with entries', () => { const symbol = Symbol('test'); - const context = new MiddlewareContext([ + const context = new MiddlewareContext<{ test: string; [symbol]: string }>([ ['test', 'value'], [symbol, 'value'], ]); @@ -21,28 +16,48 @@ describe('MiddlewareContext', () => { expect(Object.isFrozen(context)).toBe(true); }); + it('type errors and returns undefined when getting unknown keys', () => { + const context = new MiddlewareContext<{ test: string }>(); + // @ts-expect-error - foo is not a valid key + expect(context.get('foo')).toBeUndefined(); + }); + + it('type errors and throws when assertGet:ing unknown keys', () => { + const context = new MiddlewareContext<{ test: string }>(); + // @ts-expect-error - foo is not a valid key + expect(() => context.assertGet('foo')).toThrow( + `Context key "foo" not found`, + ); + }); + + it('type errors when setting unknown keys', () => { + const context = new MiddlewareContext<{ test: string }>(); + // @ts-expect-error - foo is not a valid key + expect(context.set('foo', 'value')).toBe(context); + }); + it('assertGet throws if the key is not found', () => { - const context = new MiddlewareContext(); + const context = new MiddlewareContext<{ test: string }>(); expect(() => context.assertGet('test')).toThrow( `Context key "test" not found`, ); }); it('assertGet returns the value if the key is found (string)', () => { - const context = new MiddlewareContext(); + const context = new MiddlewareContext<{ test: string }>(); context.set('test', 'value'); expect(context.assertGet('test')).toBe('value'); }); it('assertGet returns the value if the key is found (symbol)', () => { - const context = new MiddlewareContext(); const symbol = Symbol('test'); + const context = new MiddlewareContext<{ [symbol]: string }>(); context.set(symbol, 'value'); expect(context.assertGet(symbol)).toBe('value'); }); it('throws if setting an already set key', () => { - const context = new MiddlewareContext(); + const context = new MiddlewareContext<{ test: string }>(); context.set('test', 'value'); expect(() => context.set('test', 'value')).toThrow( `MiddlewareContext key "test" already exists`, diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts index 5bfa1ac1985..8eb291dcc7b 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts @@ -1,29 +1,134 @@ +import type { UnionToIntersection } from './utils'; + /** - * A append-only context object for middleware. Its interface is frozen. + * An context object for middleware that attempts to protect against accidental + * modifications. Its interface is frozen. + * + * Map keys may not be directly overridden with {@link set}. Instead, use + * {@link delete} to remove a key and then {@link set} to add a new value. * - * Map keys may still be deleted. The append-only behavior is mostly intended - * to prevent accidental naming collisions. + * The override protections are circumvented when using e.g. `Reflect.set`, so + * don't do that. * - * The append-only behavior is overriden when using e.g. `Reflect.set`, - * so don't do that. + * @template KeyValues - The type of the keys and values in the context. + * @example + * // By default, the context permits any PropertyKey as a key. + * const context = new MiddlewareContext(); + * context.set('foo', 'bar'); + * context.get('foo'); // 'bar' + * context.get('fizz'); // undefined + * @example + * // By specifying an object type, the context permits only the keys of the object. + * type Context = MiddlewareContext<{ foo: string }>; + * const context = new Context([['foo', 'bar']]); + * context.get('foo'); // 'bar' + * context.get('fizz'); // Type error */ -export class MiddlewareContext extends Map { - constructor(entries?: Iterable) { +export class MiddlewareContext< + KeyValues extends Record = Record, +> extends Map { + constructor( + entries?: Iterable, + ) { super(entries); Object.freeze(this); } - assertGet(key: string | symbol): Value { - if (!this.has(key)) { + get(key: K): KeyValues[K] | undefined { + return super.get(key) as KeyValues[K] | undefined; + } + + /** + * Get a value from the context. Throws if the key is not found. + * + * @param key - The key to get the value for. + * @returns The value. + */ + assertGet(key: K): KeyValues[K] { + if (!super.has(key)) { throw new Error(`Context key "${String(key)}" not found`); } - return this.get(key) as Value; + return super.get(key) as KeyValues[K]; } - set(key: string | symbol, value: Value): this { - if (this.has(key)) { + /** + * Set a value in the context. Throws if the key already exists. + * {@link delete} an existing key before setting it to a new value. + * + * @throws If the key already exists. + * @param key - The key to set the value for. + * @param value - The value to set. + * @returns The context. + */ + set(key: K, value: KeyValues[K]): this { + if (super.has(key)) { throw new Error(`MiddlewareContext key "${String(key)}" already exists`); } - return super.set(key, value) as this; + super.set(key, value); + return this; } } + +/** + * Infer the KeyValues type from a {@link MiddlewareContext}. + */ +type InferKeyValues = T extends MiddlewareContext ? U : never; + +/** + * Simplifies an object type by "merging" its properties. + * + * - Expands intersections into a single object type. + * - Forces mapped/conditional results to resolve into a readable shape. + * - No runtime effect; purely a type-level normalization. + * + * @example + * type A = { a: string } & { b: number }; + * type B = Simplify; // { a: string; b: number } + */ +type Simplify = T extends infer O ? { [K in keyof O]: O[K] } : never; + +/** + * Rejects record types that contain any `never`-valued property. + * + * If any property of `T` resolves to `never`, the result is `never`; otherwise it returns `T` unchanged. + * Useful as a guard to ensure computed/merged record types didn't collapse any fields to `never`. + * + * @example + * type A = ExcludeNever<{ a: string; b: never }>; // never + * type B = ExcludeNever<{ a: string; b: number }>; // { a: string; b: number } + */ +type ExcludeNever> = { + [K in keyof T]-?: [T[K]] extends [never] ? K : never; +}[keyof T] extends never + ? T + : never; + +/** + * Merge a union of {@link MiddlewareContext}s into a single {@link MiddlewareContext} + * supertype. + * + * @param Contexts - The union of {@link MiddlewareContext}s to merge. + * @returns The merged {@link MiddlewareContext} supertype. + * @example + * type A = MiddlewareContext<{ a: string }> | MiddlewareContext<{ b: number }>; + * type B = MergeContexts; // MiddlewareContext<{ a: string, b: number }> + */ +export type MergeContexts = + ExcludeNever< + Simplify>> + > extends never + ? never + : MiddlewareContext< + ExcludeNever>>> + >; + +// Non-polluting `any` constraint. +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type ContextConstraint = MiddlewareContext; + +/** + * The empty context type, i.e. `MiddlewareContext<{}>`. + */ +// The empty object type is literally an empty object in this context. +// eslint-disable-next-line @typescript-eslint/no-empty-object-type +export type EmptyContext = MiddlewareContext<{}>; diff --git a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts index b83652976cd..b7968898f62 100644 --- a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts +++ b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts @@ -5,23 +5,27 @@ import type { } from '@metamask/utils'; import { asLegacyMiddleware } from './asLegacyMiddleware'; -import type { JsonRpcMiddleware } from './JsonRpcEngineV2'; +import type { JsonRpcMiddleware, ResultConstraint } from './JsonRpcEngineV2'; import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; -import { getExtraneousKeys, makeRequest } from '../../tests/utils'; +import { + getExtraneousKeys, + makeRequest, + makeRequestMiddleware, +} from '../../tests/utils'; import { JsonRpcEngine } from '../JsonRpcEngine'; describe('asLegacyMiddleware', () => { it('converts a v2 engine to a legacy middleware', () => { - const engine = new JsonRpcEngineV2({ - middleware: [() => null], + const engine = JsonRpcEngineV2.create({ + middleware: [makeRequestMiddleware()], }); const middleware = asLegacyMiddleware(engine); expect(typeof middleware).toBe('function'); }); it('forwards a result to the legacy engine', async () => { - const v2Engine = new JsonRpcEngineV2({ - middleware: [() => null], + const v2Engine = JsonRpcEngineV2.create({ + middleware: [makeRequestMiddleware()], }); const legacyEngine = new JsonRpcEngine(); @@ -35,8 +39,9 @@ describe('asLegacyMiddleware', () => { }); it('forwarded results are not frozen', async () => { - const v2Engine = new JsonRpcEngineV2({ - middleware: [() => []], + const v2Middleware: JsonRpcMiddleware = () => []; + const v2Engine = JsonRpcEngineV2.create({ + middleware: [v2Middleware], }); const legacyEngine = new JsonRpcEngine(); @@ -51,12 +56,11 @@ describe('asLegacyMiddleware', () => { }); it('forwards an error to the legacy engine', async () => { - const v2Engine = new JsonRpcEngineV2({ - middleware: [ - () => { - throw new Error('test'); - }, - ], + const v2Middleware: JsonRpcMiddleware = () => { + throw new Error('test'); + }; + const v2Engine = JsonRpcEngineV2.create({ + middleware: [v2Middleware], }); const legacyEngine = new JsonRpcEngine(); @@ -79,8 +83,10 @@ describe('asLegacyMiddleware', () => { }); it('allows the legacy engine to continue when not ending the request', async () => { - const v2Middleware = jest.fn(({ next }) => next()); - const v2Engine = new JsonRpcEngineV2({ + const v2Middleware: JsonRpcMiddleware = jest.fn( + ({ next }) => next(), + ); + const v2Engine = JsonRpcEngineV2.create({ middleware: [v2Middleware], }); @@ -99,8 +105,10 @@ describe('asLegacyMiddleware', () => { }); it('allows the legacy engine to continue when not ending the request (passing through the original request)', async () => { - const v2Middleware = jest.fn(({ request, next }) => next(request)); - const v2Engine = new JsonRpcEngineV2({ + const v2Middleware: JsonRpcMiddleware = jest.fn( + ({ request, next }) => next(request), + ); + const v2Engine = JsonRpcEngineV2.create({ middleware: [v2Middleware], }); @@ -119,7 +127,7 @@ describe('asLegacyMiddleware', () => { }); it('propagates request modifications to the legacy engine', async () => { - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create>({ middleware: [ ({ request, next }) => next({ ...request, method: 'test_request_2' }), ], @@ -147,15 +155,18 @@ describe('asLegacyMiddleware', () => { const observedContextValues: number[] = []; const v2Middleware = jest.fn((({ context, next }) => { - observedContextValues.push(context.assertGet('value')); + observedContextValues.push(context.assertGet('value') as number); expect(Array.from(context.keys())).toStrictEqual(['value']); context.set('newValue', 2); return next(); - }) satisfies JsonRpcMiddleware); + }) satisfies JsonRpcMiddleware< + JsonRpcRequest, + ResultConstraint + >); - const v2Engine = new JsonRpcEngineV2({ + const v2Engine = JsonRpcEngineV2.create({ middleware: [v2Middleware], }); diff --git a/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts b/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts index d8c477ae601..9e75753daa3 100644 --- a/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts +++ b/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts @@ -123,7 +123,7 @@ describe('compatibility-utils', () => { params: undefined, }; - // @ts-expect-error - Intentional abuse + // @ts-expect-error - Destructive testing const request = fromLegacyRequest(legacyRequest); expect(request).toStrictEqual({ @@ -140,7 +140,7 @@ describe('compatibility-utils', () => { id: 42, }; - // @ts-expect-error - Intentional abuse + // @ts-expect-error - Destructive testing const request = fromLegacyRequest(legacyRequest); expect(request).toStrictEqual({ @@ -159,7 +159,7 @@ describe('compatibility-utils', () => { id: 42, }; - // @ts-expect-error - Intentional abuse + // @ts-expect-error - Destructive testing const request = fromLegacyRequest(legacyRequest); expect(request).toStrictEqual({ @@ -265,7 +265,7 @@ describe('compatibility-utils', () => { params: [1], id: 42, }; - const context = new MiddlewareContext(); + const context = new MiddlewareContext>(); context.set('extraProp', 'value'); context.set('anotherProp', { nested: true }); @@ -282,16 +282,18 @@ describe('compatibility-utils', () => { }); it('does not copy non-string properties from context to request', () => { + const symbol = Symbol('anotherProp'); + const context = new MiddlewareContext(); + context.set('extraProp', 'value'); + context.set(symbol, { nested: true }); + context.set(42, 'value'); + const request = { jsonrpc, method: 'test_method', params: [1], id: 42, }; - const context = new MiddlewareContext(); - context.set('extraProp', 'value'); - context.set(Symbol('anotherProp'), { nested: true }); - propagateToRequest(request, context); expect(request).toStrictEqual({ @@ -301,6 +303,7 @@ describe('compatibility-utils', () => { id: 42, extraProp: 'value', }); + expect(symbol in request).toBe(false); }); it('excludes JSON-RPC properties from propagation', () => { @@ -310,7 +313,7 @@ describe('compatibility-utils', () => { params: [1], id: 42, }; - const context = new MiddlewareContext(); + const context = new MiddlewareContext>(); context.set('jsonrpc', '3.0'); context.set('method', 'other_method'); context.set('params', [2]); @@ -336,7 +339,7 @@ describe('compatibility-utils', () => { id: 42, existingKey: 'oldValue', }; - const context = new MiddlewareContext(); + const context = new MiddlewareContext>(); context.set('existingKey', 'newValue'); propagateToRequest(request, context); diff --git a/packages/json-rpc-engine/src/v2/compatibility-utils.ts b/packages/json-rpc-engine/src/v2/compatibility-utils.ts index d4eebf9cccc..03257b6a2ef 100644 --- a/packages/json-rpc-engine/src/v2/compatibility-utils.ts +++ b/packages/json-rpc-engine/src/v2/compatibility-utils.ts @@ -87,7 +87,7 @@ export function makeContext>( */ export function propagateToContext( req: Record, - context: MiddlewareContext, + context: MiddlewareContext>, ) { Object.keys(req) .filter( diff --git a/packages/json-rpc-engine/src/v2/index.ts b/packages/json-rpc-engine/src/v2/index.ts index e9298e2ae67..eb63fa05b7b 100644 --- a/packages/json-rpc-engine/src/v2/index.ts +++ b/packages/json-rpc-engine/src/v2/index.ts @@ -2,6 +2,12 @@ export { asLegacyMiddleware } from './asLegacyMiddleware'; export { getUniqueId } from '../getUniqueId'; export * from './JsonRpcEngineV2'; export { JsonRpcServer } from './JsonRpcServer'; -export type { MiddlewareContext } from './MiddlewareContext'; +export type { MiddlewareContext, EmptyContext } from './MiddlewareContext'; export { isNotification, isRequest, JsonRpcEngineError } from './utils'; -export type { JsonRpcCall, JsonRpcNotification, JsonRpcRequest } from './utils'; +export type { + Json, + JsonRpcCall, + JsonRpcNotification, + JsonRpcParams, + JsonRpcRequest, +} from './utils'; diff --git a/packages/json-rpc-engine/src/v2/utils.test.ts b/packages/json-rpc-engine/src/v2/utils.test.ts index 7dcc5185ebe..f7fde4f0e05 100644 --- a/packages/json-rpc-engine/src/v2/utils.test.ts +++ b/packages/json-rpc-engine/src/v2/utils.test.ts @@ -27,7 +27,7 @@ describe('utils', () => { }, false, ], - ])('should return $expected for $request', (request, expected) => { + ])('returns $expected for $request', (request, expected) => { expect(isRequest(request)).toBe(expected); }); }); @@ -39,13 +39,13 @@ describe('utils', () => { { id: 1, jsonrpc, method: 'eth_getBlockByNumber', params: ['latest'] }, false, ], - ])('should return $expected for $request', (request, expected) => { + ])('returns $expected for $request', (request, expected) => { expect(isNotification(request)).toBe(expected); }); }); describe('stringify', () => { - it('should stringify a JSON object', () => { + it('stringifies a JSON object', () => { expect(stringify({ foo: 'bar' })).toMatchInlineSnapshot(` "{ \\"foo\\": \\"bar\\" @@ -55,11 +55,17 @@ describe('utils', () => { }); describe('JsonRpcEngineError', () => { - it('should create an error with the correct name', () => { + it('creates an error with the correct name', () => { const error = new JsonRpcEngineError('test'); expect(error).toBeInstanceOf(Error); expect(error.name).toBe('JsonRpcEngineError'); expect(error.message).toBe('test'); }); + + it('isInstance checks if a value is a JsonRpcEngineError instance', () => { + const error = new JsonRpcEngineError('test'); + expect(JsonRpcEngineError.isInstance(error)).toBe(true); + expect(JsonRpcEngineError.isInstance(new Error('test'))).toBe(false); + }); }); }); diff --git a/packages/json-rpc-engine/src/v2/utils.ts b/packages/json-rpc-engine/src/v2/utils.ts index abe761098cb..345c9907b18 100644 --- a/packages/json-rpc-engine/src/v2/utils.ts +++ b/packages/json-rpc-engine/src/v2/utils.ts @@ -1,15 +1,16 @@ import { hasProperty, - type JsonRpcNotification as BaseJsonRpcNotification, + type JsonRpcNotification, type JsonRpcParams, - type JsonRpcRequest as BaseJsonRpcRequest, + type JsonRpcRequest, } from '@metamask/utils'; -export type JsonRpcNotification = - BaseJsonRpcNotification; - -export type JsonRpcRequest = - BaseJsonRpcRequest; +export type { + Json, + JsonRpcParams, + JsonRpcRequest, + JsonRpcNotification, +} from '@metamask/utils'; export type JsonRpcCall = | JsonRpcNotification @@ -23,6 +24,20 @@ export const isNotification = ( msg: JsonRpcCall, ): msg is JsonRpcNotification => !isRequest(msg); +/** + * An unholy incantation that converts a union of object types into an + * intersection of object types. + * + * @example + * type A = { a: string } | { b: number }; + * type B = UnionToIntersection; // { a: string } & { b: number } + */ +export type UnionToIntersection = ( + U extends never ? never : (k: U) => void +) extends (k: infer I) => void + ? I + : never; + /** * JSON-stringifies a value. * @@ -33,9 +48,29 @@ export function stringify(value: unknown): string { return JSON.stringify(value, null, 2); } +const JsonRpcEngineErrorSymbol = Symbol.for('JsonRpcEngineError'); + export class JsonRpcEngineError extends Error { + private readonly [JsonRpcEngineErrorSymbol] = true; + constructor(message: string) { super(message); this.name = 'JsonRpcEngineError'; } + + /** + * Check if a value is a {@link JsonRpcEngineError} instance. + * Works across different package versions in the same realm. + * + * @param value - The value to check. + * @returns Whether the value is a {@link JsonRpcEngineError} instance. + */ + static isInstance( + value: Value, + ): value is Value & JsonRpcEngineError { + return ( + hasProperty(value, JsonRpcEngineErrorSymbol) && + value[JsonRpcEngineErrorSymbol] === true + ); + } } diff --git a/packages/json-rpc-engine/tests/utils.ts b/packages/json-rpc-engine/tests/utils.ts index 03f3d748e0c..2ed886379a9 100644 --- a/packages/json-rpc-engine/tests/utils.ts +++ b/packages/json-rpc-engine/tests/utils.ts @@ -1,4 +1,5 @@ import type { JsonRpcRequest } from '@metamask/utils'; +import type { JsonRpcMiddleware } from 'src/v2/JsonRpcEngineV2'; import { requestProps } from '../src/v2/compatibility-utils'; import type { JsonRpcNotification } from '../src/v2/utils'; @@ -24,6 +25,34 @@ export const makeNotification = >( ...params, }) as JsonRpcNotification; +/** + * Creates a {@link JsonRpcCall} middleware that returns `null`. + * + * @returns The middleware. + */ +export const makeNullMiddleware = (): JsonRpcMiddleware => { + return () => null; +}; + +/** + * Creates a {@link JsonRpcRequest} middleware that returns `null`. + * + * @returns The middleware. + */ +export const makeRequestMiddleware = (): JsonRpcMiddleware => { + return () => null; +}; + +/** + * Creates a {@link JsonRpcNotification} middleware that returns `undefined`. + * + * @returns The middleware. + */ +export const makeNotificationMiddleware = + (): JsonRpcMiddleware => { + return () => undefined; + }; + /** * Get the keys of a request that are not part of the standard JSON-RPC request * properties.