got icon indicating copy to clipboard operation
got copied to clipboard

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

Open phawxby opened this issue 3 years ago • 21 comments

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

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

Related Tickets

https://github.com/lukechilds/cacheable-request/issues/97

phawxby avatar Nov 12 '20 09:11 phawxby

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.

szmarczak avatar Nov 12 '20 13:11 szmarczak

@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?

phawxby avatar Nov 12 '20 13:11 phawxby

Correct.

szmarczak avatar Nov 12 '20 14:11 szmarczak

@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.

phawxby avatar Nov 16 '20 17:11 phawxby

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.

szmarczak avatar Nov 16 '20 20:11 szmarczak

@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

phawxby avatar Nov 17 '20 10:11 phawxby

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 :(

szmarczak avatar Nov 17 '20 21:11 szmarczak

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

phawxby avatar Nov 17 '20 22:11 phawxby

Hello, I got the same issue Using native JS Map() object. What can I do ? I am using got "got": "^11.8.0"


const cache = new Map();
export const https = new HttpsProxyAgent({
  keepAlive: true,
  maxSockets: 256,
  proxy: 'http://plugin-gateway.platform:8081',
});

const product: IProduct = await got.get(`${apiUrl}/product/${productId}`, {
      retry: 2,
      cache: cache,
      agent: { https },
      headers: {
        Authorization: apiPassword,
      },
    }).json();

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

Fab-z4qx avatar Nov 27 '20 14:11 Fab-z4qx

Up ?

Fab-z4qx avatar Dec 11 '20 10:12 Fab-z4qx

Hi guys I'm experiencing the same issue as described here. Does that mean there is no way to send many request parallel with GOT using cache? How would this be possible? Sorry about my ignorance if this sounds a stupid question and thanks for your help

nardo7 avatar Dec 15 '20 15:12 nardo7

Hello guys I am experiencing this issue too. May be the work around kind of solution (or solid fix) could be added to got library itself?

ckotyan avatar Jan 19 '21 21:01 ckotyan

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?

simonecorsi avatar Feb 10 '21 17:02 simonecorsi

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.

szmarczak avatar Apr 11 '21 20:04 szmarczak

@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?

phawxby avatar Nov 01 '21 13:11 phawxby

This has nothing to do with timeouts.

szmarczak avatar Nov 01 '21 14:11 szmarczak

Any update on it ?

Fab-z4qx avatar Feb 21 '22 10:02 Fab-z4qx

@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 avatar Sep 12 '22 16:09 jaredwray

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

jaredwray avatar Sep 23 '22 17:09 jaredwray

@sindresorhus - did you want to close this?

jaredwray avatar Jun 07 '23 16:06 jaredwray

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

image

balaji2711 avatar Sep 04 '23 09:09 balaji2711