From 817363d40e059f6c0db97556e79bf2076ca3b1e5 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Sun, 28 Mar 2021 13:34:03 -0700 Subject: [PATCH 1/4] Remove next callback --- src/JsonRpcEngine.ts | 45 +++++---- src/createScaffoldMiddleware.ts | 6 +- src/idRemapMiddleware.ts | 6 +- test/asMiddleware.spec.js | 52 +++++----- test/createScaffoldMiddleware.spec.js | 6 +- test/engine.spec.js | 137 +++++++++++++------------- test/idRemapMiddleware.spec.js | 7 +- test/mergeMiddleware.spec.js | 30 +++--- 8 files changed, 143 insertions(+), 146 deletions(-) diff --git a/src/JsonRpcEngine.ts b/src/JsonRpcEngine.ts index f71b2cf..2bd7178 100644 --- a/src/JsonRpcEngine.ts +++ b/src/JsonRpcEngine.ts @@ -67,11 +67,9 @@ export interface PendingJsonRpcResponse extends JsonRpcResponseBase { export type JsonRpcEngineCallbackError = Error | JsonRpcError | null; -export type JsonRpcEngineReturnHandler = () => void | Promise; +type MaybePromise = Promise | T; -export type JsonRpcEngineNextCallback = ( - returnHandlerCallback?: JsonRpcEngineReturnHandler -) => void; +export type JsonRpcEngineReturnHandler = () => MaybePromise; export type JsonRpcEngineEndCallback = ( error?: JsonRpcEngineCallbackError @@ -80,9 +78,8 @@ export type JsonRpcEngineEndCallback = ( export type JsonRpcMiddleware = ( req: JsonRpcRequest, res: PendingJsonRpcResponse, - next: JsonRpcEngineNextCallback, end: JsonRpcEngineEndCallback -) => void | Promise; +) => MaybePromise; /** * A JSON-RPC request and response processor. @@ -171,7 +168,7 @@ export class JsonRpcEngine extends SafeEventEmitter { * @returns This engine as a middleware function. */ asMiddleware(): JsonRpcMiddleware { - return async (req, res, next, end) => { + return async (req, res, end) => { try { const [ middlewareError, @@ -184,9 +181,9 @@ export class JsonRpcEngine extends SafeEventEmitter { return end(middlewareError as JsonRpcEngineCallbackError); } - return next(async () => { + return async () => { await JsonRpcEngine._runReturnHandlers(returnHandlers); - }); + }; } catch (error) { return end(error); } @@ -387,12 +384,10 @@ export class JsonRpcEngine extends SafeEventEmitter { middleware: JsonRpcMiddleware, returnHandlers: JsonRpcEngineReturnHandler[], ): Promise<[unknown, boolean]> { - let resolve: (value: [unknown, boolean]) => void; - const middlewareCallbackPromise = new Promise<[unknown, boolean]>( - (_resolve) => { - resolve = _resolve; - }, - ); + const [ + middlewareCallbackPromise, + resolve, + ] = getDeferredPromise<[unknown, boolean]>(); const end: JsonRpcEngineEndCallback = (err?: unknown) => { const error = err || res.error; @@ -403,9 +398,9 @@ export class JsonRpcEngine extends SafeEventEmitter { resolve([error, true]); }; - const next: JsonRpcEngineNextCallback = ( - returnHandler?: JsonRpcEngineReturnHandler, - ) => { + try { + const returnHandler = await middleware(req, res, end); + if (res.error) { end(res.error); } else { @@ -414,7 +409,7 @@ export class JsonRpcEngine extends SafeEventEmitter { end( new EthereumRpcError( errorCodes.rpc.internal, - `JsonRpcEngine: "next" return handlers must be functions. ` + + `JsonRpcEngine: return handlers must be functions. ` + `Received "${typeof returnHandler}" for request:\n${jsonify( req, )}`, @@ -428,10 +423,6 @@ export class JsonRpcEngine extends SafeEventEmitter { // False indicates that the request should not end resolve([null, false]); } - }; - - try { - await middleware(req, res, next, end); } catch (error) { end(error); } @@ -481,3 +472,11 @@ export class JsonRpcEngine extends SafeEventEmitter { function jsonify(request: JsonRpcRequest): string { return JSON.stringify(request, null, 2); } + +function getDeferredPromise(): [ Promise, (value: T) => void] { + let resolve: any; + const promise: Promise = new Promise((_resolve) => { + resolve = _resolve; + }); + return [promise, resolve]; +} diff --git a/src/createScaffoldMiddleware.ts b/src/createScaffoldMiddleware.ts index a6f48ae..23e29ad 100644 --- a/src/createScaffoldMiddleware.ts +++ b/src/createScaffoldMiddleware.ts @@ -5,15 +5,15 @@ type ScaffoldMiddlewareHandler = JsonRpcMiddleware | Json; export function createScaffoldMiddleware(handlers: { [methodName: string]: ScaffoldMiddlewareHandler; }): JsonRpcMiddleware { - return (req, res, next, end) => { + return (req, res, end) => { const handler = handlers[req.method]; // if no handler, return if (handler === undefined) { - return next(); + return undefined; } // if handler is fn, call as middleware if (typeof handler === 'function') { - return handler(req, res, next, end); + return handler(req, res, end); } // if handler is some other value, use as result (res as JsonRpcSuccess).result = handler; diff --git a/src/idRemapMiddleware.ts b/src/idRemapMiddleware.ts index b9972d6..78ed988 100644 --- a/src/idRemapMiddleware.ts +++ b/src/idRemapMiddleware.ts @@ -2,14 +2,14 @@ import { getUniqueId } from './getUniqueId'; import { JsonRpcMiddleware } from './JsonRpcEngine'; export function createIdRemapMiddleware(): JsonRpcMiddleware { - return (req, res, next, _end) => { + return (req, res, _end) => { const originalId = req.id; const newId = getUniqueId(); req.id = newId; res.id = newId; - next(() => { + return () => { req.id = originalId; res.id = originalId; - }); + }; }; } diff --git a/test/asMiddleware.spec.js b/test/asMiddleware.spec.js index 9f0d7d8..b9510bd 100644 --- a/test/asMiddleware.spec.js +++ b/test/asMiddleware.spec.js @@ -10,7 +10,7 @@ describe('asMiddleware', function () { const subengine = new JsonRpcEngine(); let originalReq; - subengine.push(function (req, res, _next, end) { + subengine.push(function (req, res, end) { originalReq = req; res.result = 'saw subengine'; end(); @@ -35,7 +35,7 @@ describe('asMiddleware', function () { const subengine = new JsonRpcEngine(); let originalReq; - subengine.push(function (req, res, _next, end) { + subengine.push(function (req, res, end) { originalReq = req; res.xyz = true; res.result = true; @@ -61,7 +61,7 @@ describe('asMiddleware', function () { const subengine = new JsonRpcEngine(); let originalReq; - subengine.push(function (req, res, _next, end) { + subengine.push(function (req, res, end) { originalReq = req; req.xyz = true; res.result = true; @@ -86,10 +86,10 @@ describe('asMiddleware', function () { const engine = new JsonRpcEngine(); const subengine = new JsonRpcEngine(); - subengine.push((_req, _res, next, _end) => next()); + subengine.push((_req, _res, _end) => undefined); engine.push(subengine.asMiddleware()); - engine.push((_req, res, _next, end) => { + engine.push((_req, res, end) => { res.result = true; end(); }); @@ -103,18 +103,18 @@ describe('asMiddleware', function () { }); }); - it('handles next handler correctly when nested', function (done) { + it('handles return handler correctly when nested', function (done) { const engine = new JsonRpcEngine(); const subengine = new JsonRpcEngine(); - subengine.push((_req, res, next, _end) => { - next(() => { + subengine.push((_req, res, _end) => { + return () => { res.copy = res.result; - }); + }; }); engine.push(subengine.asMiddleware()); - engine.push((_req, res, _next, end) => { + engine.push((_req, res, end) => { res.result = true; end(); }); @@ -128,17 +128,17 @@ describe('asMiddleware', function () { }); }); - it('handles next handler correctly when flat', function (done) { + it('handles return handler correctly when flat', function (done) { const engine = new JsonRpcEngine(); const subengine = new JsonRpcEngine(); - subengine.push((_req, res, next, _end) => { - next(() => { + subengine.push((_req, res, _end) => { + return () => { res.copy = res.result; - }); + }; }); - subengine.push((_req, res, _next, end) => { + subengine.push((_req, res, end) => { res.result = true; end(); }); @@ -158,7 +158,7 @@ describe('asMiddleware', function () { const engine = new JsonRpcEngine(); const subengine = new JsonRpcEngine(); - subengine.push(function (_req, _res, _next, _end) { + subengine.push(function (_req, _res, _end) { throw new Error('foo'); }); @@ -174,18 +174,18 @@ describe('asMiddleware', function () { }); }); - it('handles next handler error correctly when nested', function (done) { + it('handles return handler error correctly when nested', function (done) { const engine = new JsonRpcEngine(); const subengine = new JsonRpcEngine(); - subengine.push((_req, _res, next, _end) => { - next(() => { + subengine.push((_req, _res, _end) => { + return () => { throw new Error('foo'); - }); + }; }); engine.push(subengine.asMiddleware()); - engine.push((_req, res, _next, end) => { + engine.push((_req, res, end) => { res.result = true; end(); }); @@ -199,17 +199,17 @@ describe('asMiddleware', function () { }); }); - it('handles next handler error correctly when flat', function (done) { + it('handles return handler error correctly when flat', function (done) { const engine = new JsonRpcEngine(); const subengine = new JsonRpcEngine(); - subengine.push((_req, _res, next, _end) => { - next(() => { + subengine.push((_req, _res, _end) => { + return () => { throw new Error('foo'); - }); + }; }); - subengine.push((_req, res, _next, end) => { + subengine.push((_req, res, end) => { res.result = true; end(); }); diff --git a/test/createScaffoldMiddleware.spec.js b/test/createScaffoldMiddleware.spec.js index 8d4ecde..a350095 100644 --- a/test/createScaffoldMiddleware.spec.js +++ b/test/createScaffoldMiddleware.spec.js @@ -10,18 +10,18 @@ describe('createScaffoldMiddleware', function () { const scaffold = { 'method1': 'foo', - 'method2': (_req, res, _next, end) => { + 'method2': (_req, res, end) => { res.result = 42; end(); }, - 'method3': (_req, res, _next, end) => { + 'method3': (_req, res, end) => { res.error = new Error('method3'); end(); }, }; engine.push(createScaffoldMiddleware(scaffold)); - engine.push((_req, res, _next, end) => { + engine.push((_req, res, end) => { res.result = 'passthrough'; end(); }); diff --git a/test/engine.spec.js b/test/engine.spec.js index 6a50d91..1a6950e 100644 --- a/test/engine.spec.js +++ b/test/engine.spec.js @@ -40,7 +40,7 @@ describe('JsonRpcEngine', function () { it('handle: basic middleware test 1', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (_req, res, _next, end) { + engine.push(function (_req, res, end) { res.result = 42; end(); }); @@ -58,7 +58,7 @@ describe('JsonRpcEngine', function () { it('handle: basic middleware test 2', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (req, res, _next, end) { + engine.push(function (req, res, end) { req.method = 'banana'; res.result = 42; end(); @@ -78,7 +78,7 @@ describe('JsonRpcEngine', function () { it('handle (async): basic middleware test', async function () { const engine = new JsonRpcEngine(); - engine.push(function (_req, res, _next, end) { + engine.push(function (_req, res, end) { res.result = 42; end(); }); @@ -93,7 +93,7 @@ describe('JsonRpcEngine', function () { it('allow null result', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (_req, res, _next, end) { + engine.push(function (_req, res, end) { res.result = null; end(); }); @@ -111,12 +111,11 @@ describe('JsonRpcEngine', function () { it('interacting middleware test', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (req, _res, next, _end) { + engine.push(function (req, _res, _end) { req.resultShouldBe = 42; - next(); }); - engine.push(function (req, res, _next, end) { + engine.push(function (req, res, end) { res.result = req.resultShouldBe; end(); }); @@ -134,12 +133,12 @@ describe('JsonRpcEngine', function () { it('middleware ending request before all middlewares applied', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (_req, res, _next, end) { + engine.push(function (_req, res, end) { res.result = 42; end(); }); - engine.push(function (_req, _res, _next, _end) { + engine.push(function (_req, _res, _end) { assert.fail('should not have called second middleware'); }); @@ -156,7 +155,7 @@ describe('JsonRpcEngine', function () { it('erroring middleware test: end(error)', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (_req, _res, _next, end) { + engine.push(function (_req, _res, end) { end(new Error('no bueno')); }); @@ -171,12 +170,11 @@ describe('JsonRpcEngine', function () { }); }); - it('erroring middleware test: res.error -> next()', function (done) { + it('erroring middleware test: res.error -> return', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (_req, res, next, _end) { + engine.push(function (_req, res, _end) { res.error = new Error('no bueno'); - next(); }); const payload = { id: 1, jsonrpc: '2.0', method: 'hello' }; @@ -193,7 +191,7 @@ describe('JsonRpcEngine', function () { it('erroring middleware test: res.error -> end()', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (_req, res, _next, end) { + engine.push(function (_req, res, end) { res.error = new Error('no bueno'); end(); }); @@ -209,11 +207,11 @@ describe('JsonRpcEngine', function () { }); }); - it('erroring middleware test: non-function passsed to next()', function (done) { + it('erroring middleware test: returning truthy non-function', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (_req, _res, next, _end) { - next(true); + engine.push(function (_req, _res, _end) { + return true; }); const payload = { id: 1, jsonrpc: '2.0', method: 'hello' }; @@ -224,7 +222,7 @@ describe('JsonRpcEngine', function () { assert.ok(res.error, 'should have error on response'); assert.equal(res.error.code, -32603, 'should have expected error'); assert.ok( - res.error.message.startsWith('JsonRpcEngine: "next" return handlers must be functions.'), + res.error.message.startsWith('JsonRpcEngine: return handlers must be functions.'), 'should have expected error', ); assert.ok(!res.result, 'should not have result on response'); @@ -246,7 +244,7 @@ describe('JsonRpcEngine', function () { it('handle: batch payloads', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (req, res, _next, end) { + engine.push(function (req, res, end) { if (req.id === 4) { delete res.result; res.error = new Error('foobar'); @@ -280,7 +278,7 @@ describe('JsonRpcEngine', function () { it('handle: batch payloads (async signature)', async function () { const engine = new JsonRpcEngine(); - engine.push(function (req, res, _next, end) { + engine.push(function (req, res, end) { if (req.id === 4) { delete res.result; res.error = new Error('foobar'); @@ -311,7 +309,7 @@ describe('JsonRpcEngine', function () { it('handle: batch payload with bad request object', async function () { const engine = new JsonRpcEngine(); - engine.push(function (req, res, _next, end) { + engine.push(function (req, res, end) { res.result = req.id; return end(); }); @@ -345,25 +343,25 @@ describe('JsonRpcEngine', function () { it('return handlers test', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (_req, res, next, _end) { - next(function () { + engine.push(function (_req, res, _end) { + return () => { res.sawReturnHandler.push(3); - }); + }; }); - engine.push(function (_req, res, next, _end) { - next(async function () { + engine.push(function (_req, res, _end) { + return () => { res.sawReturnHandler.push(2); - }); + }; }); - engine.push(function (_req, res, next, _end) { - next(function () { + engine.push(function (_req, res, _end) { + return () => { res.sawReturnHandler = [1]; - }); + }; }); - engine.push(function (_req, res, _next, end) { + engine.push(function (_req, res, end) { res.result = true; end(); }); @@ -387,23 +385,23 @@ describe('JsonRpcEngine', function () { const events = []; - engine.push(function (_req, _res, next, _end) { - events.push('1-next'); - next(function () { + engine.push(function (_req, _res, _end) { + events.push('1-middleware'); + return () => { events.push('1-return'); - }); + }; }); // Async middleware function - engine.push(async function (_req, _res, next, _end) { - events.push('2-next'); + engine.push(async function (_req, _res, _end) { + events.push('2-middleware'); await delay(); - next(function () { + return () => { events.push('2-return'); - }); + }; }); - engine.push(function (_req, res, _next, end) { + engine.push(function (_req, res, end) { events.push('3-end'); res.result = true; end(); @@ -413,8 +411,8 @@ describe('JsonRpcEngine', function () { engine.handle(payload, function (err, _res) { assert.ifError(err, 'did not error'); - assert.equal(events[0], '1-next', '(event 0) was "1-next"'); - assert.equal(events[1], '2-next', '(event 1) was "2-next"'); + assert.equal(events[0], '1-middleware', '(event 0) was "1-middleware"'); + assert.equal(events[1], '2-middleware', '(event 1) was "2-middleware"'); assert.equal(events[2], '3-end', '(event 2) was "3-end"'); assert.equal(events[3], '2-return', '(event 3) was "2-return"'); assert.equal(events[4], '1-return', '(event 4) was "1-return"'); @@ -422,18 +420,18 @@ describe('JsonRpcEngine', function () { }); }); - it('calls back next handler even if error', function (done) { + it('calls back return handler even if error', function (done) { const engine = new JsonRpcEngine(); - let sawNextReturnHandlerCalled = false; + let sawReturnHandlerCalled = false; - engine.push(function (_req, _res, next, _end) { - next(function () { - sawNextReturnHandlerCalled = true; - }); + engine.push(function (_req, _res, _end) { + return () => { + sawReturnHandlerCalled = true; + }; }); - engine.push(function (_req, _res, _next, end) { + engine.push(function (_req, _res, end) { end(new Error('boom')); }); @@ -441,23 +439,23 @@ describe('JsonRpcEngine', function () { engine.handle(payload, (err, _res) => { assert.ok(err, 'did error'); - assert.ok(sawNextReturnHandlerCalled, 'saw next return handler called'); + assert.ok(sawReturnHandlerCalled, 'saw return handler called'); done(); }); }); - it('calls back next handler even if async middleware rejects', function (done) { + it('calls back return handler even if async middleware rejects', function (done) { const engine = new JsonRpcEngine(); - let sawNextReturnHandlerCalled = false; + let sawReturnHandlerCalled = false; - engine.push(function (_req, _res, next, _end) { - next(function () { - sawNextReturnHandlerCalled = true; - }); + engine.push(function (_req, _res, _end) { + return () => { + sawReturnHandlerCalled = true; + }; }); - engine.push(async function (_req, _res, _next, _end) { + engine.push(async function (_req, _res, _end) { throw new Error('boom'); }); @@ -465,21 +463,21 @@ describe('JsonRpcEngine', function () { engine.handle(payload, (err, _res) => { assert.ok(err, 'did error'); - assert.ok(sawNextReturnHandlerCalled, 'saw next return handler called'); + assert.ok(sawReturnHandlerCalled, 'saw return handler called'); done(); }); }); - it('handles error in next handler', function (done) { + it('handles error in return handler', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (_req, _res, next, _end) { - next(function () { + engine.push(function (_req, _res, _end) { + return () => { throw new Error('foo'); - }); + }; }); - engine.push(function (_req, res, _next, end) { + engine.push(function (_req, res, end) { res.result = 42; end(); }); @@ -493,16 +491,16 @@ describe('JsonRpcEngine', function () { }); }); - it('handles error in async next handler', function (done) { + it('handles error in async return handler', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (_req, _res, next, _end) { - next(async function () { + engine.push(function (_req, _res, _end) { + return () => { throw new Error('foo'); - }); + }; }); - engine.push(function (_req, res, _next, end) { + engine.push(function (_req, res, end) { res.result = 42; end(); }); @@ -519,9 +517,8 @@ describe('JsonRpcEngine', function () { it('handles failure to end request', function (done) { const engine = new JsonRpcEngine(); - engine.push(function (_req, res, next, _end) { + engine.push(function (_req, res, _end) { res.result = 42; - next(); }); const payload = { id: 1, jsonrpc: '2.0', method: 'hello' }; diff --git a/test/idRemapMiddleware.spec.js b/test/idRemapMiddleware.spec.js index 566c97e..62c5744 100644 --- a/test/idRemapMiddleware.spec.js +++ b/test/idRemapMiddleware.spec.js @@ -13,13 +13,14 @@ describe('idRemapMiddleware', function () { after: {}, }; - engine.push(function (req, res, next, _end) { + engine.push(function (req, res, _end) { observedIds.before.req = req.id; observedIds.before.res = res.id; - next(); }); + engine.push(createIdRemapMiddleware()); - engine.push(function (req, res, _next, end) { + + engine.push(function (req, res, end) { observedIds.after.req = req.id; observedIds.after.res = res.id; // set result so it doesnt error diff --git a/test/mergeMiddleware.spec.js b/test/mergeMiddleware.spec.js index f5f8ecf..2d8b066 100644 --- a/test/mergeMiddleware.spec.js +++ b/test/mergeMiddleware.spec.js @@ -10,7 +10,7 @@ describe('mergeMiddleware', function () { let originalReq; engine.push(mergeMiddleware([ - function (req, res, _next, end) { + function (req, res, end) { originalReq = req; res.result = 'saw merged middleware'; end(); @@ -29,16 +29,16 @@ describe('mergeMiddleware', function () { }); }); - it('handles next handler correctly for multiple merged', function (done) { + it('handles return handler correctly for multiple merged', function (done) { const engine = new JsonRpcEngine(); engine.push(mergeMiddleware([ - (_req, res, next, _end) => { - next(() => { + (_req, res, _end) => { + return () => { res.copy = res.result; - }); + }; }, - (_req, res, _next, end) => { + (_req, res, end) => { res.result = true; end(); }, @@ -59,7 +59,7 @@ describe('mergeMiddleware', function () { let originalReq; engine.push(mergeMiddleware([ - function (req, res, _next, end) { + function (req, res, end) { originalReq = req; res.xyz = true; res.result = true; @@ -84,7 +84,7 @@ describe('mergeMiddleware', function () { let originalReq; engine.push(mergeMiddleware([ - function (req, res, _next, end) { + function (req, res, end) { originalReq = req; req.xyz = true; res.result = true; @@ -108,9 +108,9 @@ describe('mergeMiddleware', function () { const engine = new JsonRpcEngine(); engine.push(mergeMiddleware([ - (_req, _res, next, _end) => next(), + (_req, _res, _end) => undefined, ])); - engine.push((_req, res, _next, end) => { + engine.push((_req, res, end) => { res.result = true; end(); }); @@ -124,17 +124,17 @@ describe('mergeMiddleware', function () { }); }); - it('handles next handler correctly across middleware', function (done) { + it('handles return handler correctly across middleware', function (done) { const engine = new JsonRpcEngine(); engine.push(mergeMiddleware([ - (_req, res, next, _end) => { - next(() => { + (_req, res, _end) => { + return () => { res.copy = res.result; - }); + }; }, ])); - engine.push((_req, res, _next, end) => { + engine.push((_req, res, end) => { res.result = true; end(); }); From 0be55c48960b1932582c6f5769321e7ac8f472d3 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Sun, 28 Mar 2021 13:41:57 -0700 Subject: [PATCH 2/4] Update readme --- README.md | 100 ++++++++++++++++++++++++++---------------------------- 1 file changed, 48 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index dca0dec..86bbdbc 100644 --- a/README.md +++ b/README.md @@ -5,73 +5,76 @@ A tool for processing JSON-RPC requests and responses. ## Usage ```js -const { JsonRpcEngine } = require('json-rpc-engine') +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. ```js -engine.push(function(req, res, next, end){ - res.result = 42 - end() -}) +engine.push(function (req, res, end) { + res.result = 42; + 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){ +engine.handle(request, (err, response) => { // Do something with response.result, or handle response.error -}) +}); // There is also a Promise signature -const response = await engine.handle(request) +const response = await engine.handle(request); ``` Middleware have direct access to the request and response objects. -They can let processing continue down the stack with `next()`, or complete the request with `end()`. +They can let processing continue down the stack by returning, or complete the request with `end()`. ```js -engine.push(function(req, res, next, end){ - if (req.skipCache) return next() - res.result = getResultFromCache(req) - return end() -}) +engine.push(function (req, res, end) { + if (req.skipCache) return; + res.result = getResultFromCache(req); + return end(); +}); ``` 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() -}) +engine.push(async function (req, res, end) { + if (req.method !== targetMethod) return; + 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. +By returning a _return handler function_, middleware can interact with the response before it is returned to the requester. ```js -engine.push((req, res, next, end) => { - next(() => { - await insertIntoCache(res) - }) +engine.push((req, res, end) => { + return () => { + await insertIntoCache(res); + } }) ``` Return handlers can be synchronous or asynchronous. They take no callbacks, and should only interact with the request and/or the response. +Middleware functions **must** return a falsy value or a function. +If anything else is returned, the request will end with an error. + Engines can be nested by converting them to middleware using `JsonRpcEngine.asMiddleware()`: ```js -const engine = new JsonRpcEngine() -const subengine = new JsonRpcEngine() -engine.push(subengine.asMiddleware()) +const engine = new JsonRpcEngine(); +const subengine = new JsonRpcEngine(); +engine.push(subengine.asMiddleware()); ``` ### Error Handling @@ -79,8 +82,7 @@ engine.push(subengine.asMiddleware()) Errors should be handled by throwing inside middleware functions. 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. +or set the error on the response object, and then return or call `end`. Errors always take precedent over results. If an error is detected, the response's `result` property will be deleted. @@ -90,27 +92,21 @@ It does not matter of the middleware function is synchronous or asynchronous. ```js // Throwing is preferred. -engine.push(function(req, res, next, end){ - throw new Error() -}) +engine.push(function (req, res, end) { + throw new Error(); +}); // For backwards compatibility, you can also do this: -engine.push(function(req, res, next, end){ - end(new Error()) -}) - -engine.push(function(req, res, next, end){ - res.error = new Error() - end() -}) - -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()) -}) +engine.push(function (req, res, end) { + end(new Error()); +}); + +engine.push(function (req, res, end) { + res.error = new Error(); + end(); +}); + +engine.push(function (req, res, end) { + res.error = new Error(); +}); ``` From 0cecd95ee82a16f5b0f291eb1ad962e06e85ae97 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Sun, 28 Mar 2021 14:00:02 -0700 Subject: [PATCH 3/4] Ignore middleware return value if request is ended --- README.md | 2 ++ src/JsonRpcEngine.ts | 44 +++++++++++++++++++++++++------------------- test/engine.spec.js | 22 ++++++++++++++++++++++ 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 86bbdbc..b51c380 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,8 @@ They take no callbacks, and should only interact with the request and/or the res Middleware functions **must** return a falsy value or a function. If anything else is returned, the request will end with an error. +If a middleware calls `end()`, its return value will be ignored. + Engines can be nested by converting them to middleware using `JsonRpcEngine.asMiddleware()`: ```js diff --git a/src/JsonRpcEngine.ts b/src/JsonRpcEngine.ts index 2bd7178..b41c483 100644 --- a/src/JsonRpcEngine.ts +++ b/src/JsonRpcEngine.ts @@ -389,39 +389,45 @@ export class JsonRpcEngine extends SafeEventEmitter { resolve, ] = getDeferredPromise<[unknown, boolean]>(); + let ended = false; const end: JsonRpcEngineEndCallback = (err?: unknown) => { const error = err || res.error; if (error) { res.error = serializeError(error); } + // True indicates that the request should end + ended = true; resolve([error, true]); }; try { const returnHandler = await middleware(req, res, end); - if (res.error) { - end(res.error); - } else { - if (returnHandler) { - if (typeof returnHandler !== 'function') { - end( - new EthereumRpcError( - errorCodes.rpc.internal, - `JsonRpcEngine: return handlers must be functions. ` + - `Received "${typeof returnHandler}" for request:\n${jsonify( - req, - )}`, - { request: req }, - ), - ); + // If the request is already ended, there's nothing to do. + if (!ended) { + if (res.error) { + end(res.error); + } else { + if (returnHandler) { + if (typeof returnHandler !== 'function') { + end( + new EthereumRpcError( + errorCodes.rpc.internal, + `JsonRpcEngine: return handlers must be functions. ` + + `Received "${typeof returnHandler}" for request:\n${jsonify( + req, + )}`, + { request: req }, + ), + ); + } + returnHandlers.push(returnHandler); } - returnHandlers.push(returnHandler); - } - // False indicates that the request should not end - resolve([null, false]); + // False indicates that the request should not end + resolve([null, false]); + } } } catch (error) { end(error); diff --git a/test/engine.spec.js b/test/engine.spec.js index 1a6950e..a49778c 100644 --- a/test/engine.spec.js +++ b/test/engine.spec.js @@ -444,6 +444,28 @@ describe('JsonRpcEngine', function () { }); }); + it('ignores return handler if ending request first', function (done) { + const engine = new JsonRpcEngine(); + + let sawReturnHandlerCalled = false; + + engine.push(function (_req, res, end) { + res.result = 'foo'; + end(); + return () => { + sawReturnHandlerCalled = true; + }; + }); + + const payload = { id: 1, jsonrpc: '2.0', method: 'hello' }; + + engine.handle(payload, (_err, res) => { + assert.strictEqual(res.result, 'foo', 'has correct result'); + assert.ok(!sawReturnHandlerCalled, 'should not have called return handler'); + done(); + }); + }); + it('calls back return handler even if async middleware rejects', function (done) { const engine = new JsonRpcEngine(); From bb91bf3b664c3273b2afadaaf678e1f46060cfc5 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Wed, 23 Jun 2021 23:33:33 -0700 Subject: [PATCH 4/4] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b51c380..e76ba63 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,7 @@ By returning a _return handler function_, middleware can interact with the respo ```js engine.push((req, res, end) => { - return () => { + return async () => { await insertIntoCache(res); } })