Initial notification handling implementation

Author: rekmarks

src/engine.test.ts renamed to src/JsonRpcEngine.test.ts

@@ -30,13 +30,13 @@ 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);
+    let response: any = await engine.handle({ id: 1, method: null } as any);
     expect(response.error.code).toStrictEqual(-32600);
     expect(response.result).toBeUndefined();
 
+    // No response if duck-typed as a notification
     response = await engine.handle({ method: true } as any);
-    expect(response.error.code).toStrictEqual(-32600);
-    expect(response.result).toBeUndefined();
+    expect(response).toBeUndefined();
   });
 
   it('handle: basic middleware test 1', async () => {

src/JsonRpcEngine.ts

@@ -4,6 +4,9 @@ import {
   JsonRpcError,
   JsonRpcRequest,
   JsonRpcResponse,
+  JsonRpcNotification,
+  isJsonRpcRequest,
+  isNullOrUndefined,
 } from '@metamask/utils';
 import { errorCodes, EthereumRpcError, serializeError } from 'eth-rpc-errors';
 
@@ -36,16 +39,23 @@ export type JsonRpcMiddleware<Params, Result> = (
   end: JsonRpcEngineEndCallback,
 ) => void;
 
+export type JsonRpcNotificationHandler<Params> = (
+  notification: JsonRpcNotification<Params>,
+) => void | Promise<void>;
+
 /**
  * A JSON-RPC request and response processor.
  * Give it a stack of middleware, pass it requests, and get back responses.
  */
 export class JsonRpcEngine extends SafeEventEmitter {
   private _middleware: JsonRpcMiddleware<unknown, unknown>[];
 
-  constructor() {
+  private readonly _notificationHandler?: JsonRpcNotificationHandler<unknown>;
+
+  constructor(notificationHandler?: JsonRpcNotificationHandler<unknown>) {
     super();
     this._middleware = [];
+    this._notificationHandler = notificationHandler;
   }
 
   /**
@@ -69,14 +79,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<Params>(
+    notification: JsonRpcNotification<Params>,
+    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<Params, Result>(
-    requests: JsonRpcRequest<Params>[],
+    requests: (JsonRpcRequest<Params> | JsonRpcNotification<Params>)[],
     callback: (error: unknown, responses: JsonRpcResponse<Result>[]) => void,
   ): void;
 
@@ -91,13 +113,21 @@ export class JsonRpcEngine extends SafeEventEmitter {
   ): Promise<JsonRpcResponse<Result>>;
 
   /**
-   * 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<Params>(notification: JsonRpcNotification<Params>): Promise<void>;
+
+  /**
+   * 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<Params, Result>(
-    requests: JsonRpcRequest<Params>[],
+    requests: (JsonRpcRequest<Params> | JsonRpcNotification<Params>)[],
   ): Promise<JsonRpcResponse<Result>[]>;
 
   handle(req: unknown, callback?: any) {
@@ -153,14 +183,14 @@ export class JsonRpcEngine extends SafeEventEmitter {
    * Like _handle, but for batch requests.
    */
   private _handleBatch(
-    reqs: JsonRpcRequest<unknown>[],
+    reqs: (JsonRpcRequest<unknown> | JsonRpcNotification<unknown>)[],
   ): Promise<JsonRpcResponse<unknown>[]>;
 
   /**
    * Like _handle, but for batch requests.
    */
   private _handleBatch(
-    reqs: JsonRpcRequest<unknown>[],
+    reqs: (JsonRpcRequest<unknown> | JsonRpcNotification<unknown>)[],
     callback: (error: unknown, responses?: JsonRpcResponse<unknown>[]) => void,
   ): Promise<void>;
 
@@ -173,17 +203,22 @@ export class JsonRpcEngine extends SafeEventEmitter {
    * @returns The array of responses, or nothing if a callback was specified.
    */
   private async _handleBatch(
-    reqs: JsonRpcRequest<unknown>[],
+    reqs: (JsonRpcRequest<unknown> | JsonRpcNotification<unknown>)[],
     callback?: (error: unknown, responses?: JsonRpcResponse<unknown>[]) => void,
   ): Promise<JsonRpcResponse<unknown>[] | 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 out falsy responses from notifications
+        )
+      ).filter(
+        (response) => !isNullOrUndefined(response),
+      ) as JsonRpcResponse<unknown>[];
 
       // 3. Return batch response
       if (callback) {
@@ -206,8 +241,8 @@ export class JsonRpcEngine extends SafeEventEmitter {
    * @returns The JSON-RPC response.
    */
   private _promiseHandle(
-    req: JsonRpcRequest<unknown>,
-  ): Promise<JsonRpcResponse<unknown>> {
+    req: JsonRpcRequest<unknown> | JsonRpcNotification<unknown>,
+  ): Promise<JsonRpcResponse<unknown> | void> {
     return new Promise((resolve) => {
       this._handle(req, (_err, res) => {
         // There will always be a response, and it will always have any error
@@ -218,8 +253,8 @@ export class JsonRpcEngine extends SafeEventEmitter {
   }
 
   /**
-   * 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.
    *
@@ -228,8 +263,8 @@ export class JsonRpcEngine extends SafeEventEmitter {
    * @returns Nothing.
    */
   private async _handle(
-    callerReq: JsonRpcRequest<unknown>,
-    callback: (error: unknown, response: JsonRpcResponse<unknown>) => void,
+    callerReq: JsonRpcRequest<unknown> | JsonRpcNotification<unknown>,
+    callback: (error?: unknown, response?: JsonRpcResponse<unknown>) => void,
   ): Promise<void> {
     if (
       !callerReq ||
@@ -250,18 +285,37 @@ 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 (isJsonRpcRequest(callerReq)) {
+        return callback(error, {
+          id: callerReq.id ?? null,
+          jsonrpc: '2.0',
+          error,
+        });
+      }
+      // Do not reply to notifications, even malformed ones.
+      return callback(null);
     }
 
+    // Handle notifications.
+    // We can't use isJsonRpcNotification here because that narrows callerReq to
+    // "never" after the if clause for unknown reasons.
+    if (!isJsonRpcRequest(callerReq)) {
+      await this._notificationHandler?.(callerReq);
+      return callback(null);
+    }
+
+    let error: JsonRpcEngineCallbackError = null;
+
+    // Handle requests.
     const req: JsonRpcRequest<unknown> = { ...callerReq };
     const res: PendingJsonRpcResponse<unknown> = {
       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.
@@ -286,13 +340,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<unknown>,
     res: PendingJsonRpcResponse<unknown>,
+    middlewares: JsonRpcMiddleware<unknown, unknown>[],
   ): Promise<void> {
     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.
@@ -314,15 +370,15 @@ 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.
    */
   private static async _runAllMiddleware(
     req: JsonRpcRequest<unknown>,
     res: PendingJsonRpcResponse<unknown>,
-    middlewareStack: JsonRpcMiddleware<unknown, unknown>[],
+    middlewares: JsonRpcMiddleware<unknown, unknown>[],
   ): Promise<
     [
       unknown, // error
@@ -335,7 +391,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,