diff --git a/README.md b/README.md index 8ed2541..b881e73 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A tool for processing JSON-RPC requests and responses. ```js const { JsonRpcEngine } = require('json-rpc-engine'); -let engine = new JsonRpcEngine(); +const engine = new JsonRpcEngine(); ``` Build a stack of JSON-RPC processors by pushing middleware to the engine. @@ -22,7 +22,7 @@ engine.push(function (req, res, next, end) { Requests are handled asynchronously, stepping down the stack until complete. ```js -let request = { id: 1, jsonrpc: '2.0', method: 'hello' }; +const request = { id: 1, jsonrpc: '2.0', method: 'hello' }; engine.handle(request, function (err, response) { // Do something with response.result, or handle response.error @@ -53,6 +53,18 @@ engine.push(function (req, res, next, end) { }); ``` +If you specify a `notificationHandler` when constructing the engine, JSON-RPC notifications passed to `handle()` will be handed off directly to this function without touching the middleware stack: + +```js +const engine = new JsonRpcEngine({ notificationHandler }); + +// A notification is defined as a JSON-RPC request without an `id` property. +const notification = { jsonrpc: '2.0', method: 'hello' }; + +const response = await engine.handle(notification); +console.log(typeof response); // 'undefined' +``` + Engines can be nested by converting them to middleware using `JsonRpcEngine.asMiddleware()`: ```js diff --git a/src/JsonRpcEngine.test.ts b/src/JsonRpcEngine.test.ts index b47f657..d807ae1 100644 --- a/src/JsonRpcEngine.test.ts +++ b/src/JsonRpcEngine.test.ts @@ -11,13 +11,13 @@ const jsonrpc = '2.0' as const; describe('JsonRpcEngine', () => { it('handle: throws on truthy, non-function callback', () => { - const engine: any = new JsonRpcEngine(); - expect(() => engine.handle({}, true)).toThrow( + const engine = new JsonRpcEngine(); + expect(() => engine.handle({} as any, 'foo' as any)).toThrow( '"callback" must be a function if provided.', ); }); - it('handle: returns error for invalid request parameter', async () => { + it('handle: returns error for invalid request value', async () => { const engine = new JsonRpcEngine(); let response: any = await engine.handle(null as any); expect(response.error.code).toStrictEqual(-32600); @@ -30,15 +30,104 @@ describe('JsonRpcEngine', () => { it('handle: returns error for invalid request method', async () => { const engine = new JsonRpcEngine(); - let response: any = await engine.handle({ method: null } as any); + const response: any = await engine.handle({ id: 1, method: null } as any); + expect(response.error.code).toStrictEqual(-32600); expect(response.result).toBeUndefined(); + }); + + it('handle: returns error for invalid request method with nullish id', async () => { + const engine = new JsonRpcEngine(); + const response: any = await engine.handle({ + id: undefined, + method: null, + } as any); - response = await engine.handle({ method: true } as any); expect(response.error.code).toStrictEqual(-32600); expect(response.result).toBeUndefined(); }); + it('handle: returns undefined for malformed notifications', async () => { + const middleware = jest.fn(); + const notificationHandler = jest.fn(); + const engine = new JsonRpcEngine({ notificationHandler }); + engine.push(middleware); + + expect( + await engine.handle({ jsonrpc, method: true } as any), + ).toBeUndefined(); + expect(notificationHandler).not.toHaveBeenCalled(); + expect(middleware).not.toHaveBeenCalled(); + }); + + it('handle: treats notifications as requests when no notification handler is specified', async () => { + const middleware = jest.fn().mockImplementation((_req, res, _next, end) => { + res.result = 'bar'; + end(); + }); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + expect(await engine.handle({ jsonrpc, method: 'foo' })).toStrictEqual({ + jsonrpc, + result: 'bar', + id: undefined, + }); + expect(middleware).toHaveBeenCalledTimes(1); + }); + + it('handle: forwards notifications to handlers', async () => { + const middleware = jest.fn(); + const notificationHandler = jest.fn(); + const engine = new JsonRpcEngine({ notificationHandler }); + engine.push(middleware); + + expect(await engine.handle({ jsonrpc, method: 'foo' })).toBeUndefined(); + expect(notificationHandler).toHaveBeenCalledTimes(1); + expect(notificationHandler).toHaveBeenCalledWith({ + jsonrpc, + method: 'foo', + }); + expect(middleware).not.toHaveBeenCalled(); + }); + + it('handle: re-throws errors from notification handlers (async)', async () => { + const notificationHandler = jest.fn().mockImplementation(() => { + throw new Error('baz'); + }); + const engine = new JsonRpcEngine({ notificationHandler }); + + await expect(engine.handle({ jsonrpc, method: 'foo' })).rejects.toThrow( + new Error('baz'), + ); + expect(notificationHandler).toHaveBeenCalledTimes(1); + expect(notificationHandler).toHaveBeenCalledWith({ + jsonrpc, + method: 'foo', + }); + }); + + it('handle: re-throws errors from notification handlers (callback)', async () => { + const notificationHandler = jest.fn().mockImplementation(() => { + throw new Error('baz'); + }); + const engine = new JsonRpcEngine({ notificationHandler }); + + await new Promise((resolve) => { + engine.handle({ jsonrpc, method: 'foo' }, (error, response) => { + expect(error).toStrictEqual(new Error('baz')); + expect(response).toBeUndefined(); + + expect(notificationHandler).toHaveBeenCalledTimes(1); + expect(notificationHandler).toHaveBeenCalledWith({ + jsonrpc, + method: 'foo', + }); + resolve(); + }); + }); + }); + it('handle: basic middleware test 1', async () => { const engine = new JsonRpcEngine(); diff --git a/src/JsonRpcEngine.ts b/src/JsonRpcEngine.ts index 1b1ad17..e101271 100644 --- a/src/JsonRpcEngine.ts +++ b/src/JsonRpcEngine.ts @@ -4,6 +4,8 @@ import { JsonRpcError, JsonRpcRequest, JsonRpcResponse, + JsonRpcNotification, + isJsonRpcRequest, } from '@metamask/utils'; import { errorCodes, EthereumRpcError, serializeError } from 'eth-rpc-errors'; @@ -42,6 +44,23 @@ export interface JsonRpcMiddleware { const DESTROYED_ERROR_MESSAGE = 'This engine is destroyed and can no longer be used.'; +export type JsonRpcNotificationHandler = ( + notification: JsonRpcNotification, +) => void | Promise; + +interface JsonRpcEngineArgs { + /** + * A function for handling JSON-RPC notifications. A JSON-RPC notification is + * defined as a JSON-RPC request without an `id` property. If this option is + * _not_ provided, notifications will be treated the same as requests. If this + * option _is_ provided, notifications will be passed to the handler + * function without touching the engine's middleware stack. + * + * This function should not throw or reject. + */ + notificationHandler?: JsonRpcNotificationHandler; +} + /** * A JSON-RPC request and response processor. * Give it a stack of middleware, pass it requests, and get back responses. @@ -54,9 +73,23 @@ export class JsonRpcEngine extends SafeEventEmitter { private _middleware: JsonRpcMiddleware[]; - constructor() { + private readonly _notificationHandler?: JsonRpcNotificationHandler; + + /** + * Constructs a {@link JsonRpcEngine} instance. + * + * @param options - Options bag. + * @param options.notificationHandler - A function for handling JSON-RPC + * notifications. A JSON-RPC notification is defined as a JSON-RPC request + * without an `id` property. If this option is _not_ provided, notifications + * will be treated the same as requests. If this option _is_ provided, + * notifications will be passed to the handler function without touching + * the engine's middleware stack. This function should not throw or reject. + */ + constructor({ notificationHandler }: JsonRpcEngineArgs = {}) { super(); this._middleware = []; + this._notificationHandler = notificationHandler; } /** @@ -112,14 +145,26 @@ export class JsonRpcEngine extends SafeEventEmitter { ): void; /** - * Handle an array of JSON-RPC requests, and return an array of responses. + * Handle a JSON-RPC notification. + * + * @param notification - The notification to handle. + * @param callback - An error-first callback that will receive a `void` response. + */ + handle( + notification: JsonRpcNotification, + callback: (error: unknown, response: void) => void, + ): void; + + /** + * Handle an array of JSON-RPC requests and/or notifications, and return an + * array of responses to any included requests. * * @param request - The requests to handle. * @param callback - An error-first callback that will receive the array of * responses. */ handle( - requests: JsonRpcRequest[], + requests: (JsonRpcRequest | JsonRpcNotification)[], callback: (error: unknown, responses: JsonRpcResponse[]) => void, ): void; @@ -134,13 +179,21 @@ export class JsonRpcEngine extends SafeEventEmitter { ): Promise>; /** - * Handle an array of JSON-RPC requests, and return an array of responses. + * Handle a JSON-RPC notification. + * + * @param notification - The notification to handle. + */ + handle(notification: JsonRpcNotification): Promise; + + /** + * Handle an array of JSON-RPC requests and/or notifications, and return an + * array of responses to any included requests. * * @param request - The JSON-RPC requests to handle. * @returns An array of JSON-RPC responses. */ handle( - requests: JsonRpcRequest[], + requests: (JsonRpcRequest | JsonRpcNotification)[], ): Promise[]>; handle(req: unknown, callback?: any) { @@ -199,14 +252,14 @@ export class JsonRpcEngine extends SafeEventEmitter { * Like _handle, but for batch requests. */ private _handleBatch( - reqs: JsonRpcRequest[], + reqs: (JsonRpcRequest | JsonRpcNotification)[], ): Promise[]>; /** * Like _handle, but for batch requests. */ private _handleBatch( - reqs: JsonRpcRequest[], + reqs: (JsonRpcRequest | JsonRpcNotification)[], callback: (error: unknown, responses?: JsonRpcResponse[]) => void, ): Promise; @@ -219,17 +272,22 @@ export class JsonRpcEngine extends SafeEventEmitter { * @returns The array of responses, or nothing if a callback was specified. */ private async _handleBatch( - reqs: JsonRpcRequest[], + reqs: (JsonRpcRequest | JsonRpcNotification)[], callback?: (error: unknown, responses?: JsonRpcResponse[]) => void, ): Promise[] | void> { // The order here is important try { // 2. Wait for all requests to finish, or throw on some kind of fatal // error - const responses = await Promise.all( - // 1. Begin executing each request in the order received - reqs.map(this._promiseHandle.bind(this)), - ); + const responses = ( + await Promise.all( + // 1. Begin executing each request in the order received + reqs.map(this._promiseHandle.bind(this)), + ) + ).filter( + // Filter out any notification responses. + (response) => response !== undefined, + ) as JsonRpcResponse[]; // 3. Return batch response if (callback) { @@ -252,20 +310,26 @@ export class JsonRpcEngine extends SafeEventEmitter { * @returns The JSON-RPC response. */ private _promiseHandle( - req: JsonRpcRequest, - ): Promise> { - return new Promise((resolve) => { - this._handle(req, (_err, res) => { - // There will always be a response, and it will always have any error - // that is caught and propagated. + req: JsonRpcRequest | JsonRpcNotification, + ): Promise | void> { + return new Promise((resolve, reject) => { + this._handle(req, (error, res) => { + // For notifications, the response will be `undefined`, and any caught + // errors are unexpected and should be surfaced to the caller. + if (error && res === undefined) { + reject(error); + } + + // Excepting notifications, there will always be a response, and it will + // always have any error that is caught and propagated. resolve(res); }); }); } /** - * Ensures that the request object is valid, processes it, and passes any - * error and the response object to the given callback. + * Ensures that the request / notification object is valid, processes it, and + * passes any error and response object to the given callback. * * Does not reject. * @@ -274,8 +338,8 @@ export class JsonRpcEngine extends SafeEventEmitter { * @returns Nothing. */ private async _handle( - callerReq: JsonRpcRequest, - callback: (error: unknown, response: JsonRpcResponse) => void, + callerReq: JsonRpcRequest | JsonRpcNotification, + callback: (error?: unknown, response?: JsonRpcResponse) => void, ): Promise { if ( !callerReq || @@ -296,18 +360,45 @@ export class JsonRpcEngine extends SafeEventEmitter { `Must specify a string method. Received: ${typeof callerReq.method}`, { request: callerReq }, ); - return callback(error, { id: callerReq.id, jsonrpc: '2.0', error }); + + if (this._notificationHandler && !isJsonRpcRequest(callerReq)) { + // Do not reply to notifications, even if they are malformed. + return callback(null); + } + + return callback(error, { + // Typecast: This could be a notification, but we want to access the + // `id` even if it doesn't exist. + id: (callerReq as JsonRpcRequest).id ?? null, + jsonrpc: '2.0', + error, + }); } - const req: JsonRpcRequest = { ...callerReq }; + // Handle notifications. + // We can't use isJsonRpcNotification here because that narrows callerReq to + // "never" after the if clause for unknown reasons. + if (this._notificationHandler && !isJsonRpcRequest(callerReq)) { + try { + await this._notificationHandler(callerReq); + } catch (error) { + return callback(error); + } + return callback(null); + } + + let error: JsonRpcEngineCallbackError = null; + + // Handle requests. + // Typecast: Permit missing id's for backwards compatibility. + const req = { ...(callerReq as JsonRpcRequest) }; const res: PendingJsonRpcResponse = { id: req.id, jsonrpc: req.jsonrpc, }; - let error: JsonRpcEngineCallbackError = null; try { - await this._processRequest(req, res); + await JsonRpcEngine._processRequest(req, res, this._middleware); } catch (_error) { // A request handler error, a re-thrown middleware error, or something // unexpected. @@ -332,13 +423,15 @@ export class JsonRpcEngine extends SafeEventEmitter { * * @param req - The request object. * @param res - The response object. + * @param middlewares - The stack of middleware functions. */ - private async _processRequest( + private static async _processRequest( req: JsonRpcRequest, res: PendingJsonRpcResponse, + middlewares: JsonRpcMiddleware[], ): Promise { const [error, isComplete, returnHandlers] = - await JsonRpcEngine._runAllMiddleware(req, res, this._middleware); + await JsonRpcEngine._runAllMiddleware(req, res, middlewares); // Throw if "end" was not called, or if the response has neither a result // nor an error. @@ -360,7 +453,7 @@ export class JsonRpcEngine extends SafeEventEmitter { * * @param req - The request object. * @param res - The response object. - * @param middlewareStack - The stack of middleware functions to execute. + * @param middlewares - The stack of middleware functions to execute. * @returns An array of any error encountered during middleware execution, * a boolean indicating whether the request was completed, and an array of * middleware-defined return handlers. @@ -368,7 +461,7 @@ export class JsonRpcEngine extends SafeEventEmitter { private static async _runAllMiddleware( req: JsonRpcRequest, res: PendingJsonRpcResponse, - middlewareStack: JsonRpcMiddleware[], + middlewares: JsonRpcMiddleware[], ): Promise< [ unknown, // error @@ -381,7 +474,7 @@ export class JsonRpcEngine extends SafeEventEmitter { let isComplete = false; // Go down stack of middleware, call and collect optional returnHandlers - for (const middleware of middlewareStack) { + for (const middleware of middlewares) { [error, isComplete] = await JsonRpcEngine._runMiddleware( req, res,