diff --git a/packages/json-rpc-engine/CHANGELOG.md b/packages/json-rpc-engine/CHANGELOG.md index e3617409f25..6b0fcd75c3c 100644 --- a/packages/json-rpc-engine/CHANGELOG.md +++ b/packages/json-rpc-engine/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `JsonRpcEngineV2` ([#6176](https://github.com/MetaMask/core/pull/6176), [#6971](https://github.com/MetaMask/core/pull/6971), [#6975](https://github.com/MetaMask/core/pull/6975), [#6990](https://github.com/MetaMask/core/pull/6990)) +- `JsonRpcEngineV2` ([#6176](https://github.com/MetaMask/core/pull/6176), [#6971](https://github.com/MetaMask/core/pull/6971), [#6975](https://github.com/MetaMask/core/pull/6975), [#6990](https://github.com/MetaMask/core/pull/6990), [#6991](https://github.com/MetaMask/core/pull/6991)) - This is a complete rewrite of `JsonRpcEngine`, intended to replace the original implementation. See the readme for details. diff --git a/packages/json-rpc-engine/README.md b/packages/json-rpc-engine/README.md index 464755feaf8..8611bba9141 100644 --- a/packages/json-rpc-engine/README.md +++ b/packages/json-rpc-engine/README.md @@ -81,8 +81,15 @@ await server.handle(notification); ### Legacy compatibility -Use the `asLegacyMiddleware` function to use a `JsonRpcEngineV2` as a -middleware in a legacy `JsonRpcEngine`: +Use `asLegacyMiddleware()` to convert a `JsonRpcEngineV2` or one or more V2 middleware into a legacy middleware. + +#### Context propagation + +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 **only `string` keys** of +the `context` will be copied over._ + +#### Converting a V2 engine ```ts import { @@ -102,9 +109,31 @@ const v2Engine = JsonRpcEngineV2.create({ 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 **only `string` keys** of -the `context` will be copied over._ +#### Converting V2 middleware + +```ts +import { + asLegacyMiddleware, + type JsonRpcMiddleware, +} from '@metamask/json-rpc-engine/v2'; +import { JsonRpcEngine } from '@metamask/json-rpc-engine'; + +// Convert a single V2 middleware +const middleware1: JsonRpcMiddleware = ({ request }) => { + /* ... */ +}; + +const legacyEngine = new JsonRpcEngine(); +legacyEngine.push(asLegacyMiddleware(middleware1)); + +// Convert multiple V2 middlewares at once +const middleware2: JsonRpcMiddleware = ({ context, next }) => { + /* ... */ +}; + +const legacyEngine2 = new JsonRpcEngine(); +legacyEngine2.push(asLegacyMiddleware(middleware1, middleware2)); +``` ### Middleware diff --git a/packages/json-rpc-engine/src/README.md b/packages/json-rpc-engine/src/README.md index a83af82fbc2..48be9407e05 100644 --- a/packages/json-rpc-engine/src/README.md +++ b/packages/json-rpc-engine/src/README.md @@ -23,8 +23,15 @@ engine.push(function (req, res, next, end) { ### V2 compatibility -Use the `asV2Middleware` function to use a `JsonRpcEngine` as a middleware in a -`JsonRpcEngineV2`: +Use `asV2Middleware()` to convert a `JsonRpcEngine` or one or more legacy middleware into a V2 middleware. + +#### Context propagation + +Non-JSON-RPC string properties on the request object will be copied over to the V2 engine's `context` object +once the legacy engine is done with the request, _unless_ they already exist on the `context`, in which case +they will be ignored. + +#### Converting a legacy engine ```ts import { JsonRpcEngineV2 } from '@metamask/json-rpc-engine/v2'; @@ -38,9 +45,32 @@ const v2Engine = JsonRpcEngineV2.create({ }); ``` -Non-JSON-RPC string properties on the request object will be copied over to the V2 engine's `context` object -once the legacy engine is done with the request, _unless_ they already exist on the `context`, in which case -they will be ignored. +#### Converting legacy middleware + +You can also directly convert one or more legacy middlewares without creating an engine: + +```ts +import { JsonRpcEngineV2 } from '@metamask/json-rpc-engine/v2'; +import { asV2Middleware } from '@metamask/json-rpc-engine'; + +// Convert a single legacy middleware +const middleware1 = (req, res, next, end) => { + /* ... */ +}; + +const v2Engine = JsonRpcEngineV2.create({ + middleware: [asV2Middleware(middleware1)], +}); + +// Convert multiple legacy middlewares at once +const middleware2 = (req, res, next, end) => { + /* ... */ +}; + +const v2Engine2 = JsonRpcEngineV2.create({ + middleware: [asV2Middleware(middleware1, middleware2)], +}); +``` ### Middleware diff --git a/packages/json-rpc-engine/src/asV2Middleware.test.ts b/packages/json-rpc-engine/src/asV2Middleware.test.ts index 81d06190e93..d268b4cf3f9 100644 --- a/packages/json-rpc-engine/src/asV2Middleware.test.ts +++ b/packages/json-rpc-engine/src/asV2Middleware.test.ts @@ -117,4 +117,72 @@ describe('asV2Middleware', () => { await v2Engine.handle(makeRequest()); expect(observedContextValues).toStrictEqual([1, 2]); }); + + describe('with legacy middleware', () => { + it('accepts a single legacy middleware', async () => { + const legacyMiddleware = jest.fn((_req, res, _next, end) => { + res.result = 'test-result'; + end(); + }); + + const v2Engine = JsonRpcEngineV2.create({ + middleware: [asV2Middleware(legacyMiddleware)], + }); + + const result = await v2Engine.handle(makeRequest()); + expect(result).toBe('test-result'); + expect(legacyMiddleware).toHaveBeenCalledTimes(1); + }); + + it('accepts multiple legacy middlewares via rest params', async () => { + const middleware1 = jest.fn((req, _res, next) => { + req.visited1 = true; + next(); + }); + + const middleware2 = jest.fn((req, res, _next, end) => { + expect(req.visited1).toBe(true); + res.result = 'composed-result'; + end(); + }); + + const v2Engine = JsonRpcEngineV2.create({ + middleware: [asV2Middleware(middleware1, middleware2)], + }); + + const result = await v2Engine.handle(makeRequest()); + expect(result).toBe('composed-result'); + expect(middleware1).toHaveBeenCalledTimes(1); + expect(middleware2).toHaveBeenCalledTimes(1); + }); + + it('forwards errors from legacy middleware', async () => { + const legacyMiddleware = jest.fn((_req, res, _next, end) => { + res.error = rpcErrors.internal('legacy-error'); + end(); + }); + + const v2Engine = JsonRpcEngineV2.create({ + middleware: [asV2Middleware(legacyMiddleware)], + }); + + await expect(v2Engine.handle(makeRequest())).rejects.toThrow( + rpcErrors.internal('legacy-error'), + ); + }); + + it('allows v2 engine to continue when legacy middleware does not end', async () => { + const legacyMiddleware = jest.fn((_req, _res, next) => { + next(); + }); + + const v2Engine = JsonRpcEngineV2.create({ + middleware: [asV2Middleware(legacyMiddleware), makeNullMiddleware()], + }); + + const result = await v2Engine.handle(makeRequest()); + expect(result).toBeNull(); + expect(legacyMiddleware).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/packages/json-rpc-engine/src/asV2Middleware.ts b/packages/json-rpc-engine/src/asV2Middleware.ts index 31d514643c8..34f4bd34877 100644 --- a/packages/json-rpc-engine/src/asV2Middleware.ts +++ b/packages/json-rpc-engine/src/asV2Middleware.ts @@ -2,6 +2,7 @@ import { serializeError } from '@metamask/rpc-errors'; import type { JsonRpcFailure, JsonRpcResponse } from '@metamask/utils'; import { hasProperty, + type Json, type JsonRpcParams, type JsonRpcRequest, } from '@metamask/utils'; @@ -11,6 +12,8 @@ import type { JsonRpcEngineEndCallback, JsonRpcEngineNextCallback, } from './JsonRpcEngine'; +import { type JsonRpcMiddleware as LegacyMiddleware } from './JsonRpcEngine'; +import { mergeMiddleware } from './mergeMiddleware'; import { deepClone, fromLegacyRequest, @@ -19,7 +22,7 @@ import { unserializeError, } from './v2/compatibility-utils'; import type { - // JsonRpcEngineV2 is used in docs. + // Used in docs. // eslint-disable-next-line @typescript-eslint/no-unused-vars JsonRpcEngineV2, JsonRpcMiddleware, @@ -35,8 +38,40 @@ import type { export function asV2Middleware< Params extends JsonRpcParams, Request extends JsonRpcRequest, ->(engine: JsonRpcEngine): JsonRpcMiddleware { - const middleware = engine.asMiddleware(); +>(engine: JsonRpcEngine): JsonRpcMiddleware; + +/** + * Convert one or more legacy middleware into a {@link JsonRpcEngineV2} middleware. + * + * @param middleware - The legacy middleware to convert. + * @returns The {@link JsonRpcEngineV2} middleware. + */ +export function asV2Middleware< + Params extends JsonRpcParams, + Request extends JsonRpcRequest, +>( + ...middleware: LegacyMiddleware[] +): JsonRpcMiddleware; + +/** + * The asV2Middleware implementation. + * + * @param engineOrMiddleware - A legacy engine or legacy middleware. + * @param rest - Any additional legacy middleware when the first argument is a middleware. + * @returns The {@link JsonRpcEngineV2} middleware. + */ +export function asV2Middleware< + Params extends JsonRpcParams, + Request extends JsonRpcRequest, +>( + engineOrMiddleware: JsonRpcEngine | LegacyMiddleware, + ...rest: LegacyMiddleware[] +): JsonRpcMiddleware { + const legacyMiddleware = + typeof engineOrMiddleware === 'function' + ? mergeMiddleware([engineOrMiddleware, ...rest]) + : engineOrMiddleware.asMiddleware(); + return async ({ request, context, next }) => { const req = deepClone(request) as JsonRpcRequest; propagateToRequest(req, context); @@ -62,7 +97,7 @@ export function asV2Middleware< const legacyNext = ((cb: JsonRpcEngineEndCallback) => cb(end)) as JsonRpcEngineNextCallback; - middleware(req, res, legacyNext, end); + legacyMiddleware(req, res, legacyNext, end); }); propagateToContext(req, context); diff --git a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts index b7968898f62..4274f2be027 100644 --- a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts +++ b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.test.ts @@ -190,4 +190,93 @@ describe('asLegacyMiddleware', () => { await legacyEngine.handle(makeRequest()); expect(observedContextValues).toStrictEqual([1, 2]); }); + + describe('with V2 middleware', () => { + it('accepts a single V2 middleware', async () => { + const v2Middleware: JsonRpcMiddleware = jest.fn( + () => 'test-result', + ); + + const legacyEngine = new JsonRpcEngine(); + legacyEngine.push(asLegacyMiddleware(v2Middleware)); + + const response = (await legacyEngine.handle( + makeRequest(), + )) as JsonRpcSuccess; + + expect(response.result).toBe('test-result'); + expect(v2Middleware).toHaveBeenCalledTimes(1); + }); + + it('accepts multiple V2 middlewares via rest params', async () => { + const middleware1: JsonRpcMiddleware = jest.fn( + ({ context, next }) => { + context.set('visited1', true); + return next(); + }, + ); + + const middleware2: JsonRpcMiddleware = jest.fn( + ({ context }) => { + expect(context.get('visited1')).toBe(true); + return 'composed-result'; + }, + ); + + const legacyEngine = new JsonRpcEngine(); + legacyEngine.push(asLegacyMiddleware(middleware1, middleware2)); + + const response = (await legacyEngine.handle( + makeRequest(), + )) as JsonRpcSuccess; + + expect(response.result).toBe('composed-result'); + expect(middleware1).toHaveBeenCalledTimes(1); + expect(middleware2).toHaveBeenCalledTimes(1); + }); + + it('forwards errors from V2 middleware', async () => { + const v2Middleware: JsonRpcMiddleware = jest.fn(() => { + throw new Error('v2-error'); + }); + + const legacyEngine = new JsonRpcEngine(); + legacyEngine.push(asLegacyMiddleware(v2Middleware)); + + const response = (await legacyEngine.handle( + makeRequest(), + )) as JsonRpcFailure; + + expect(response.error).toStrictEqual({ + message: 'v2-error', + code: -32603, + data: { + cause: { + message: 'v2-error', + stack: expect.any(String), + }, + }, + }); + }); + + it('allows legacy engine to continue when V2 middleware does not end', async () => { + const v2Middleware: JsonRpcMiddleware = jest.fn( + ({ next }) => next(), + ); + + const legacyEngine = new JsonRpcEngine(); + legacyEngine.push(asLegacyMiddleware(v2Middleware)); + legacyEngine.push((_req, res, _next, end) => { + res.result = 'continued'; + end(); + }); + + const response = (await legacyEngine.handle( + makeRequest(), + )) as JsonRpcSuccess; + + expect(response.result).toBe('continued'); + expect(v2Middleware).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.ts b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.ts index f5cfe6be406..66ceba3b26a 100644 --- a/packages/json-rpc-engine/src/v2/asLegacyMiddleware.ts +++ b/packages/json-rpc-engine/src/v2/asLegacyMiddleware.ts @@ -6,7 +6,8 @@ import { makeContext, propagateToRequest, } from './compatibility-utils'; -import type { JsonRpcEngineV2, ResultConstraint } from './JsonRpcEngineV2'; +import type { JsonRpcMiddleware, ResultConstraint } from './JsonRpcEngineV2'; +import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; import { createAsyncMiddleware } from '..'; import type { JsonRpcMiddleware as LegacyMiddleware } from '..'; @@ -21,14 +22,50 @@ export function asLegacyMiddleware< Request extends JsonRpcRequest, >( engine: JsonRpcEngineV2, +): LegacyMiddleware>; + +/** + * Convert one or more V2 middlewares into a legacy middleware. + * + * @param middleware - The V2 middleware(s) to convert. + * @returns The legacy middleware. + */ +export function asLegacyMiddleware< + Params extends JsonRpcParams, + Request extends JsonRpcRequest, +>( + ...middleware: JsonRpcMiddleware>[] +): LegacyMiddleware>; + +/** + * The asLegacyMiddleware implementation. + * + * @param engineOrMiddleware - A V2 engine or V2 middleware. + * @param rest - Any additional V2 middleware when the first argument is a middleware. + * @returns The legacy middleware. + */ +export function asLegacyMiddleware< + Params extends JsonRpcParams, + Request extends JsonRpcRequest, +>( + engineOrMiddleware: + | JsonRpcEngineV2 + | JsonRpcMiddleware>, + ...rest: JsonRpcMiddleware>[] ): LegacyMiddleware> { - const middleware = engine.asMiddleware(); + const v2Middleware = + typeof engineOrMiddleware === 'function' + ? JsonRpcEngineV2.create({ + middleware: [engineOrMiddleware, ...rest], + }).asMiddleware() + : engineOrMiddleware.asMiddleware(); + return createAsyncMiddleware(async (req, res, next) => { const request = fromLegacyRequest(req as Request); const context = makeContext(req); let modifiedRequest: Request | undefined; - const result = await middleware({ + const result = await v2Middleware({ request, context, next: (finalRequest) => {