MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 error listeners added to [Keyv]. Use emitter.setMaxListeners() to increase limit

[phawxby]

Describe the bug

When using simple cache adapters MaxListenersExceededWarning is emitted. This has previously been investigated in #792 and #1128 but either it is a regression or did not account for highly asynchronous environments with 100's of parallel requests.

Unsure if this is best opened in either keyv or cacheable-request, but given it was declared solved in #1128 I've opened it here first.

Environment Info:
  System:
    OS: macOS 10.15.7
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  Binaries:
    Node: 12.19.0 - ~/.nvm/versions/node/v12.19.0/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 6.14.8 - ~/.nvm/versions/node/v12.19.0/bin/npm
  Browsers:
    Chrome: 86.0.4240.198
    Safari: 14.0

Got: 11.8.0

Actual behavior

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 error listeners added to [Keyv]. Use emitter.setMaxListeners() to increase limit

Code to reproduce

const got = require('got');

const client = got.extend({
    cache: {
        get: (key) => {
            return undefined;
        },
        set: (key, value) => {

        }
    }
});

(async () => {
    const promises = [];

    for (let i = 0; i < 20; i++) {
        promises.push(client('https://www.google.com', { method: 'HEAD' }));
    };

    await Promise.all(promises);

    console.log('Done');
})();

Checklist

• I have read the documentation.
• I have tried my code with the latest version of Node.js and Got.

Related Tickets

jaredwray/cacheable#97

[szmarczak]

Actually this isn't a memory leak.

https://github.com/lukechilds/cacheable-request/blob/0656a46a3573a4de09f592f1d334433d223e68b7/src/index.js#L190

But the code can be optimized anyway.

[phawxby]

@szmarczak ah, so the listener is removed after handling is complete so this only occurs when you have more than 10 requests in flight at any one time?

[szmarczak]

Correct.

[phawxby]

@szmarczak so how would you suggest proceeding because having many requests in flight at any one time is fairly normal use-case. Does the max listeners limit need raising? Warnings being emitted during normal operation scares the villagers, such as myself.

[szmarczak]

The workaround lies in the very title of the issue:

Use emitter.setMaxListeners() to increase limit

Simply pass a Keyv instance as the cache option. Just set cache.setMaxListeners(100000) and you're good to go.

[phawxby]

@szmarczak I expected that too, but it doesn't. That's why I created this (by the way, thanks for helping chase this around).

const got = require('got');
const Keyv = require('keyv');

const keyv = new Keyv();
keyv.setMaxListeners(10000);

const client = got.extend({
    cache: {
        get: (key) => {
            return undefined;
        },
        set: (key, value) => {

        }
    }
});

const keyvClient = got.extend({
    cache: keyv
});

(async () => {

    console.log('Simple cache client', 'start');
    let promises = [];
    for (let i = 0; i < 20; i++) {
        promises.push(client(`https://www.google.com?${i}`, { method: 'HEAD' }));
    };
    await Promise.all(promises);
    console.log('Simple cache client', 'end');

    console.log('Keyv cache client', 'start');
    promises = [];
    for (let i = 0; i < 20; i++) {
        promises.push(keyvClient(`https://www.google.com?${i}`, { method: 'HEAD' }));
    };
    await Promise.all(promises);
    console.log('Keyv cache client', 'end');

    console.log('Done');
})();

Output

paul@Pauls-iPad got-keyv % node test.js         
Simple cache client start
(node:55811) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 error listeners added to [Keyv]. Use emitter.setMaxListeners() to increase limit
(Use `node --trace-warnings ...` to show where the warning was created)
Simple cache client end
Keyv cache client start
(node:55811) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 error listeners added to [Keyv]. Use emitter.setMaxListeners() to increase limit
Keyv cache client end
Done

[szmarczak]

It's because it creates another Keyv instance:

https://github.com/lukechilds/cacheable-request/blob/0656a46a3573a4de09f592f1d334433d223e68b7/src/index.js#L19

So I don't think there is any workaround at all for now :(

[phawxby]

@szmarczak ok. Many thanks for the help with all of this!

[simonecorsi]

It's because it creates another Keyv instance:

I'm facing issues for similar reasons, it's also registering a once error listener that gets cleared after response, which is okay-ish but it doesn't account for databases connection error (eg: replicas failover) which crashes the process and prevent network fallback.

I think Got could also benefit from this, I've opened a PR about this at cacheable-request, @szmarczak are you also a maintainer of that lib? WDYT?

[szmarczak]

I've opened a PR about this at cacheable-request

Looks good. I'll look deeper this week to look out for bugs if any.

@szmarczak are you also a maintainer of that lib?

Yes. I'm going to rewrite cacheable-request completely but first we're releasing Got 12 soon.

[phawxby]

@szmarczak we've been having some issues with timeouts of tests lately and we're wondering if this is now potentially part of the cause. Have you made any progress on figuring out a fix for this?

[szmarczak]

This has nothing to do with timeouts.

[Fab-z4qx]

Any update on it ?

[jaredwray]

@Fab-z4qx - I believe this has been updated in keyv and we should be good to go. If not can you provide an updated detail of how you are getting this error and we will look into it.

[jaredwray]

@sindresorhus or @szmarczak - this I believe has been resolved and you can close it as the pull request was merged here: jaredwray/cacheable#101

[jaredwray]

@sindresorhus - did you want to close this?

[balaji2711]

I was trying to run the playwright tests and ended up with the MaxListenersExceededWarning error -