From 12be9e243a407eaf7a30cbb16e399ee2a46dec93 Mon Sep 17 00:00:00 2001 From: Guilherme Hermeto Date: Sat, 27 Jun 2020 00:08:13 -0700 Subject: [PATCH] feat: async/await support BREAKING CHANGE: adds async/await support to pre, use and handler chains --- .eslintrc.js | 2 +- .travis.yml | 1 - docs/guides/8to9guide.md | 96 ++++++++++++ lib/chain.js | 45 +++++- lib/errorTypes.js | 3 +- lib/server.js | 48 +++++- package.json | 2 +- test/chain.test.js | 147 +++++++++++++++++++ test/lib/helper.js | 8 + test/plugins/inflightRequestThrottle.test.js | 2 +- test/server.test.js | 142 ++++++++++++++++++ 11 files changed, 483 insertions(+), 13 deletions(-) create mode 100644 docs/guides/8to9guide.md diff --git a/.eslintrc.js b/.eslintrc.js index 4b3fe94da..4a95582df 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -127,7 +127,7 @@ if (!process.env.NO_LINT) { // stylistic. if (!process.env.NO_STYLE) { // Global - config.rules['max-len'] = [ERROR, { code: 80 }]; + config.rules['max-len'] = [ERROR, { code: 80, ignoreComments: true }]; // Prettier config.extends.push('prettier'); diff --git a/.travis.yml b/.travis.yml index a71ea2aea..98b7e4ea4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,6 @@ sudo: false language: node_js node_js: - - '8' - '10' - "lts/*" # Active LTS release - "node" # Latest stable release diff --git a/docs/guides/8to9guide.md b/docs/guides/8to9guide.md new file mode 100644 index 000000000..ef8e2505c --- /dev/null +++ b/docs/guides/8to9guide.md @@ -0,0 +1,96 @@ +--- +title: restify 8.x to 9.x migration guide +permalink: /docs/8to9/ +--- + +## Introduction + +restify `9.x` comes with `async/await` support! + +## Breaking Changes + +### Drops support for Node.js `8.x` + +Restify requires Node.js version `>=10.0.0`. + +### Async/await support + +`async/await` basic support for `.pre()`, `.use()` and route handlers. + +#### Example + +```js +const restify = require('restify'); + +const server = restify.createServer({}); + +server.use(async (req, res) => { + req.something = await doSomethingAsync(); +}); + +server.get('/params', async (req, res) => { + const value = await asyncOperation(req.something); + res.send(value); +}); +``` + +#### Middleware API (`.pre()` and `.use()`) + +```js +server.use(async (req, res) => { + req.something = await doSomethingAsync(); +}); +``` +- `fn.length === 2` (arity 2); +- `fn instanceof AsyncFunction`; +- if the async function resolves, it calls `next()`; +- any value returned by the async function will be discarded; +- if it rejects with an `Error` instance it calls `next(err)`; +- if it rejects with anything else it wraps in a `AsyncError` and calls `next(err)`; + +#### Route handler API + +```js +server.get('/something', async (req, res) => { + const someData = await fetchSomeDataAsync(); + res.send({ data: someData }); +}); +``` +- `fn.length === 2` (arity 2); +- `fn instanceof AsyncFunction`; +- if the async function resolves without a value, it calls `next()`; +- if the async function resolves with a string value, it calls `next(string)` (re-routes*); +- if the async function resolves with a value other than string, it calls `next(any)`; +- if it rejects with an `Error` instance it calls `next(err)`; +- if it rejects with anything else it wraps in a `AsyncError` and calls `next(err)` (error-handing**); + +##### (*) Note about re-routing: +The `8.x` API allows re-routing when calling `next()` with a string value. If the string matches a valid route, +it will re-route to the given handler. The same is valid for resolving a async function. If the value returned by +the async function is a string, it will try to re-route to the given handler. + +##### (**) Note about error handling: +Although it is recommended to always reject with an instance of Error, in a async function it is possible to +throw or reject without returning an `Error` instance or even anything at all. In such cases, the value rejected +will be wrapped on a `AsyncError`. + +### Handler arity check +Handlers expecting 2 or fewer parameters added to a `.pre()`, `.use()` or route chain must be async functions, as: + +```js +server.use(async (req, res) => { + req.something = await doSomethingAsync(); +}); +``` + +Handlers expecting more than 2 parameters shouldn't be async functions, as: + +````js +// This middleware will be rejected and restify will throw +server.use(async (req, res, next) => { + doSomethingAsync(function callback(val) { + req.something = val; + next(); + }); +}); +```` diff --git a/lib/chain.js b/lib/chain.js index 9bf718fa7..42f62d551 100644 --- a/lib/chain.js +++ b/lib/chain.js @@ -2,6 +2,7 @@ var assert = require('assert-plus'); var once = require('once'); +var customErrorTypes = require('./errorTypes'); module.exports = Chain; @@ -71,6 +72,15 @@ Chain.prototype.getHandlers = function getHandlers() { * @returns {undefined} no return value */ Chain.prototype.add = function add(handler) { + assert.func(handler); + if (handler.length <= 2) { + // arity <= 2, must be AsyncFunction + assert.equal(handler.constructor.name, 'AsyncFunction'); + } else { + // otherwise shouldn't be AsyncFunction + assert.notEqual(handler.constructor.name, 'AsyncFunction'); + } + // _name is assigned in the server and router handler._name = handler._name || handler.name; @@ -144,7 +154,6 @@ Chain.prototype.run = function run(req, res, done) { */ function call(handler, err, req, res, _next) { var arity = handler.length; - var error = err; var hasError = err === false || Boolean(err); // Meassure handler timings @@ -157,6 +166,32 @@ function call(handler, err, req, res, _next) { _next(nextErr, req, res); } + function resolve(value) { + if (value && req.log) { + // logs resolved value + req.log.debug({ value }, 'Async handler resolved with a value'); + } + + return next(value); + } + + function reject(error) { + if (!(error instanceof Error)) { + error = new customErrorTypes.AsyncError( + { + info: { + cause: error, + handler: handler._name, + method: req.method, + path: req.path ? req.path() : undefined + } + }, + 'Async middleware rejected without an error' + ); + } + return next(error); + } + if (hasError && arity === 4) { // error-handling middleware handler(err, req, res, next); @@ -164,12 +199,14 @@ function call(handler, err, req, res, _next) { } else if (!hasError && arity < 4) { // request-handling middleware process.nextTick(function nextTick() { - handler(req, res, next); + const result = handler(req, res, next); + if (result && typeof result.then === 'function') { + result.then(resolve, reject); + } }); return; } // continue - next(error, req, res); - return; + next(err); } diff --git a/lib/errorTypes.js b/lib/errorTypes.js index 8a07d137b..22b6d4a1e 100644 --- a/lib/errorTypes.js +++ b/lib/errorTypes.js @@ -5,5 +5,6 @@ var errors = require('restify-errors'); // This allows Restify to work with restify-errors v6+ module.exports = { RequestCloseError: errors.makeConstructor('RequestCloseError'), - RouteMissingError: errors.makeConstructor('RouteMissingError') + RouteMissingError: errors.makeConstructor('RouteMissingError'), + AsyncError: errors.makeConstructor('AsyncError') }; diff --git a/lib/server.js b/lib/server.js index 6ac5dab3f..65ab4869e 100644 --- a/lib/server.js +++ b/lib/server.js @@ -367,6 +367,13 @@ Server.prototype.close = function close(callback) { * res.send({ hello: 'world' }); * next(); * }); + * @example + * using with async/await + * server.get('/', function (req, res) { + * await somethingAsync(); + * res.send({ hello: 'world' }); + * next(); + * } */ Server.prototype.get = serverMethodFactory('GET'); @@ -474,9 +481,16 @@ Server.prototype.opts = serverMethodFactory('OPTIONS'); * return next(); * }); * @example + * using with async/await + * server.pre(async function(req, res) { + * await somethingAsync(); + * somethingSync(); + * } + * @example * For example, `pre()` can be used to deduplicate slashes in * URLs * server.pre(restify.pre.dedupeSlashes()); + * @see {@link http://restify.com/docs/plugins-api/#serverpre-plugins|Restify pre() plugins} */ Server.prototype.pre = function pre() { var self = this; @@ -575,6 +589,22 @@ Server.prototype.first = function first() { * * and/or a * variable number of nested arrays of handler functions * @returns {Object} returns self + * @example + * server.use(function(req, res, next) { + * // do something... + * return next(); + * }); + * @example + * using with async/await + * server.use(async function(req, res) { + * await somethingAsync(); + * somethingSync(); + * } + * @example + * For example, `use()` can be used to attach a request logger + * + * server.pre(restify.plugins.requestLogger()); + * @see {@link http://restify.com/docs/plugins-api/#serveruse-plugins|Restify use() plugins} */ Server.prototype.use = function use() { var self = this; @@ -596,12 +626,22 @@ Server.prototype.use = function use() { * new middleware function that only fires if the specified parameter exists * in req.params * - * Exposes an API: - * server.param("user", function (req, res, next) { - * // load the user's information here, always making sure to call next() + * @example + * server.param("user", function (req, res, next) { + * // load the user's information here, always making sure to call next() + * fetchUserInformation(req, function callback(user) { + * req.user = user; + * next(); * }); + * }); + * @example + * using with async/await + * server.param("user", async function(req, res) { + * req.user = await fetchUserInformation(req); + * somethingSync(); + * } * - * @see http://expressjs.com/guide.html#route-param%20pre-conditions + * @see {@link http://expressjs.com/guide.html#route-param%20pre-conditions| Express route param pre-conditions} * @public * @memberof Server * @instance diff --git a/package.json b/package.json index 07f1a9311..e4e1ae20b 100644 --- a/package.json +++ b/package.json @@ -90,7 +90,7 @@ "report-latency": "./bin/report-latency" }, "engines": { - "node": ">=10.21.0" + "node": ">=10.0.0" }, "dependencies": { "assert-plus": "^1.0.0", diff --git a/test/chain.test.js b/test/chain.test.js index a714b228d..77081a896 100644 --- a/test/chain.test.js +++ b/test/chain.test.js @@ -282,3 +282,150 @@ test('getHandlers returns with the array of handlers', function(t) { t.deepEqual(chain.getHandlers(), handlers); t.end(); }); + +test('waits async handlers', function(t) { + const chain = new Chain(); + let counter = 0; + + chain.add(async function(req, res) { + await helper.sleep(50); + counter++; + }); + chain.add(function(req, res, next) { + counter++; + next(); + }); + chain.run( + { + startHandlerTimer: function() {}, + endHandlerTimer: function() {}, + closed: function() { + return false; + } + }, + {}, + function() { + t.equal(counter, 2); + t.done(); + } + ); +}); + +test('abort with rejected promise', function(t) { + const myError = new Error('Foo'); + const chain = new Chain(); + let counter = 0; + + chain.add(async function(req, res) { + counter++; + await helper.sleep(10); + return Promise.reject(myError); + }); + chain.add(function(req, res, next) { + counter++; + next(); + }); + chain.run( + { + startHandlerTimer: function() {}, + endHandlerTimer: function() {}, + closed: function() { + return false; + } + }, + {}, + function(err) { + t.deepEqual(err, myError); + t.equal(counter, 1); + t.done(); + } + ); +}); + +test('abort with rejected promise without error', function(t) { + const chain = new Chain(); + let counter = 0; + + chain.add(async function(req, res) { + counter++; + await helper.sleep(10); + return Promise.reject(); + }); + chain.add(function(req, res, next) { + counter++; + next(); + }); + chain.run( + { + startHandlerTimer: function() {}, + endHandlerTimer: function() {}, + closed: function() { + return false; + }, + path: function() { + return '/'; + } + }, + {}, + function(err) { + t.ok(typeof err === 'object'); + t.equal(err.name, 'AsyncError'); + t.equal(err.jse_info.cause, undefined); + t.equal(counter, 1); + t.done(); + } + ); +}); + +test('abort with throw inside async function', function(t) { + const myError = new Error('Foo'); + const chain = new Chain(); + let counter = 0; + + chain.add(async function(req, res) { + counter++; + await helper.sleep(10); + throw myError; + }); + chain.add(function(req, res, next) { + counter++; + next(); + }); + chain.run( + { + startHandlerTimer: function() {}, + endHandlerTimer: function() {}, + closed: function() { + return false; + } + }, + {}, + function(err) { + t.deepEqual(err, myError); + t.equal(counter, 1); + t.done(); + } + ); +}); + +test('fails to add non async function with arity 2', function(t) { + var chain = new Chain(); + + t.throws(function() { + chain.add(function(req, res) { + res.send('ok'); + }); + }, Error); + t.end(); +}); + +test('fails to add async function with arity 3', function(t) { + var chain = new Chain(); + + t.throws(function() { + chain.add(async function(req, res, next) { + res.send('ok'); + }); + }, Error); + t.end(); +}); diff --git a/test/lib/helper.js b/test/lib/helper.js index 92fc2473d..53f6adb5f 100644 --- a/test/lib/helper.js +++ b/test/lib/helper.js @@ -85,5 +85,13 @@ module.exports = { get dtrace() { return true; + }, + + sleep: function sleep(timeInMs) { + return new Promise(function sleepPromise(resolve) { + setTimeout(function timeout() { + resolve(); + }, timeInMs); + }); } }; diff --git a/test/plugins/inflightRequestThrottle.test.js b/test/plugins/inflightRequestThrottle.test.js index d3b363f2a..dcc15c895 100644 --- a/test/plugins/inflightRequestThrottle.test.js +++ b/test/plugins/inflightRequestThrottle.test.js @@ -90,7 +90,7 @@ describe('inlfightRequestThrottle', function() { var opts = { server: server, limit: 1, err: err }; server.pre(inflightRequestThrottle(opts)); var RES; - server.get('/foo', function(req, res) { + server.get('/foo', function(req, res, next) { if (RES) { res.send(999); } else { diff --git a/test/server.test.js b/test/server.test.js index ddd313a95..c2bc3decc 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -2926,3 +2926,145 @@ test('inflightRequest accounting stable with firstChain', function(t) { CLIENT.get('/foobar', getDone); CLIENT.get('/foobar', getDone); }); + +test('async prerouting chain with error', function(t) { + SERVER.pre(async function(req, res) { + await helper.sleep(10); + throw new RestError({ statusCode: 400, restCode: 'BadRequest' }, 'bum'); + }); + + SERVER.get('/hello/:name', function tester(req, res, next) { + res.send(req.params.name); + next(); + }); + + CLIENT.get('/hello/mark', function(err, _, res) { + t.ok(err); + t.equal(res.statusCode, 400); + t.end(); + }); +}); + +test('async prerouting chain with empty rejection', function(t) { + SERVER.pre(async function(req, res) { + await helper.sleep(10); + return Promise.reject(); + }); + + SERVER.get('/hello/:name', function tester(req, res, next) { + res.send(req.params.name); + next(); + }); + + SERVER.on('Async', function(req, res, err, callback) { + t.equal(err.jse_info.cause, undefined); + t.equal(err.jse_info.method, 'GET'); + t.equal(err.jse_info.path, '/hello/mark'); + callback(); + }); + + CLIENT.get('/hello/mark', function(err, _, res) { + t.ok(err); + t.equal(res.statusCode, 500); + t.end(); + }); +}); + +test('async use chain with error', function(t) { + SERVER.use(async function(req, res) { + await helper.sleep(10); + throw new RestError({ statusCode: 400, restCode: 'BadRequest' }, 'bum'); + }); + + SERVER.get('/hello/:name', function tester(req, res, next) { + res.send(req.params.name); + next(); + }); + + CLIENT.get('/hello/mark', function(err, _, res) { + t.ok(err); + t.equal(res.statusCode, 400); + t.end(); + }); +}); + +test('async handler with error', function(t) { + SERVER.get('/hello/:name', async function tester(req, res) { + await helper.sleep(10); + throw new RestError({ statusCode: 400, restCode: 'BadRequest' }, 'bum'); + }); + + CLIENT.get('/hello/mark', function(err, _, res) { + t.ok(err); + t.equal(res.statusCode, 400); + t.end(); + }); +}); + +test('async handler with error after send succeeds', function(t) { + SERVER.get('/hello/:name', async function tester(req, res) { + await helper.sleep(10); + res.send(req.params.name); + throw new RestError({ statusCode: 400, restCode: 'BadRequest' }, 'bum'); + }); + + CLIENT.get('/hello/mark', function(err, _, res) { + t.ok(!err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + +test('async handler with error after send succeeds', function(t) { + SERVER.get('/hello/:name', async function tester(req, res) { + res.send(req.params.name); + await helper.sleep(20); + throw new RestError({ statusCode: 400, restCode: 'BadRequest' }, 'bum'); + }); + + SERVER.on('after', function(req, res, route, error) { + t.ok(error); + t.end(); + }); + + CLIENT.get('/hello/mark', function(err, _, res) { + t.ok(!err); + t.equal(res.statusCode, 200); + }); +}); + +test('async handler without next', function(t) { + SERVER.get('/hello/:name', async function tester(req, res) { + await helper.sleep(10); + res.send(req.params.name); + }); + + SERVER.on('after', function(req, res, route, error) { + t.ok(!error); + t.equal(res.statusCode, 200); + t.end(); + }); + + CLIENT.get('/hello/mark', function(err, _, res) { + t.ok(!err); + t.equal(res.statusCode, 200); + }); +}); + +test('async handler resolved with string should re-route', function(t) { + SERVER.get('/hello/:name', async function tester(req, res) { + await helper.sleep(10); + return 'getredirected'; + }); + + SERVER.get('/redirected', async function tester(req, res) { + res.send(req.params.name); + }); + + CLIENT.get('/hello/mark', function(err, _, res) { + t.ok(!err); + t.equal(res.statusCode, 200); + t.equal(res.body, '"mark"'); + t.end(); + }); +});