warning: possible EventEmitter memory leak detected. 11 finish listeners added. about good-console

[arb]

When I run npm test in the good-console directory, I do not see this error on both version 0.10.40 and version 4.0.0 of Node. What version are you using? Also, that's a pretty hefty stack and it doesn't look like you are testing good-console directly.

[KeKs0r]

I am on node 4.2.2 and using those dependencies:

    "good": "~6.4.0",
    "good-console": "^5.2.0",

No my tests are not testing good console, they just start my hapi app 11x times and then shut it down again.

[arb]

Yeah are you using wreck and listening for wreck events? That is generally where that error comes from. Since wreck is a singleton, every test attaches another event listener to the singleton and is never cleaned up.

https://github.com/hapijs/good/blob/master/test/index.js#L11-L12

[KeKs0r]

I am not using Wreck directly, I am using h2o2 and other modules, so I can't even directly require wreck to set MaxListeners on it.

[arb]

Good attaches a listener to wreck so if you are creating your server over and over in your tests, that's what is going to cause it. Without seeing anything else, that is the best I can do.

I'm going to close this issue as good-console doesn't even have a reference to wreck and the problem is not originating from this module. If you need additional help, open an issue at https://github.com/hapijs/discuss/issues.

[KeKs0r]

I am not 100% sure if this issue is actually related to wreck. I was able to get rid of the warning by changing the the GoodConsole.init function:

internals.GoodConsole.prototype.init = function (stream, emitter, callback) {
    var self = this;

    if (!stream._readableState.objectMode) {
        return callback(new Error('stream must be in object mode'));
    }


    var filter = this._filter;
    var transform = Through.obj(function goodConsoleTransform (data, enc, next) {

        var eventName = data.event;
        var tags = [];

        /*eslint-disable */
        if (Array.isArray(data.tags)) {
            tags = data.tags.concat([]);
        } else if (data.tags != null) {
            tags = [data.tags];
        }
        /*eslint-enable */

        tags.unshift(eventName);

        if (eventName === 'response') {
            this.push(self._formatResponse(data, tags));
            return next();
        }

        if (eventName ===  'wreck') {
            this.push(self._formatWreck(data, tags));
            return next();
        }

        var eventPrintData = {
            timestamp: data.timestamp || Date.now(),
            tags: tags,
            data: undefined
        };

        if (eventName === 'ops') {
            eventPrintData.data = Hoek.format('memory: %sMb, uptime (seconds): %s, load: %s',
                Math.round(data.proc.mem.rss / (1024 * 1024)),
                data.proc.uptime,
                data.os.load);

            this.push(self._printEvent(eventPrintData));
            return next();
        }

        if (eventName === 'error') {
            eventPrintData.data = 'message: ' + data.error.message + ' stack: ' + data.error.stack;

            this.push(self._printEvent(eventPrintData));
            return next();
        }

        if (eventName === 'request' || eventName === 'log') {
            eventPrintData.data = 'data: ' + (typeof data.data === 'object' ? SafeStringify(data.data) : data.data);

            this.push(self._printEvent(eventPrintData));
            return next();
        }

        // Event that is unknown to good-console, try a defualt.
        if (data.data) {
            eventPrintData.data = 'data: ' + (typeof data.data === 'object' ? SafeStringify(data.data) : data.data);
        }
        else {
            eventPrintData.data = 'data: (none)';
        }

        this.push(self._printEvent(eventPrintData));
        return next();
    })

    var first = stream.pipe(filter);
    var second = first.pipe(transform);
    var last = second.pipe(process.stdout);

    emitter.on('stop', function(){
        stream.emit('close');
        first.emit('close');
        second.emit('close');
        last.emit('close');
    })


    callback();
};

Not sure if this is a good approach, if so I would create a PR.

[arb]

What did you change? That is a lot of code.

[KeKs0r]

the MAIN change is adding these:

    emitter.on('stop', function(){
        stream.emit('close');
        first.emit('close');
        second.emit('close');
        last.emit('close');
    })

rest is smaller refactoring, I could try to only emit a close event on the last stream, maybe its sufficient. Ill create a PR, in order to make comparison easier.

[arb]

Again; when I run the tests on just good-console, I don't have this issue.

Also, you shouldn't be emitting anything on stream because that object is being passed around to a bunch of other reporters and you have no idea what the downstream impact is of emitting a "close" event on a stream that is being consumed by multiple other streams.

If you can create an example using ONLY good-console that shows this error, then we can look at trying to make it go away. At present, I am not convinced that the issue is in good-console.

I also don't think you are supposed to manually emit a "close" event as the stream should emit that when it is actually closed, like when null is pushed through. Like here - https://github.com/hapijs/good-console/blob/master/test/index.js#L526

[KeKs0r]

I have now done the minimal necessary change to get rid of this error in #58.

My Testcase for it was just starting my server 11 times.
So any example app with good console should be able to reproduce this issue, unfortunately the good-console testcases, are not embedded within hapi, so its not that easy to show that behaviour.

[arb]

There isn't an error though. The error is coming from your hapi server setup. If you open good-console and run npm test, this message is not printed.

As I said earlier, it's the registration of good - https://github.com/hapijs/good/blob/master/lib/monitor.js#L186 that line happens 11 times and adds 11 listeners to the wreck instance.

[KeKs0r]

I have created a sample app that reproduces the error: https://github.com/KeKs0r/hapi-good
node_modules/lab/bin/lab test.js

You could replace the good console with my version in order to show that the error is gone afterwards

[arb]

I pulled your code and fiddled with it. The problem is that when good is stopped (https://github.com/hapijs/good/blob/master/lib/monitor.js#L201) it does push null through the _dataStream readable stream. When I make that change to the local install of Good, the tests pass fine without the warning. The issue was on piping to process.stdout as that is ALSO a singleton (like Wreck).

While this obviously isn't correct for Good to not tear down these streams properly, I'm hesitant to make this change in Good because it may have many downstream impacts on the existing reporter ecosystem. That being said, there is a version 7 branch of good, and I'll be sure to add this change there.

[lock]

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.