Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MaxListenersExceededWarning when trying to resolve more than 10 URLs #42

Open
castus opened this issue Nov 28, 2019 · 6 comments
Open
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@castus
Copy link

castus commented Nov 28, 2019

When I invoke code:

const urlsArray = [] // more than 10 items
let imagePromises = []
urlsArray.forEach(item => {
    const imagePath = `./tmp/${item.id}.jpg`
    const captureWebsitePromise = captureWebsite.file(item.url, imagePath, {
      width: 1280,
      height: 800,
      type: "jpeg",
      overwrite: true
    })
    imagePromises.push(captureWebsitePromise)
  }
})
Promise.all(imagePromises).then(function(imageResult) {
})

I have these errors:

(node:9535) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 exit listeners added to [process]. Use emitter.setMaxListeners() to increase limit
(node:9535) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGINT listeners added to [process]. Use emitter.setMaxListeners() to increase limit
(node:9535) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGTERM listeners added to [process]. Use emitter.setMaxListeners() to increase limit
(node:9535) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGHUP listeners added to [process]. Use emitter.setMaxListeners() to increase limit

Do you have an idea how to capture more than 10 pages using your module?

@broeker
Copy link

broeker commented Nov 29, 2019

I've been fighting with this also -- you can run more than 10 by adding this to the top of your code:

process.setMaxListeners(0);

to override the default number of listeners, but the problem here is that Node is warning you of possible memory leaks, which actually comes true depending on your server power.

In my case, I'm trying to spin through a list of 50 sites, which almost always crashes my $5/mo Digital Ocean droplet. (On my local machine it will run.) Not sure how much server power it will require to run through a list this big and I don't want to pay more, so for now I'm attempting to chunk my array of sites into lists of 10 (which runs fine on d.o), followed by a long setTimeout but so far no luck due to a different set of errors that seem to be related to trying to use different Chrome ids.

That said I've tried a bunch of similar libraries and this one seems the best, and i think the limitations may ultimately come down to puppeter/headless Chrome and the need for a lot of resources on large lists. I'm not familiar enough with the code myself to know if anything could be done inside capture-website to improve performance on bigger lists.

@broeker
Copy link

broeker commented Nov 29, 2019

This might not be ideal but after messing around with numerous other approaches, I finally found a very simple solution that will allow me to spin through 50 sites (and presumably more) on a cheap server:

  let offset = 0;
  items.map(item => {
    setTimeout(() => {
    (async () => {
      await captureWebsite.file(item[0], './screenshots/' + item[1] + '.png', options);
    })();
    }, 10000 + offset);
    offset += 10000;
  })

It seems for me the key is to wait 10 seconds between captures to give my server some breathing room. Three seconds was not long enough, I may be able to go lower but 10 seconds and it seems to chug right along with no error.

Note that this code does NOT need the unlimited listeners code i posted above, see here for a good discussion and particularly this comment:

https://stackoverflow.com/a/44446859/11777731

YMMV and if you have stronger server you may not need this but after a few days I can finally set a nightly cron with more than a few sites :)

@castus
Copy link
Author

castus commented Nov 30, 2019

Thanks for comprehensive answer. I've ended up with the usage of puppeteer itself and making screenshot using the same instance. In the loop I'm taking a screenshot, reload chrome to another URL, taking a screenshot, reload chrome and so on.

(async () => {
        const browser = await puppeteer.launch({
          args: ['--no-sandbox']
        });
        const page = await browser.newPage();
        const result = []
        // Array of { url: "https://", id: "string" }

        for (var i = 0; i < result.length; i++) {
          await page.setViewport({
            width: 1280,
            height: 800,
            deviceScaleFactor: 2
          })
          await page.goto(result[i].url, {
            waitUntil: 'networkidle2'
          })
          await page.screenshot({
            path: `./tmp/${result[i].id}.jpg`,
            type: "jpeg"
          })
          console.log(`Screenshot of ${result[i].url}, saved to ./tmp/${result[i].id}.jpg`)
        }

        await browser.close();
      })().then(function(imageResult) {
      })

@sindresorhus
Copy link
Owner

I found some workarounds we could try to add to capture-website:

@sindresorhus sindresorhus added bug Something isn't working help wanted Extra attention is needed labels Jan 1, 2020
@brc-dd
Copy link

brc-dd commented Mar 30, 2021

As a workaround one can limit the concurrency of the promises by wrapping them with a limiter function like one provided by p-limit. Ref.: What is the best way to limit concurrency when using ES6's Promise.all()?

Your code will look something like this:

  const pLimit = require('p-limit');
  const urlsArray = []; // more than 10 items
  let imagePromises = [];

  const limit = pLimit(4);

  urlsArray.forEach(item => {
    const imagePath = `./tmp/${item.id}.jpg`;
    const captureWebsitePromise = captureWebsite.file(item.url, imagePath, {
      width: 1280,
      height: 800,
      type: 'jpeg',
      overwrite: true
    });
    imagePromises.push(limit(captureWebsitePromise));
  });

  Promise.all(imagePromises).then(function (imageResult) {});

@sindresorhus
Copy link
Owner

Even easier to use p-map with its concurrency option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants