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

Generating multiple images output issue #85

Closed
iamparthaonline opened this issue Dec 21, 2020 · 8 comments · Fixed by #157
Closed

Generating multiple images output issue #85

iamparthaonline opened this issue Dec 21, 2020 · 8 comments · Fixed by #157
Labels
bug Something isn't working needs reproduction

Comments

@iamparthaonline
Copy link

When I am trying to generate multiple image files by using array of values in content property as specified in the readme section below,
Generating multiple images
It gives 2 peculire issues -

  1. Sometimes the outputs are organised ( same as the content property value array ) but most of the time those aren't.
  2. Sometimes some images are skipped totally.
@frinyvonnick frinyvonnick added bug Something isn't working needs reproduction labels Dec 21, 2020
@frinyvonnick
Copy link
Owner

Hi @iamparthaonline 👋

Thank you for posting this issue. Is it possible to share with me some reproduction code?

@iamparthaonline
Copy link
Author

iamparthaonline commented Dec 22, 2020

Hi @frinyvonnick ,

so this are the puppeteer config that I'm using,
const puppeteerArgs = { args: [ '--no-sandbox', '--disable-setuid-sandbox', '--disable-dev-shm-usage', '--disable-accelerated-2d-canvas', '--no-first-run', '--headless', '--no-zygote', '--disable-gpu' ], headless: true, ignoreHTTPSErrors: true };

Then I'm creating an array of image properties inside a for loop,
imagesBeingProcessed.push({ image: remoteImageURL, height, width, branding, name });

After that I'm running multiple image generation by calling once ( out side the loop ),
let data = await nodeHtmlToImage({ html: getMarkup(), content: imagesBeingProcessed, puppeteerArgs });

Now as the document stated data should have an array of image buffers, and i expect the output to be in the same order as imagesBeingProcessed , but it's not giving like that ( it's giving in random order or some images are missing in between) and also some images aren't even present even though the length of data is same as imagesBeingProcessed.

P.S. Single image generation works like a charm. I used to do multiple single image generation, but because of performance optimization, I'm trying out the multiple-image generation by single call approach. Also, any performance-related advice will be really helpful.

@frinyvonnick
Copy link
Owner

Thank you for your complete answer. I'll try to reproduce your issue as soon as possible. Regarding the performance part there is an issue on the subject. node-html-to-image needs to be improved with the information from this issue 😄

@KL13NT
Copy link

KL13NT commented Dec 29, 2020

This probably has to do with how puppeteer-cluster works. It spawns a specific number of puppeteer instances (2 currently) and passes jobs to each of these instances. node-html-to-image iterates using forEach on the array of images you pass in, and queues them to the clusters using cluster.queue({ output, content: pageContent }). So whenever a cluster is done rendering an image, it's passed another, which would eventually violate the original ordering of the passed array of images because of the delays in spawning these different instances and the host system resource allocation to each instance.

To maintain the ordering AFAIK you could spawn a single instance in the cluster, which will then render the images in order. This of course comes with a performance hit.

@iamparthaonline
Copy link
Author

This probably has to do with how puppeteer-cluster works. It spawns a specific number of puppeteer instances (2 currently) and passes jobs to each of these instances. node-html-to-image iterates using forEach on the array of images you pass in, and queues them to the clusters using cluster.queue({ output, content: pageContent }). So whenever a cluster is done rendering an image, it's passed another, which would eventually violate the original ordering of the passed array of images because of the delays in spawning these different instances and the host system resource allocation to each instance.

To maintain the ordering AFAIK you could spawn a single instance in the cluster, which will then render the images in order. This of course comes with a performance hit.

I have been doing what you said, spawning a single instance and let it handle images one by one. This is really performance heavy that's why I'm trying to understand how can we make benefit out of puppeteer-cluster and why only 2 instances are being spawned ?? it should be more else there's not much much we are making out of clustering.

@increpare
Copy link

increpare commented Sep 16, 2021

Also found myself experiencing this - happy to have stumbled across this thread ^^ . I guess if

buffers.push(buffer);
kept track of the index and assigned it explicitly rather than pushing to the array, it'd solve this problem, right?

increpare added a commit to increpare/node-html-to-image that referenced this issue Sep 16, 2021
@frinyvonnick
Copy link
Owner

Thank you for sharing @increpare, feel free to open a pull request. It could be great to write a test that reproduce the issue and try your patch to see if it fixes it 👍

@frinyvonnick
Copy link
Owner

Published in v3.2.1 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs reproduction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants