Use async/await and try/catch in place of Promise methods (#74)

* Remove use of .then/.catch in asMiddleware

* Use async/await in _handleBatch

* Use async/await in createAsyncMiddleware

Author: rekmarks

src/JsonRpcEngine.ts

@@ -155,10 +155,7 @@ export class JsonRpcEngine extends SafeEventEmitter {
 
     if (Array.isArray(req)) {
       if (cb) {
-        this._handleBatch(req)
-          .then((res) => cb(null, res as JsonRpcResponse<unknown>[]))
-          .catch((err) => cb(err)); // fatal error
-        return undefined;
+        return this._handleBatch(req, cb);
       }
       return this._handleBatch(req);
     }
@@ -176,44 +173,73 @@ export class JsonRpcEngine extends SafeEventEmitter {
    * @returns This engine as a middleware function.
    */
   asMiddleware(): JsonRpcMiddleware<unknown, unknown> {
-    return (req, res, next, end) => {
-      JsonRpcEngine._runAllMiddleware(req, res, this._middleware)
-        .then(
-          async ([middlewareError, isComplete, returnHandlers]) => {
-            if (isComplete) {
-              await JsonRpcEngine._runReturnHandlers(returnHandlers);
-              return end(middlewareError as JsonRpcEngineCallbackError);
-            }
+    return async (req, res, next, end) => {
+      try {
+        const [
+          middlewareError,
+          isComplete,
+          returnHandlers,
+        ] = await JsonRpcEngine._runAllMiddleware(req, res, this._middleware);
+
+        if (isComplete) {
+          await JsonRpcEngine._runReturnHandlers(returnHandlers);
+          return end(middlewareError as JsonRpcEngineCallbackError);
+        }
 
-            return next(async (handlerCallback) => {
-              try {
-                await JsonRpcEngine._runReturnHandlers(returnHandlers);
-              } catch (error) {
-                return handlerCallback(error);
-              }
-              return handlerCallback();
-            });
-          },
-        )
-        .catch((error) => {
-          end(error);
+        return next(async (handlerCallback) => {
+          try {
+            await JsonRpcEngine._runReturnHandlers(returnHandlers);
+          } catch (error) {
+            return handlerCallback(error);
+          }
+          return handlerCallback();
         });
+      } catch (error) {
+        return end(error);
+      }
     };
   }
 
   /**
    * Like _handle, but for batch requests.
    */
+  private _handleBatch(
+    reqs: JsonRpcRequest<unknown>[],
+  ): Promise<JsonRpcResponse<unknown>[]>;
+
+  /**
+   * Like _handle, but for batch requests.
+   */
+  private _handleBatch(
+    reqs: JsonRpcRequest<unknown>[],
+    cb: (error: unknown, responses?: JsonRpcResponse<unknown>[]) => void,
+  ): Promise<void>;
+
   private async _handleBatch(
     reqs: JsonRpcRequest<unknown>[],
-  ): Promise<JsonRpcResponse<unknown>[]> {
+    cb?: (error: unknown, responses?: JsonRpcResponse<unknown>[]) => void,
+  ): Promise<JsonRpcResponse<unknown>[] | void> {
     // The order here is important
-    // 3. Return batch response, or reject on some kind of fatal error
-    return await Promise.all(
-      // 2. Wait for all requests to finish
-      // 1. Begin executing each request in the order received
-      reqs.map(this._promiseHandle.bind(this)),
-    );
+    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)),
+      );
+
+      // 3. Return batch response
+      if (cb) {
+        return cb(null, responses);
+      }
+      return responses;
+    } catch (error) {
+      if (cb) {
+        return cb(error);
+      }
+
+      throw error;
+    }
   }
 
   /**
@@ -330,11 +356,13 @@ export class JsonRpcEngine extends SafeEventEmitter {
     req: JsonRpcRequest<unknown>,
     res: InternalJsonRpcResponse,
     middlewareStack: JsonRpcMiddleware<unknown, unknown>[],
-  ): Promise<[
+  ): Promise<
+    [
       unknown, // error
       boolean, // isComplete
       JsonRpcEngineReturnHandler[],
-    ]> {
+    ]
+    > {
     const returnHandlers: JsonRpcEngineReturnHandler[] = [];
     let error = null;
     let isComplete = false;
@@ -384,12 +412,16 @@ export class JsonRpcEngine extends SafeEventEmitter {
         } else {
           if (returnHandler) {
             if (typeof returnHandler !== 'function') {
-              end(new EthereumRpcError(
-                errorCodes.rpc.internal,
-                `JsonRpcEngine: "next" return handlers must be functions. ` +
-                `Received "${typeof returnHandler}" for request:\n${jsonify(req)}`,
-                { request: req },
-              ));
+              end(
+                new EthereumRpcError(
+                  errorCodes.rpc.internal,
+                  `JsonRpcEngine: "next" return handlers must be functions. ` +
+                    `Received "${typeof returnHandler}" for request:\n${jsonify(
+                      req,
+                    )}`,
+                  { request: req },
+                ),
+              );
             }
             returnHandlers.push(returnHandler);
           }
@@ -433,7 +465,9 @@ export class JsonRpcEngine extends SafeEventEmitter {
     if (!('result' in res) && !('error' in res)) {
       throw new EthereumRpcError(
         errorCodes.rpc.internal,
-        `JsonRpcEngine: Response has no error or result for request:\n${jsonify(req)}`,
+        `JsonRpcEngine: Response has no error or result for request:\n${jsonify(
+          req,
+        )}`,
         { request: req },
       );
     }

src/createAsyncMiddleware.ts

@@ -12,6 +12,8 @@ export type AsyncJsonrpcMiddleware<T, U> = (
   next: AsyncJsonRpcEngineNextCallback
 ) => Promise<void>;
 
+type ReturnHandlerCallback = (error: null | Error) => void;
+
 /**
  * JsonRpcEngine only accepts callback-based middleware directly.
  * createAsyncMiddleware exists to enable consumers to pass in async middleware
@@ -32,7 +34,7 @@ export type AsyncJsonrpcMiddleware<T, U> = (
 export function createAsyncMiddleware<T, U>(
   asyncMiddleware: AsyncJsonrpcMiddleware<T, U>,
 ): JsonRpcMiddleware<T, U> {
-  return (req, res, next, end) => {
+  return async (req, res, next, end) => {
     // nextPromise is the key to the implementation
     // it is resolved by the return handler passed to the
     // "next" function
@@ -41,8 +43,8 @@ export function createAsyncMiddleware<T, U>(
       resolveNextPromise = resolve;
     });
 
-    let returnHandlerCallback: (error: null | Error) => void;
-    let nextWasCalled: boolean;
+    let returnHandlerCallback: unknown = null;
+    let nextWasCalled = false;
 
     // This will be called by the consumer's async middleware.
     const asyncNext = async () => {
@@ -51,28 +53,29 @@ export function createAsyncMiddleware<T, U>(
       // We pass a return handler to next(). When it is called by the engine,
       // the consumer's async middleware will resume executing.
       // eslint-disable-next-line node/callback-return
-      next((callback) => {
-        returnHandlerCallback = callback;
+      next((runReturnHandlersCallback) => {
+        // This callback comes from JsonRpcEngine._runReturnHandlers
+        returnHandlerCallback = runReturnHandlersCallback;
         resolveNextPromise();
       });
       await nextPromise;
     };
 
-    asyncMiddleware(req, res, asyncNext)
-      .then(async () => {
-        if (nextWasCalled) {
-          await nextPromise; // we must wait until the return handler is called
-          returnHandlerCallback(null);
-        } else {
-          end(null);
-        }
-      })
-      .catch((error) => {
-        if (returnHandlerCallback) {
-          returnHandlerCallback(error);
-        } else {
-          end(error);
-        }
-      });
+    try {
+      await asyncMiddleware(req, res, asyncNext);
+
+      if (nextWasCalled) {
+        await nextPromise; // we must wait until the return handler is called
+        (returnHandlerCallback as ReturnHandlerCallback)(null);
+      } else {
+        end(null);
+      }
+    } catch (error) {
+      if (returnHandlerCallback) {
+        (returnHandlerCallback as ReturnHandlerCallback)(error);
+      } else {
+        end(error);
+      }
+    }
   };
 }