From dca6cbda41da3d79f74459f776ed9bd08987f859 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Mon, 30 Nov 2020 17:51:03 +0100 Subject: [PATCH 1/3] Reduse overhead of monitor pipeline --- lib/monitor.js | 29 ++++++++++++++++++++++------- package.json | 3 +-- test/monitor.js | 14 +++++++++++--- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/lib/monitor.js b/lib/monitor.js index ab16cbd..c815655 100755 --- a/lib/monitor.js +++ b/lib/monitor.js @@ -1,10 +1,10 @@ 'use strict'; const Os = require('os'); +const { pipeline } = require('stream'); const Hoek = require('@hapi/hoek'); const Oppsy = require('@hapi/oppsy'); -const Pumpify = require('pumpify'); const Package = require('../package.json'); const Utils = require('./utils'); @@ -119,15 +119,11 @@ module.exports = internals.Monitor = class { streamObjs.push(stream); } - if (streamObjs.length === 1) { - streamObjs.unshift(new Utils.NoOp()); - } - - this._reporters.set(reporterName, Pumpify.obj(streamObjs)).get(reporterName).on('error', (err) => { + this._reporters.set(reporterName, setupPipeline(streamObjs, (err) => { console.error(`There was a problem (${err}) in ${reporterName} and it has been destroyed.`); console.error(err); - }); + })); } this._state.report = true; @@ -208,3 +204,22 @@ module.exports = internals.Monitor = class { } } }; + +const setupPipeline = (streams, onError) => { + + const stream = streams[0]; + + if (streams.length === 1) { + stream.on('error', onError); + } + else { + pipeline(streams, (err) => { + + if (err) { + onError(err); + } + }); + } + + return stream; +}; diff --git a/package.json b/package.json index 3544f88..eb7758e 100644 --- a/package.json +++ b/package.json @@ -18,8 +18,7 @@ "dependencies": { "@hapi/hoek": "9.x.x", "@hapi/oppsy": "3.x.x", - "@hapi/validate": "1.x.x", - "pumpify": "2.x.x" + "@hapi/validate": "1.x.x" }, "devDependencies": { "@hapi/code": "8.x.x", diff --git a/test/monitor.js b/test/monitor.js index de0536d..54a5cc6 100755 --- a/test/monitor.js +++ b/test/monitor.js @@ -91,7 +91,7 @@ describe('Monitor', () => { monitor.stop(); }); - it('logs and destroys a reporter in the event of a stream error', { plan: 3 }, () => { + it('logs and destroys a reporter in the event of a stream error', { plan: 3 }, async () => { const one = new Reporters.Incrementer(1); const two = new Reporters.Writer(true); @@ -114,12 +114,20 @@ describe('Monitor', () => { // Verion 8 of node misses this change inside monitor, so force it here const foo = monitor._reporters.get('foo'); foo.destroyed = true; - foo.emit('error'); + foo.emit('error', new Error('bar')); monitor.push(() => ({ id: 3, number: 100 })); expect(two.data).to.have.length(2); expect(two.data).to.equal([{ id: 1, number: 3 }, { id: 2, number: 6 }]); - console.error = err; + + await new Promise((resolve) => { + + process.nextTick(() => { + + console.error = err; + resolve(); + }); + }); }); describe('start()', () => { From c2c051a6f350458c57909171a7558c9d23e4dea7 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Mon, 30 Nov 2020 19:29:38 +0100 Subject: [PATCH 2/3] Use finished instead of on-error --- lib/monitor.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/monitor.js b/lib/monitor.js index c815655..a63e898 100755 --- a/lib/monitor.js +++ b/lib/monitor.js @@ -1,7 +1,7 @@ 'use strict'; const Os = require('os'); -const { pipeline } = require('stream'); +const { pipeline, finished } = require('stream'); const Hoek = require('@hapi/hoek'); const Oppsy = require('@hapi/oppsy'); @@ -121,8 +121,10 @@ module.exports = internals.Monitor = class { this._reporters.set(reporterName, setupPipeline(streamObjs, (err) => { - console.error(`There was a problem (${err}) in ${reporterName} and it has been destroyed.`); - console.error(err); + if (err) { + console.error(`There was a problem (${err}) in ${reporterName} and it has been destroyed.`); + console.error(err); + } })); } @@ -205,20 +207,15 @@ module.exports = internals.Monitor = class { } }; -const setupPipeline = (streams, onError) => { +const setupPipeline = (streams, onFinished) => { const stream = streams[0]; if (streams.length === 1) { - stream.on('error', onError); + finished(stream, onFinished); } else { - pipeline(streams, (err) => { - - if (err) { - onError(err); - } - }); + pipeline(streams, onFinished); } return stream; From a0851a436f776b137216f0ef5a004fbfb3246813 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Tue, 1 Dec 2020 08:41:54 +0100 Subject: [PATCH 3/3] Remove unused code --- lib/utils.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 9323544..cd12bc3 100755 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,7 +1,5 @@ 'use strict'; -const Stream = require('stream'); - const Hoek = require('@hapi/hoek'); @@ -168,17 +166,3 @@ exports.RequestLog = class { } } }; - - -exports.NoOp = class extends Stream.Transform { - - constructor() { - - super({ objectMode: true }); - } - - _transform(value, encoding, callback) { - - callback(null, value); - } -};