From 378a8205e182921506ec981dc216471543d2952e Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Sat, 27 Mar 2021 10:37:25 -0700 Subject: [PATCH 1/6] Remove createAsyncMiddleware * Handle errors fro masync middleware function rejections * Update readme * Update JsonRpcMiddleware type * Prevent promise resolution function from being called twice --- README.md | 130 +++++++++------------------- src/JsonRpcEngine.test.ts | 42 +++++++++- src/JsonRpcEngine.ts | 88 ++++++++++--------- src/createAsyncMiddleware.test.ts | 135 ------------------------------ src/createAsyncMiddleware.ts | 95 --------------------- src/createScaffoldMiddleware.ts | 2 +- src/index.ts | 1 - 7 files changed, 128 insertions(+), 365 deletions(-) delete mode 100644 src/createAsyncMiddleware.test.ts delete mode 100644 src/createAsyncMiddleware.ts diff --git a/README.md b/README.md index 30b804c..a1e559d 100644 --- a/README.md +++ b/README.md @@ -39,11 +39,21 @@ They can let processing continue down the stack with `next()`, or complete the r engine.push(function (req, res, next, end) { if (req.skipCache) return next(); res.result = getResultFromCache(req); - end(); + return end(); }); ``` -By passing a _return handler_ to the `next` function, you can get a peek at the result before it returns. +Middleware functions can be `async`: + +```js +engine.push(async function (req, res, next, end) { + if (req.method !== targetMethod) return next(); + res.result = await processTargetMethodRequest(req); + return end(); +}); +``` + +By passing a _return handler_ to the `next` function, you can get a peek at the response before it is returned to the requester. ```js engine.push(function (req, res, next, end) { @@ -73,69 +83,45 @@ const subengine = new JsonRpcEngine(); engine.push(subengine.asMiddleware()); ``` -### `async` Middleware +### Error Handling -If you require your middleware function to be `async`, use `createAsyncMiddleware`: +Errors should be handled by throwing inside middleware functions. -```js -const { createAsyncMiddleware } = require('@metamask/json-rpc-engine'); - -let engine = new RpcEngine(); -engine.push( - createAsyncMiddleware(async (req, res, next) => { - res.result = 42; - next(); - }), -); -``` - -`async` middleware do not take an `end` callback. -Instead, the request ends if the middleware returns without calling `next()`: +For backwards compatibility, you can also pass an error to the `end` callback, +or set the error on the response object, and then call `end` or `next`. +However, errors must **not** be passed to the `next` callback. -```js -engine.push( - createAsyncMiddleware(async (req, res, next) => { - res.result = 42; - /* The request will end when this returns */ - }), -); -``` +Errors always take precedent over results. +If an error is detected, the response's `result` property will be deleted. -The `next` callback of `async` middleware also don't take return handlers. -Instead, you can `await next()`. -When the execution of the middleware resumes, you can work with the response again. +All of the following examples are equivalent. +It does not matter of the middleware function is synchronous or asynchronous. ```js -engine.push( - createAsyncMiddleware(async (req, res, next) => { - res.result = 42; - await next(); - /* Your return handler logic goes here */ - addToMetrics(res); - }), -); -``` +// Throwing is preferred. +engine.push(function (req, res, next, end) { + throw new Error(); +}); -You can freely mix callback-based and `async` middleware: +// For backwards compatibility, you can also do this: +engine.push(function (req, res, next, end) { + end(new Error()); +}); -```js engine.push(function (req, res, next, end) { - if (!isCached(req)) { - return next((cb) => { - insertIntoCache(res, cb); - }); - } - res.result = getResultFromCache(req); + res.error = new Error(); end(); }); -engine.push( - createAsyncMiddleware(async (req, res, next) => { - res.result = 42; - await next(); - addToMetrics(res); - }), -); +engine.push(function (req, res, next, end) { + res.error = new Error(); + next(); +}); + +// INCORRECT. Do not do this: +engine.push(function (req, res, next, end) { + next(new Error()); +}); ``` ### Teardown @@ -164,41 +150,3 @@ engine.destroy(); // will throw an error. engine.handle(req); ``` - -### Gotchas - -Handle errors via `end(err)`, _NOT_ `next(err)`. - -```js -/* INCORRECT */ -engine.push(function (req, res, next, end) { - next(new Error()); -}); - -/* CORRECT */ -engine.push(function (req, res, next, end) { - end(new Error()); -}); -``` - -However, `next()` will detect errors on the response object, and cause -`end(res.error)` to be called. - -```js -engine.push(function (req, res, next, end) { - res.error = new Error(); - next(); /* This will cause end(res.error) to be called. */ -}); -``` - -## Running tests - -Build the project if not already built: - -```bash -yarn build -``` - -```bash -yarn test -``` diff --git a/src/JsonRpcEngine.test.ts b/src/JsonRpcEngine.test.ts index fe77684..be946c0 100644 --- a/src/JsonRpcEngine.test.ts +++ b/src/JsonRpcEngine.test.ts @@ -499,8 +499,10 @@ describe('JsonRpcEngine', () => { }); }); - engine.push(function (_request, _response, next, _end) { + // Async middleware function + engine.push(async function (_request, _response, next, _end) { events.push('2-next'); + await delay(); next(function (callback) { events.push('2-return'); callback(); @@ -555,6 +557,33 @@ describe('JsonRpcEngine', () => { }); }); + it('calls back next handler even if async middleware rejects', async () => { + const engine = new JsonRpcEngine(); + + let sawNextReturnHandlerCalled = false; + + engine.push(function (_req, _res, next, _end) { + next(function (callback) { + sawNextReturnHandlerCalled = true; + callback(); + }); + }); + + engine.push(async function (_req, _res, _next, _end) { + throw new Error('boom'); + }); + + const payload = { id: 1, jsonrpc, method: 'hello' }; + + await new Promise((resolve) => { + engine.handle(payload, (error, _response) => { + expect(error).toBeDefined(); + expect(sawNextReturnHandlerCalled).toBe(true); + resolve(); + }); + }); + }); + it('handles error in next handler', async () => { const engine = new JsonRpcEngine(); @@ -673,3 +702,14 @@ describe('JsonRpcEngine', () => { }); }); }); + +/** + * Delay for a number of milliseconds. + * + * @param ms - The number of milliseconds to delay. + */ +async function delay(ms = 1) { + return new Promise((resolve) => { + setTimeout(() => resolve(), ms); + }); +} diff --git a/src/JsonRpcEngine.ts b/src/JsonRpcEngine.ts index c31421c..721af70 100644 --- a/src/JsonRpcEngine.ts +++ b/src/JsonRpcEngine.ts @@ -35,7 +35,7 @@ export type JsonRpcMiddleware< res: PendingJsonRpcResponse, next: JsonRpcEngineNextCallback, end: JsonRpcEngineEndCallback, - ): void; + ): void | Promise; destroy?: () => void | Promise; }; @@ -517,49 +517,55 @@ export class JsonRpcEngine extends SafeEventEmitter { middleware: JsonRpcMiddleware, returnHandlers: JsonRpcEngineReturnHandler[], ): Promise<[unknown, boolean]> { - return new Promise((resolve) => { - const end: JsonRpcEngineEndCallback = (error?: unknown) => { - const parsedError = error || response.error; - if (parsedError) { - response.error = serializeError(parsedError); - } - // True indicates that the request should end - resolve([parsedError, true]); - }; - - const next: JsonRpcEngineNextCallback = ( - returnHandler?: JsonRpcEngineReturnHandler, - ) => { - if (response.error) { - end(response.error); - } else { - if (returnHandler) { - if (typeof returnHandler !== 'function') { - end( - new JsonRpcError( - errorCodes.rpc.internal, - `JsonRpcEngine: "next" return handlers must be functions. ` + - `Received "${typeof returnHandler}" for request:\n${jsonify( - request, - )}`, - { request: request as Json }, - ), - ); - } - returnHandlers.push(returnHandler); - } + let resolve: (value: [unknown, boolean]) => void; + const middlewareCallbackPromise = new Promise<[unknown, boolean]>( + (_resolve) => { + resolve = _resolve; + }, + ); - // False indicates that the request should not end - resolve([null, false]); - } - }; + const end: JsonRpcEngineEndCallback = (error?: unknown) => { + const parsedError = error || response.error; + if (parsedError) { + response.error = serializeError(parsedError); + } + // True indicates that the request should end + resolve([parsedError, true]); + }; - try { - middleware(request, response, next, end); - } catch (error: any) { - end(error); + const next: JsonRpcEngineNextCallback = ( + returnHandler?: JsonRpcEngineReturnHandler, + ) => { + if (response.error) { + return end(response.error); } - }); + + if (returnHandler) { + if (typeof returnHandler !== 'function') { + return end( + new JsonRpcError( + errorCodes.rpc.internal, + `JsonRpcEngine: "next" return handlers must be functions. ` + + `Received "${typeof returnHandler}" for request:\n${jsonify( + request, + )}`, + { request: request as Json }, + ), + ); + } + returnHandlers.push(returnHandler); + } + + // False indicates that the request should not end + return resolve([null, false]); + }; + + try { + await middleware(request, response, next, end); + } catch (error: any) { + end(error); + } + return middlewareCallbackPromise; } /** diff --git a/src/createAsyncMiddleware.test.ts b/src/createAsyncMiddleware.test.ts deleted file mode 100644 index 087de22..0000000 --- a/src/createAsyncMiddleware.test.ts +++ /dev/null @@ -1,135 +0,0 @@ -import { assertIsJsonRpcSuccess } from '@metamask/utils'; - -import { JsonRpcEngine, createAsyncMiddleware } from '.'; - -const jsonrpc = '2.0' as const; - -describe('createAsyncMiddleware', () => { - it('basic middleware test', async () => { - const engine = new JsonRpcEngine(); - - engine.push( - createAsyncMiddleware(async (_request, response, _next) => { - response.result = 42; - }), - ); - - const payload = { id: 1, jsonrpc, method: 'hello' }; - - await new Promise((resolve) => { - engine.handle(payload, function (error, response) { - expect(error).toBeNull(); - expect(response).toBeDefined(); - assertIsJsonRpcSuccess(response); - expect(response.result).toBe(42); - resolve(); - }); - }); - }); - - it('next middleware test', async () => { - const engine = new JsonRpcEngine(); - - engine.push( - createAsyncMiddleware(async (_request, response, next) => { - expect(response.result).toBeUndefined(); - await next(); // eslint-disable-line node/callback-return - expect(response.result).toBe(1234); - // override value - response.result = 42; // eslint-disable-line require-atomic-updates - }), - ); - - engine.push(function (_request, response, _next, end) { - response.result = 1234; - end(); - }); - - const payload = { id: 1, jsonrpc, method: 'hello' }; - - await new Promise((resolve) => { - engine.handle(payload, function (error, response) { - expect(error).toBeNull(); - expect(response).toBeDefined(); - assertIsJsonRpcSuccess(response); - expect(response.result).toBe(42); - resolve(); - }); - }); - }); - - it('basic throw test', async () => { - const engine = new JsonRpcEngine(); - - const thrownError = new Error('bad boy'); - - engine.push( - createAsyncMiddleware(async (_req, _res, _next) => { - throw thrownError; - }), - ); - - const payload = { id: 1, jsonrpc, method: 'hello' }; - - await new Promise((resolve) => { - engine.handle(payload, function (error, _response) { - expect(error).toBeDefined(); - expect(error).toStrictEqual(thrownError); - resolve(); - }); - }); - }); - - it('throw after next test', async () => { - const engine = new JsonRpcEngine(); - - const thrownError = new Error('bad boy'); - - engine.push( - createAsyncMiddleware(async (_request, _response, next) => { - await next(); // eslint-disable-line node/callback-return - throw thrownError; - }), - ); - - engine.push(function (_request, response, _next, end) { - response.result = 1234; - end(); - }); - - const payload = { id: 1, jsonrpc, method: 'hello' }; - - await new Promise((resolve) => { - engine.handle(payload, function (error, _response) { - expect(error).toBeDefined(); - expect(error).toStrictEqual(thrownError); - resolve(); - }); - }); - }); - - it("doesn't await next", async () => { - const engine = new JsonRpcEngine(); - - engine.push( - createAsyncMiddleware(async (_request, _response, next) => { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - next(); - }), - ); - - engine.push(function (_request, response, _next, end) { - response.result = 1234; - end(); - }); - - const payload = { id: 1, jsonrpc, method: 'hello' }; - - await new Promise((resolve) => { - engine.handle(payload, function (error, _response) { - expect(error).toBeDefined(); - resolve(); - }); - }); - }); -}); diff --git a/src/createAsyncMiddleware.ts b/src/createAsyncMiddleware.ts deleted file mode 100644 index 1b90ac4..0000000 --- a/src/createAsyncMiddleware.ts +++ /dev/null @@ -1,95 +0,0 @@ -import { - Json, - JsonRpcParams, - JsonRpcRequest, - PendingJsonRpcResponse, -} from '@metamask/utils'; - -import { JsonRpcMiddleware } from './JsonRpcEngine'; - -export type AsyncJsonRpcEngineNextCallback = () => Promise; - -export type AsyncJsonrpcMiddleware< - Params extends JsonRpcParams, - Result extends Json, -> = ( - request: JsonRpcRequest, - response: PendingJsonRpcResponse, - next: AsyncJsonRpcEngineNextCallback, -) => Promise; - -type ReturnHandlerCallback = (error: null | Error) => void; - -/** - * JsonRpcEngine only accepts callback-based middleware directly. - * createAsyncMiddleware exists to enable consumers to pass in async middleware - * functions. - * - * Async middleware have no "end" function. Instead, they "end" if they return - * without calling "next". Rather than passing in explicit return handlers, - * async middleware can simply await "next", and perform operations on the - * response object when execution resumes. - * - * To accomplish this, createAsyncMiddleware passes the async middleware a - * wrapped "next" function. That function calls the internal JsonRpcEngine - * "next" function with a return handler that resolves a promise when called. - * - * The return handler will always be called. Its resolution of the promise - * enables the control flow described above. - * - * @param asyncMiddleware - The asynchronous middleware function to wrap. - * @returns The wrapped asynchronous middleware function, ready to be consumed - * by JsonRpcEngine. - */ -export function createAsyncMiddleware< - Params extends JsonRpcParams, - Result extends Json, ->( - asyncMiddleware: AsyncJsonrpcMiddleware, -): JsonRpcMiddleware { - // eslint-disable-next-line @typescript-eslint/no-misused-promises - return async (request, response, next, end) => { - // nextPromise is the key to the implementation - // it is resolved by the return handler passed to the - // "next" function - let resolveNextPromise: () => void; - const nextPromise = new Promise((resolve) => { - resolveNextPromise = resolve; - }); - - let returnHandlerCallback: unknown = null; - let nextWasCalled = false; - - // This will be called by the consumer's async middleware. - const asyncNext = async () => { - nextWasCalled = true; - - // 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((runReturnHandlersCallback) => { - // This callback comes from JsonRpcEngine._runReturnHandlers - returnHandlerCallback = runReturnHandlersCallback; - resolveNextPromise(); - }); - await nextPromise; - }; - - try { - await asyncMiddleware(request, response, asyncNext); - - if (nextWasCalled) { - await nextPromise; // we must wait until the return handler is called - (returnHandlerCallback as ReturnHandlerCallback)(null); - } else { - end(null); - } - } catch (error: any) { - if (returnHandlerCallback) { - (returnHandlerCallback as ReturnHandlerCallback)(error); - } else { - end(error); - } - } - }; -} diff --git a/src/createScaffoldMiddleware.ts b/src/createScaffoldMiddleware.ts index 8d4b6fa..ec340da 100644 --- a/src/createScaffoldMiddleware.ts +++ b/src/createScaffoldMiddleware.ts @@ -19,7 +19,7 @@ type ScaffoldMiddlewareHandler< export function createScaffoldMiddleware(handlers: { [methodName: string]: ScaffoldMiddlewareHandler; }): JsonRpcMiddleware { - return (req, res, next, end) => { + return async (req, res, next, end) => { const handler = handlers[req.method]; // if no handler, return if (handler === undefined) { diff --git a/src/index.ts b/src/index.ts index d7644d5..e8e28b9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,3 @@ -export * from './createAsyncMiddleware'; export * from './createScaffoldMiddleware'; export * from './getUniqueId'; export * from './idRemapMiddleware'; From 3fb1a7c44271d91c8d2fc3cd095806c9867edb33 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Fri, 19 May 2023 13:32:04 -0700 Subject: [PATCH 2/6] Add back and deprecate createAsyncMiddleware --- src/createAsyncMiddleware.test.ts | 135 ++++++++++++++++++++++++++++++ src/createAsyncMiddleware.ts | 117 ++++++++++++++++++++++++++ src/index.ts | 1 + 3 files changed, 253 insertions(+) create mode 100644 src/createAsyncMiddleware.test.ts create mode 100644 src/createAsyncMiddleware.ts diff --git a/src/createAsyncMiddleware.test.ts b/src/createAsyncMiddleware.test.ts new file mode 100644 index 0000000..087de22 --- /dev/null +++ b/src/createAsyncMiddleware.test.ts @@ -0,0 +1,135 @@ +import { assertIsJsonRpcSuccess } from '@metamask/utils'; + +import { JsonRpcEngine, createAsyncMiddleware } from '.'; + +const jsonrpc = '2.0' as const; + +describe('createAsyncMiddleware', () => { + it('basic middleware test', async () => { + const engine = new JsonRpcEngine(); + + engine.push( + createAsyncMiddleware(async (_request, response, _next) => { + response.result = 42; + }), + ); + + const payload = { id: 1, jsonrpc, method: 'hello' }; + + await new Promise((resolve) => { + engine.handle(payload, function (error, response) { + expect(error).toBeNull(); + expect(response).toBeDefined(); + assertIsJsonRpcSuccess(response); + expect(response.result).toBe(42); + resolve(); + }); + }); + }); + + it('next middleware test', async () => { + const engine = new JsonRpcEngine(); + + engine.push( + createAsyncMiddleware(async (_request, response, next) => { + expect(response.result).toBeUndefined(); + await next(); // eslint-disable-line node/callback-return + expect(response.result).toBe(1234); + // override value + response.result = 42; // eslint-disable-line require-atomic-updates + }), + ); + + engine.push(function (_request, response, _next, end) { + response.result = 1234; + end(); + }); + + const payload = { id: 1, jsonrpc, method: 'hello' }; + + await new Promise((resolve) => { + engine.handle(payload, function (error, response) { + expect(error).toBeNull(); + expect(response).toBeDefined(); + assertIsJsonRpcSuccess(response); + expect(response.result).toBe(42); + resolve(); + }); + }); + }); + + it('basic throw test', async () => { + const engine = new JsonRpcEngine(); + + const thrownError = new Error('bad boy'); + + engine.push( + createAsyncMiddleware(async (_req, _res, _next) => { + throw thrownError; + }), + ); + + const payload = { id: 1, jsonrpc, method: 'hello' }; + + await new Promise((resolve) => { + engine.handle(payload, function (error, _response) { + expect(error).toBeDefined(); + expect(error).toStrictEqual(thrownError); + resolve(); + }); + }); + }); + + it('throw after next test', async () => { + const engine = new JsonRpcEngine(); + + const thrownError = new Error('bad boy'); + + engine.push( + createAsyncMiddleware(async (_request, _response, next) => { + await next(); // eslint-disable-line node/callback-return + throw thrownError; + }), + ); + + engine.push(function (_request, response, _next, end) { + response.result = 1234; + end(); + }); + + const payload = { id: 1, jsonrpc, method: 'hello' }; + + await new Promise((resolve) => { + engine.handle(payload, function (error, _response) { + expect(error).toBeDefined(); + expect(error).toStrictEqual(thrownError); + resolve(); + }); + }); + }); + + it("doesn't await next", async () => { + const engine = new JsonRpcEngine(); + + engine.push( + createAsyncMiddleware(async (_request, _response, next) => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + next(); + }), + ); + + engine.push(function (_request, response, _next, end) { + response.result = 1234; + end(); + }); + + const payload = { id: 1, jsonrpc, method: 'hello' }; + + await new Promise((resolve) => { + engine.handle(payload, function (error, _response) { + expect(error).toBeDefined(); + resolve(); + }); + }); + }); +}); diff --git a/src/createAsyncMiddleware.ts b/src/createAsyncMiddleware.ts new file mode 100644 index 0000000..13ac897 --- /dev/null +++ b/src/createAsyncMiddleware.ts @@ -0,0 +1,117 @@ +import { + Json, + JsonRpcParams, + JsonRpcRequest, + PendingJsonRpcResponse, +} from '@metamask/utils'; + +import { JsonRpcMiddleware } from './JsonRpcEngine'; + +export type AsyncJsonRpcEngineNextCallback = () => Promise; + +export type AsyncJsonrpcMiddleware< + Params extends JsonRpcParams, + Result extends Json, +> = ( + request: JsonRpcRequest, + response: PendingJsonRpcResponse, + next: AsyncJsonRpcEngineNextCallback, +) => Promise; + +type ReturnHandlerCallback = (error: null | Error) => void; + +/** + * JsonRpcEngine only accepts callback-based middleware directly. + * createAsyncMiddleware exists to enable consumers to pass in async middleware + * functions. + * + * Async middleware have no "end" function. Instead, they "end" if they return + * without calling "next". Rather than passing in explicit return handlers, + * async middleware can simply await "next", and perform operations on the + * response object when execution resumes. + * + * To accomplish this, createAsyncMiddleware passes the async middleware a + * wrapped "next" function. That function calls the internal JsonRpcEngine + * "next" function with a return handler that resolves a promise when called. + * + * The return handler will always be called. Its resolution of the promise + * enables the control flow described above. + * + * @param asyncMiddleware - The asynchronous middleware function to wrap. + * @returns The wrapped asynchronous middleware function, ready to be consumed + * by JsonRpcEngine. + */ +export function createAsyncMiddleware< + Params extends JsonRpcParams, + Result extends Json, +>( + asyncMiddleware: AsyncJsonrpcMiddleware, +): JsonRpcMiddleware { + // eslint-disable-next-line @typescript-eslint/no-misused-promises + return (request, response, next, end) => { + // nextPromise is the key to the implementation + // it is resolved by the return handler passed to the + // "next" function + let resolveNextPromise: () => void; + const nextPromise = new Promise((resolve) => { + resolveNextPromise = resolve; + }); + + let returnHandlerCallback: unknown = null; + let nextWasCalled = false; + let endWasCalled = false; + + // The control flow gets pretty messy in here, and we have to guard against + // accidentally ending the request multiple times. + // If this function weren't deprecated we probably wouldn't tolerate this. + const innerEnd = (error: null | Error) => { + if (!endWasCalled) { + end(error); + } + endWasCalled = true; + }; + + // This will be called by the consumer's async middleware. + const asyncNext = async () => { + nextWasCalled = true; + + // 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((runReturnHandlersCallback) => { + // This callback comes from JsonRpcEngine.#runReturnHandlers + returnHandlerCallback = runReturnHandlersCallback; + resolveNextPromise(); + }); + await nextPromise; + }; + + // We have to await the async middleware in order to process the return handler + // and allow the engine to complete request handling. + (async () => { + try { + await asyncMiddleware(request, response, asyncNext); + + if (nextWasCalled) { + await nextPromise; // we must wait until the return handler is called + (returnHandlerCallback as ReturnHandlerCallback)(null); + } else { + innerEnd(null); + } + } catch (error: any) { + if (returnHandlerCallback) { + (returnHandlerCallback as ReturnHandlerCallback)(error); + } else { + innerEnd(error); + } + } + })() + // This is mostly to make the linter happy. It should not be possible to + // hit this in practice. + .catch( + /* istanbul ignore next */ (error) => { + innerEnd(error); + }, + ); + }; +} diff --git a/src/index.ts b/src/index.ts index e8e28b9..d7644d5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,3 +1,4 @@ +export * from './createAsyncMiddleware'; export * from './createScaffoldMiddleware'; export * from './getUniqueId'; export * from './idRemapMiddleware'; From a9028aff47fe2e25b78988d8c1f726cb3b34ed9a Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Fri, 19 May 2023 13:45:08 -0700 Subject: [PATCH 3/6] Update createAsyncMiddleware docstring --- src/createAsyncMiddleware.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/createAsyncMiddleware.ts b/src/createAsyncMiddleware.ts index 13ac897..9f50e79 100644 --- a/src/createAsyncMiddleware.ts +++ b/src/createAsyncMiddleware.ts @@ -21,22 +21,24 @@ export type AsyncJsonrpcMiddleware< type ReturnHandlerCallback = (error: null | Error) => void; /** - * JsonRpcEngine only accepts callback-based middleware directly. - * createAsyncMiddleware exists to enable consumers to pass in async middleware - * functions. + * Originally, JsonRpcEngine could only accept synchronous middleware functions. + * `createAsyncMiddleware` was created to enable consumers to pass in async + * middleware functions. * - * Async middleware have no "end" function. Instead, they "end" if they return - * without calling "next". Rather than passing in explicit return handlers, - * async middleware can simply await "next", and perform operations on the - * response object when execution resumes. + * These async middleware have no `end` function. Instead, they `end` if they + * return without calling `next`. Rather than passing in explicit return + * handlers, async middleware can simply await `next`, and perform operations + * on the response object when execution resumes. * - * To accomplish this, createAsyncMiddleware passes the async middleware a - * wrapped "next" function. That function calls the internal JsonRpcEngine - * "next" function with a return handler that resolves a promise when called. + * To accomplish this, `createAsyncMiddleware` passes the async middleware a + * wrapped `next` function. That function calls the internal `JsonRpcEngine` + * `next` function with a return handler that resolves a promise when called. * * The return handler will always be called. Its resolution of the promise * enables the control flow described above. * + * @deprecated As of version 7.1.0, middleware functions can be asnync. This + * should no longer be used. * @param asyncMiddleware - The asynchronous middleware function to wrap. * @returns The wrapped asynchronous middleware function, ready to be consumed * by JsonRpcEngine. From afd85f047ad7688fcd42a4567e69f43fb569c6d3 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Fri, 19 May 2023 13:47:43 -0700 Subject: [PATCH 4/6] Reorder some stuff --- src/createAsyncMiddleware.ts | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/createAsyncMiddleware.ts b/src/createAsyncMiddleware.ts index 9f50e79..685c7f8 100644 --- a/src/createAsyncMiddleware.ts +++ b/src/createAsyncMiddleware.ts @@ -63,18 +63,8 @@ export function createAsyncMiddleware< let nextWasCalled = false; let endWasCalled = false; - // The control flow gets pretty messy in here, and we have to guard against - // accidentally ending the request multiple times. - // If this function weren't deprecated we probably wouldn't tolerate this. - const innerEnd = (error: null | Error) => { - if (!endWasCalled) { - end(error); - } - endWasCalled = true; - }; - // This will be called by the consumer's async middleware. - const asyncNext = async () => { + const innerNext = async () => { nextWasCalled = true; // We pass a return handler to next(). When it is called by the engine, @@ -88,11 +78,21 @@ export function createAsyncMiddleware< await nextPromise; }; + // The control flow gets pretty messy in here, and we have to guard against + // accidentally ending the request multiple times. + // If this function weren't deprecated we probably wouldn't tolerate this. + const innerEnd = (error: null | Error) => { + if (!endWasCalled) { + end(error); + } + endWasCalled = true; + }; + // We have to await the async middleware in order to process the return handler // and allow the engine to complete request handling. (async () => { try { - await asyncMiddleware(request, response, asyncNext); + await asyncMiddleware(request, response, innerNext); if (nextWasCalled) { await nextPromise; // we must wait until the return handler is called From 36d91ad0848a46a28d93c5436863dba5138f0b09 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Fri, 19 May 2023 13:48:53 -0700 Subject: [PATCH 5/6] Undo unintended change to scaffold middleware --- src/createScaffoldMiddleware.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/createScaffoldMiddleware.ts b/src/createScaffoldMiddleware.ts index ec340da..8d4b6fa 100644 --- a/src/createScaffoldMiddleware.ts +++ b/src/createScaffoldMiddleware.ts @@ -19,7 +19,7 @@ type ScaffoldMiddlewareHandler< export function createScaffoldMiddleware(handlers: { [methodName: string]: ScaffoldMiddlewareHandler; }): JsonRpcMiddleware { - return async (req, res, next, end) => { + return (req, res, next, end) => { const handler = handlers[req.method]; // if no handler, return if (handler === undefined) { From 6d5ddc63cc4fd332f1be7671f02f6db400b3ddd7 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Fri, 19 May 2023 13:53:20 -0700 Subject: [PATCH 6/6] Revert "Undo unintended change to scaffold middleware" We actually need this to please the linter. This reverts commit 36d91ad0848a46a28d93c5436863dba5138f0b09. --- src/createScaffoldMiddleware.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/createScaffoldMiddleware.ts b/src/createScaffoldMiddleware.ts index 8d4b6fa..ec340da 100644 --- a/src/createScaffoldMiddleware.ts +++ b/src/createScaffoldMiddleware.ts @@ -19,7 +19,7 @@ type ScaffoldMiddlewareHandler< export function createScaffoldMiddleware(handlers: { [methodName: string]: ScaffoldMiddlewareHandler; }): JsonRpcMiddleware { - return (req, res, next, end) => { + return async (req, res, next, end) => { const handler = handlers[req.method]; // if no handler, return if (handler === undefined) {