diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f47ab111..a6b47e5ff 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,6 +31,7 @@ jobs: - 14.x - 16.x - 18.x + - 20.x runs-on: ${{matrix.os}} steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index b7df7f4c8..020fa4e03 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -2,6 +2,7 @@ on: push: branches: - master + - 9.x name: release-please jobs: release-please: diff --git a/CHANGELOG.md b/CHANGELOG.md index d6a46239e..5f134e57b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -121,6 +121,19 @@ * remove deprecated usage of pino.child ([#1902](https://www.github.com/restify/node-restify/issues/1902)) ([0a8cf83](https://www.github.com/restify/node-restify/commit/0a8cf8345de26f8ee98e87c0085f0f9439302d98)) * **travisci:** revisit nodejs version. Change to: LTS active, LTS maintenance (4.x) and stable releases ([#1553](https://www.github.com/restify/node-restify/issues/1553)) ([49eb008](https://www.github.com/restify/node-restify/commit/49eb008d987f1c425989b78e2336e3583e05a88a)) +## [11.2.0](https://github.com/restify/node-restify/compare/v11.1.0...v11.2.0) (2023-08-11) + + +### Features + +* allow alternate name for request id in logs ([cbd16ef](https://github.com/restify/node-restify/commit/cbd16efa3be36e7888ecccc15ee28eaa8fa6c5ef)) +* support Node.js 20 ([9f1d249](https://github.com/restify/node-restify/commit/9f1d249c3fd023b05ac15c02352ec937ff7d1299)) + + +### Bug Fixes + +* tests broke due to find-my-way update ([f8beaae](https://github.com/restify/node-restify/commit/f8beaaef64c0541185bc4c2d864948d3c1299cc9)) + ## [11.1.0](https://github.com/restify/node-restify/compare/v11.0.0...v11.1.0) (2023-02-24) diff --git a/lib/plugins/audit.js b/lib/plugins/audit.js index c0c0b8879..f335af47e 100644 --- a/lib/plugins/audit.js +++ b/lib/plugins/audit.js @@ -52,6 +52,8 @@ function getResponseHeaders(res) { * via the logger. * @param {Object} [opts.serializers] - Override the default logger serializers * for err, req and res + * @param {String} [opts.requestIdFieldName] - The name of the request id property attached + * to log lines. Defaults to "req_id". * @returns {Function} Handler * @fires audit when an audit log has been generated * @example @@ -195,6 +197,7 @@ function auditLogger(opts) { var server = opts.server; var printLog = opts.printLog; + const requestIdFieldName = opts.requestIdFieldName || 'req_id'; if (typeof printLog === 'undefined') { printLog = true; @@ -270,7 +273,7 @@ function auditLogger(opts) { var obj = { remoteAddress: req.connection.remoteAddress, remotePort: req.connection.remotePort, - req_id: req.getId(), + [requestIdFieldName]: req.getId(), req: req, res: res, err: err, diff --git a/lib/plugins/requestLogger.js b/lib/plugins/requestLogger.js index 8b5a37fd8..50012ff0d 100644 --- a/lib/plugins/requestLogger.js +++ b/lib/plugins/requestLogger.js @@ -25,6 +25,11 @@ var shallowCopy = require('./utils/shallowCopy'); * @param {Object} [options] - an options object * @param {Array} [options.headers] - A list of headers to transfer from * the request to top level props on the log. + * @param {Object} [options.properties] - A set of key-values to pass to the child logger + * @param {Object} [options.serializers] - Override serializers to use in the child logger + * @param {Object} [options.log] - A logger to use as a fallback if req.log is missing + * @param {String} [options.requestIdFieldName] - The name of the request id property attached + * to log lines. Defaults to "req_id". * @returns {Function} Handler * @example * server.use(restify.plugins.requestLogger({ @@ -51,6 +56,7 @@ function requestLogger(options) { } var headersToCopy = opts.headers || []; + const requestIdFieldName = opts.requestIdFieldName || 'req_id'; return function logger(req, res, next) { if (!req.log && !opts.log) { @@ -60,7 +66,8 @@ function requestLogger(options) { var log = req.log || opts.log; - props.req_id = req.getId(); + props[requestIdFieldName] = req.getId(); + headersToCopy.forEach(function forEach(k) { if (req.headers[k]) { props[k] = req.headers[k]; @@ -72,8 +79,8 @@ function requestLogger(options) { } req.log = log.child(props, childOptions); - if (props.req_id) { - delete props.req_id; + if (props[requestIdFieldName]) { + delete props[requestIdFieldName]; } next(); diff --git a/lib/server.js b/lib/server.js index a5865323d..53600e23d 100644 --- a/lib/server.js +++ b/lib/server.js @@ -11,7 +11,6 @@ var _ = require('lodash'); var assert = require('assert-plus'); var errors = require('restify-errors'); var mime = require('mime'); -var spdy = require('spdy'); var vasync = require('vasync'); var Chain = require('./chain'); @@ -28,6 +27,7 @@ var patchResponse = require('./response'); var domain; var http2; +var spdy; patchResponse(http.ServerResponse); patchRequest(http.IncomingMessage); @@ -187,6 +187,7 @@ function Server(options) { ]; if (options.spdy) { + spdy = require('spdy'); this.spdy = true; this.server = spdy.createServer(options.spdy); } else if (options.http2) { diff --git a/package.json b/package.json index 9b286180e..6b07f155e 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,7 @@ "express", "DTrace" ], - "version": "11.1.0", + "version": "11.2.0", "repository": { "type": "git", "url": "git://github.com/restify/node-restify.git" @@ -97,7 +97,7 @@ "csv": "^6.2.2", "escape-regexp-component": "^1.0.2", "ewma": "^2.0.1", - "find-my-way": "^7.2.0", + "find-my-way": "^7.6.0", "formidable": "^1.2.1", "http-signature": "^1.3.6", "lodash": "^4.17.11", diff --git a/test/plugins/audit.test.js b/test/plugins/audit.test.js index a8499e460..31514ac5f 100644 --- a/test/plugins/audit.test.js +++ b/test/plugins/audit.test.js @@ -625,4 +625,38 @@ describe('audit logger', function() { function(err, req, res) {} ); }); + + it('should set request id using supplied field name', function(done) { + SERVER.once( + 'after', + restify.plugins.auditLogger({ + log: pino({ name: 'audit' }), + server: SERVER, + event: 'after', + requestIdFieldName: 'traceId' + }) + ); + + SERVER.once('audit', function(data) { + assert.ok(data); + assert.ok(data.traceId); + assert.notOk(data.req_id); + done(); + }); + + SERVER.get('/audit', function(req, res, next) { + setTimeout(function() { + res.send(); + next(); + }, 150); + }); + + CLIENT.get( + { + path: '/audit', + requestTimeout: 50 + }, + function(err, req, res) {} + ); + }); }); diff --git a/test/plugins/plugins.test.js b/test/plugins/plugins.test.js index c837c6e4c..83f8258ca 100644 --- a/test/plugins/plugins.test.js +++ b/test/plugins/plugins.test.js @@ -95,6 +95,39 @@ describe('all other plugins', function() { done(); }); }); + + it('adds the request id to logs', function(done) { + SERVER.use(restify.plugins.requestLogger()); + SERVER.get('/requestLogger/test', function(req, res, next) { + var childings = req.log[pino.symbols.chindingsSym]; + assert.match(childings, /"req_id":"[0-9A-F-]+"/i); + res.send(); + next(); + }); + CLIENT.get('/requestLogger/test', function(err, _, res) { + assert.equal(res.statusCode, 200); + assert.ifError(err); + done(); + }); + }); + + it('adds the request id with a custom field name', function(done) { + SERVER.use( + restify.plugins.requestLogger({ requestIdFieldName: 'traceId' }) + ); + SERVER.get('/requestLogger/test', function(req, res, next) { + var childings = req.log[pino.symbols.chindingsSym]; + assert.match(childings, /"traceId":"[0-9A-F-]+"/i); + assert.notMatch(childings, /"req_id"/); + res.send(); + next(); + }); + CLIENT.get('/requestLogger/test', function(err, _, res) { + assert.equal(res.statusCode, 200); + assert.ifError(err); + done(); + }); + }); }); describe('full response', function() { diff --git a/test/router.test.js b/test/router.test.js index 769873c61..d6277cd63 100644 --- a/test/router.test.js +++ b/test/router.test.js @@ -343,8 +343,7 @@ test('toString()', function(t) { t.deepEqual( router.toString(), // prettier-ignore - '└── / (GET)\n' + - ' / (POST)\n' + + '└── / (GET, POST)\n' + ' └── a (GET)\n' + ' └── /b (GET)\n' ); @@ -368,8 +367,7 @@ test('toString() with ignoreTrailingSlash', function(t) { t.deepEqual( router.toString(), // prettier-ignore - '└── / (GET)\n' + - ' / (POST)\n' + + '└── / (GET, POST)\n' + ' └── a (GET)\n' + ' └── /b (GET)\n' ); diff --git a/test/routerRegistryRadix.test.js b/test/routerRegistryRadix.test.js index 16b264d76..5027b05e6 100644 --- a/test/routerRegistryRadix.test.js +++ b/test/routerRegistryRadix.test.js @@ -96,8 +96,7 @@ test('toString()', function(t) { t.deepEqual( registry.toString(), // prettier-ignore - '└── / (GET)\n' + - ' / (POST)\n' + + '└── / (GET, POST)\n' + ' └── a (GET)\n' + ' └── /b (GET)\n' ); @@ -114,8 +113,7 @@ test('toString() with ignoreTrailingSlash', function(t) { t.deepEqual( registry.toString(), // prettier-ignore - '└── / (GET)\n' + - ' / (POST)\n' + + '└── / (GET, POST)\n' + ' └── a (GET)\n' + ' └── /b (GET)\n' ); diff --git a/test/server.test.js b/test/server.test.js index 5fd108061..c1d2cbc89 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -1094,7 +1094,9 @@ test('fire event on error', function(t) { }); }); -test('error handler defers "after" event', function(t) { +test('error handler defers "after" event', async function(t) { + let afterResolve; + let clientResolve; t.expect(9); SERVER.once('NotFound', function(req, res, err, cb) { t.ok(req); @@ -1107,7 +1109,7 @@ test('error handler defers "after" event', function(t) { SERVER.once('after', function(req2, res2) { t.ok(req2); t.ok(res2); - t.end(); + afterResolve(); }); return cb(); }); @@ -1118,8 +1120,18 @@ test('error handler defers "after" event', function(t) { CLIENT.get('/' + uuid.v4(), function(err, _, res) { t.ok(err); t.equal(res.statusCode, 404); - t.end(); + clientResolve(); }); + + await Promise.all([ + new Promise(resolve => { + afterResolve = resolve; + }), + new Promise(resolve => { + clientResolve = resolve; + }) + ]); + t.end(); }); // eslint-disable-next-line @@ -1916,7 +1928,9 @@ test("should emit 'close' on server close", function(t) { }); }); -test('should cleanup inflight requests count for 404s', function(t) { +test('should cleanup inflight requests count for 404s', async function(t) { + let afterResolve; + let clientResolve; SERVER.get('/foo1', function(req, res, next) { t.equal(SERVER.inflightRequests(), 1); res.send(); @@ -1926,7 +1940,7 @@ test('should cleanup inflight requests count for 404s', function(t) { SERVER.on('after', function(req) { if (req.path() === '/doesnotexist') { t.equal(SERVER.inflightRequests(), 0); - t.end(); + afterResolve(); } }); @@ -1939,8 +1953,19 @@ test('should cleanup inflight requests count for 404s', function(t) { t.ok(err2); t.equal(res2.statusCode, 404); t.equal(SERVER.inflightRequests(), 0); + clientResolve(); }); }); + + await Promise.all([ + new Promise(resolve => { + afterResolve = resolve; + }), + new Promise(resolve => { + clientResolve = resolve; + }) + ]); + t.end(); }); test('should cleanup inflight requests count for timeouts', function(t) {