From 765c6b9cc17dcd4898012963bbb828a9610ca5e4 Mon Sep 17 00:00:00 2001 From: James Sumners Date: Sat, 2 Dec 2017 17:56:18 -0500 Subject: [PATCH 1/5] Fix #42 --- logger.js | 14 +++++++++++++- test.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/logger.js b/logger.js index 3575482..07a8d1d 100644 --- a/logger.js +++ b/logger.js @@ -12,7 +12,7 @@ function pinoLogger (opts, stream) { opts = opts || {} opts.serializers = opts.serializers || {} - opts.serializers.req = opts.serializers.req || asReqValue + opts.serializers.req = wrapReqSerializer(opts.serializers.req || asReqValue) opts.serializers.res = opts.serializers.res || pino.stdSerializers.res opts.serializers.err = opts.serializers.err || pino.stdSerializers.err @@ -64,6 +64,18 @@ function pinoLogger (opts, stream) { } } +function wrapReqSerializer (serializer) { + if (serializer === asReqValue) return asReqValue + return function wrappedReqSerializer (req) { + var _req = asReqValue(req) + Object.defineProperty(_req, 'raw', { + enumerable: false, + value: req + }) + return serializer(_req) + } +} + function asReqValue (req) { var connection = req.connection return { diff --git a/test.js b/test.js index ab4d16a..0754c8c 100644 --- a/test.js +++ b/test.js @@ -303,3 +303,34 @@ test('does not crash when no request connection object', function (t) { res.end() } }) + +// https://github.com/pinojs/pino-http/issues/42 +test('does not return excessively log object', function (t) { + var dest = split(JSON.parse) + var logger = pinoHttp({ + logger: pino(dest), + serializers: { + req: function (req) { + delete req.connection + return req + } + } + }) + t.plan(1) + + var server = http.createServer(handler) + server.unref() + server.listen(0, () => { + const port = server.address().port + http.get(`http://127.0.0.1:${port}`, () => {}) + }) + + function handler (req, res) { + logger(req, res) + res.end() + } + + dest.on('data', function (obj) { + t.is(Object.keys(obj.req).length, 6) + }) +}) From 626eede60281e460e77968836293d671d709f19a Mon Sep 17 00:00:00 2001 From: James Sumners Date: Sun, 3 Dec 2017 12:35:43 -0500 Subject: [PATCH 2/5] Fix typo in test description --- test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test.js b/test.js index 0754c8c..a3aaa4d 100644 --- a/test.js +++ b/test.js @@ -305,7 +305,7 @@ test('does not crash when no request connection object', function (t) { }) // https://github.com/pinojs/pino-http/issues/42 -test('does not return excessively log object', function (t) { +test('does not return excessively long object', function (t) { var dest = split(JSON.parse) var logger = pinoHttp({ logger: pino(dest), From 5aebe40a3f41e7496cd36afd616776bab011c183 Mon Sep 17 00:00:00 2001 From: James Sumners Date: Sun, 3 Dec 2017 13:42:54 -0500 Subject: [PATCH 3/5] Use a V8 class for the internal request object --- logger.js | 71 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 57 insertions(+), 14 deletions(-) diff --git a/logger.js b/logger.js index 07a8d1d..104c329 100644 --- a/logger.js +++ b/logger.js @@ -64,28 +64,71 @@ function pinoLogger (opts, stream) { } } +var rawSymbol = Symbol.for('pino-raw-request') +var pinoReqProto = Object.create({}, { + id: { + enumerable: true, + writable: true, + value: '' + }, + method: { + enumerable: true, + writable: true, + value: '' + }, + url: { + enumerable: true, + writable: true, + value: '' + }, + headers: { + enumerable: true, + writable: true, + value: {} + }, + remoteAddress: { + enumerable: true, + writable: true, + value: '' + }, + remotePort: { + enumerable: true, + writable: true, + value: '' + }, + raw: { + enumerable: false, + get: function () { + return this[rawSymbol] + }, + set: function (val) { + this[rawSymbol] = val + } + } +}) +Object.defineProperty(pinoReqProto, rawSymbol, { + writable: true, + value: {} +}) + function wrapReqSerializer (serializer) { if (serializer === asReqValue) return asReqValue return function wrappedReqSerializer (req) { - var _req = asReqValue(req) - Object.defineProperty(_req, 'raw', { - enumerable: false, - value: req - }) - return serializer(_req) + return serializer(asReqValue(req)) } } function asReqValue (req) { var connection = req.connection - return { - id: req.id, - method: req.method, - url: req.url, - headers: req.headers, - remoteAddress: connection && connection.remoteAddress, - remotePort: connection && connection.remotePort - } + const _req = Object.create(pinoReqProto) + _req.id = req.id + _req.method = req.method + _req.url = req.url + _req.headers = req.headers + _req.remoteAddress = connection && connection.remoteAddress + _req.remotePort = connection && connection.remotePort + _req.raw = req + return _req } function wrapChild (opts, stream) { From 014c271ac269ae481112a91956de6743dadd853e Mon Sep 17 00:00:00 2001 From: James Sumners Date: Mon, 4 Dec 2017 09:00:12 -0500 Subject: [PATCH 4/5] Add test for req.raw --- test.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test.js b/test.js index a3aaa4d..433612e 100644 --- a/test.js +++ b/test.js @@ -334,3 +334,30 @@ test('does not return excessively long object', function (t) { t.is(Object.keys(obj.req).length, 6) }) }) + +test('req.raw is available to custom serializers', function (t) { + t.plan(2) + var dest = split(JSON.parse) + var logger = pinoHttp({ + logger: pino(dest), + serializers: { + req: function (req) { + t.ok(req.raw) + t.ok(req.raw.connection) + return req + } + } + }) + + var server = http.createServer(handler) + server.unref() + server.listen(0, () => { + const port = server.address().port + http.get(`http://127.0.0.1:${port}`, () => {}) + }) + + function handler (req, res) { + logger(req, res) + res.end() + } +}) From e5cfc82b85e8d491b4dd4342afb2f0d252e0ac7d Mon Sep 17 00:00:00 2001 From: James Sumners Date: Fri, 8 Dec 2017 13:57:57 -0500 Subject: [PATCH 5/5] Add response wrapper to match request wrapper --- logger.js | 47 ++++++++++++++++++++++++++++++++++++++++++++--- test.js | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 3 deletions(-) diff --git a/logger.js b/logger.js index 104c329..c7ddb9a 100644 --- a/logger.js +++ b/logger.js @@ -13,7 +13,7 @@ function pinoLogger (opts, stream) { opts = opts || {} opts.serializers = opts.serializers || {} opts.serializers.req = wrapReqSerializer(opts.serializers.req || asReqValue) - opts.serializers.res = opts.serializers.res || pino.stdSerializers.res + opts.serializers.res = wrapResSerializer(opts.serializers.res || asResValue) opts.serializers.err = opts.serializers.err || pino.stdSerializers.err var useLevel = opts.useLevel || 'info' @@ -64,7 +64,7 @@ function pinoLogger (opts, stream) { } } -var rawSymbol = Symbol.for('pino-raw-request') +var rawSymbol = Symbol.for('pino-raw-ref') var pinoReqProto = Object.create({}, { id: { enumerable: true, @@ -131,6 +131,47 @@ function asReqValue (req) { return _req } +var pinoResProto = Object.create({}, { + statusCode: { + enumerable: true, + writable: true, + value: 0 + }, + header: { + enumerable: true, + writable: true, + value: '' + }, + raw: { + enumerable: false, + get: function () { + return this[rawSymbol] + }, + set: function (val) { + this[rawSymbol] = val + } + } +}) +Object.defineProperty(pinoResProto, rawSymbol, { + writable: true, + value: {} +}) + +function asResValue (res) { + const _res = Object.create(pinoResProto) + _res.statusCode = res.statusCode + _res.header = res._header + _res.raw = res + return _res +} + +function wrapResSerializer (serializer) { + if (serializer === asResValue) return asResValue + return function wrappedResSerializer (res) { + return serializer(asResValue(res)) + } +} + function wrapChild (opts, stream) { var prevLogger = opts.logger var prevGenReqId = opts.genReqId @@ -161,6 +202,6 @@ function reqIdGenFactory (func) { module.exports = pinoLogger module.exports.stdSerializers = { req: asReqValue, - res: pino.stdSerializers.res + res: asResValue } module.exports.startTime = startTime diff --git a/test.js b/test.js index 433612e..855bfe7 100644 --- a/test.js +++ b/test.js @@ -361,3 +361,56 @@ test('req.raw is available to custom serializers', function (t) { res.end() } }) + +test('res.raw is available to custom serializers', function (t) { + t.plan(2) + var dest = split(JSON.parse) + var logger = pinoHttp({ + logger: pino(dest), + serializers: { + res: function (res) { + t.ok(res.raw) + t.ok(res.raw.statusCode) + return res + } + } + }) + + var server = http.createServer(handler) + server.unref() + server.listen(0, () => { + const port = server.address().port + http.get(`http://127.0.0.1:${port}`, () => {}) + }) + + function handler (req, res) { + logger(req, res) + res.end() + } +}) + +test('res.raw is not enumerable', function (t) { + t.plan(1) + var dest = split(JSON.parse) + var logger = pinoHttp({ + logger: pino(dest), + serializers: { + res: function (res) { + t.is(res.propertyIsEnumerable('raw'), false) + return res + } + } + }) + + var server = http.createServer(handler) + server.unref() + server.listen(0, () => { + const port = server.address().port + http.get(`http://127.0.0.1:${port}`, () => {}) + }) + + function handler (req, res) { + logger(req, res) + res.end() + } +})