0

I am a having issues with implementing generic-pool using puppeteer. Below is my relevant part of the code.

UPDATE

Thanks @Jacob for the help and i am more clear about the concept and how it works and the code is also more readable and clear. I am still having issues where a generic pool is getting created on every request. How do i ensure that the same generic pool is used every time instead of creating new one

browser-pool.js

const genericPool = require('generic-pool');
const puppeteer = require('puppeteer');

class BrowserPool {
  static async getPool() {
    const browserParams = process.env.NODE_ENV == 'development' ? {
      headless: false,
      devtools: false,
      executablePath: '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome'
     }
     :
     {
       headless: true,
       devtools: false,
       executablePath: 'google-chrome-unstable',
       args: ['--no-sandbox', '--disable-dev-shm-usage']
     };

    const factory = {
      create: function() {
        return puppeteer.launch(browserParams);
      },
      destroy: function(instance) {
        console.log('closing browser in hrere.....');
        instance.close();
      }
    };

    const opts = {
      max: 5
    };

    this.myBrowserPool = genericPool.createPool(factory, opts);
  }

  static async returnPool() {
    if (this.myBrowserPool == "") {
      getPool();
    }

    return this.myBrowserPool.acquire();
  }
}

BrowserPool.myBrowserPool = null;
module.exports =  BrowserPool;

process-export.js

const BrowserPool = require('./browser-pool');
async function performExport(params){
  const myPool = BrowserPool.getPool();
  const resp = BrowserPool.myBrowserPool.acquire().then(async function(client){
    try {
      const url = config.get('url');
      const page = await client.newPage();

      await page.goto(url, {waitUntil: ['networkidle2', 'domcontentloaded']});
      let gotoUrl = `${url}/dashboards/${exportParams.dashboardId}?csv_export_id=${exportParams.csvExportId}`;
//more processing
      await page.goto(gotoUrl, {waitUntil: 'networkidle2' })
      await myPool().myBrowserPool.release(client);
      return Data;
    } catch(err) {
      try {
        const l = await BrowserPool.myBrowserPool.destroy(client);
      } catch(e) {
      }
      return err;
    }
  }).catch(function(err) {
    return err;
  });
  return resp;
}

module.exports.performExport = performExport;

My understanding is that

1) When the application starts I can spin up for example 2 chromium instances and then when ever i want to visit a page i can use either of the two connections, so the browsers are essentially open and we improve the performance since the browser start can take time. is this correct?

2) Where do I place the acquire() code, I understand this should be in the app.js, so we acquire the instances rite when the app boots, but my pupeteer code is in a different file, how do i pass the browser reference in the file which has my pupeteer code.

When I use the above the code, a new browser instances spins up every time and the max property is not considered and it opens up as many instances are requested.

My apologies if its something very trial and i might have not understood the concept fully. Any help in clarifying this would be really helpful.

Improve this question

1 Answer 1

Reset to default
1

When using a pool, you'll need to use .acquire() to obtain an object, and then .release() when you're done so the object is returned to the pool and made available to something else. Without using .release(), you'd might as well have no pool at all. I like to use this helper pattern with pools:

class BrowserPool {
  // ...

  static async withBrowser(fn) {
    const pool = BrowserPool.myBrowserPool;
    const browser = await pool.acquire();
    try {
      await fn(browser);
    } finally {
      pool.release(browser);
    }
  }
}

This can be used like this anywhere in your code:

await BrowserPool.withBrowser(async browser => {
  await browser.doSomeThing();
  await browser.doSomeThingElse();
});

The key is the finally clause makes sure that whether your tasks complete or throw an error, you'll cleanly release the browser back to the pool every time.

It sounds like you might have the concept of the max option backwards as well and are expecting the browser instances to be spawned up to max. Rather, max means "only create up to max number of resources." If you try to acquire a sixth resource without anything having been released, for example, the acquire(...) call will block until one item is returned to the pool.

The min option, on the other hand, means "keep at least this many items on hand at all times", which you can use to pre-allocate resources. If you want 5 items to be created in advance, set min to 5. If you want 5 items and only five items to be created, set both min and max to 5.

Update:

I notice in your original code that you destroy in case of error and release when there isn't an error. Still would prefer the benefit of a wrapper function like mine to centralize all resource acquiring/releasing logic (the SRP approach). Here's how it could be updated to automatically destroy on errors instead:

class BrowserPool {
  // ...

  static async withBrowser(fn) {
    const pool = BrowserPool.myBrowserPool;
    const browser = await pool.acquire();
    try {
      await fn(browser);
      pool.release(browser);
    } catch (err) {
      await pool.destroy(browser);
      throw err;
    }
  }
}

Addendum

Figuring out what's going on in your code will be easier if you embrace the async function instead of mixing async function stuff and Promise callback stuff. Here's how it can be rewritten:

async function performExport(params){
  const myPool = BrowserPool.myBrowserPool;
  const client = await myPool.acquire();

  try {
    const url = config.get('url');
    const page = await client.newPage();

    await page.goto(url, {waitUntil: ['networkidle2', 'domcontentloaded']});
    let gotoUrl = `${url}/dashboards/${exportParams.dashboardId}?csv_export_id=${exportParams.csvExportId}`;
//more processing
    await page.goto(gotoUrl, {waitUntil: 'networkidle2' })
    await myPool.release(client);
    return Data;
  } catch(err) {
    try {
      const l = await myPool.destroy(client);
    } catch(e) {
    }
    return err; // Are you sure you want to do this? Would suggest throw err.
  }
}
Improve this answer
Sign up to request clarification or add additional context in comments.

19 Comments

many thanks for the answer, i have been strugling to understand this fully and this reply helps. I am also trying to understand that when my app boots up, should i be seeing chrom instances spin up already? or will the instance spin up only when acquire is called? in that case how is this different from not using in pool
What do you mean release is not called; are you calling release in a finally clause like in my code, in an async function? It should always get called after the promise resolves/rejects.
destroy is something different. release returns the item to the pool so code later on can use the same object. destroy is for if you never want to use that object again.
If you have pool acquire/release code in multiple places I'd encourage you to change that, because then it's hard to put together a clean mental picture of how resources are being maintained. The benefit of the pattern I showed is you have all browser resource management centralized in one place.
Will just leave a comment here. The pool was working fine. However, the code was not running any requests concurrently, and it was also running requests in their own child processes, so they could not share a browser pool. Solution was to not use pooling and to use the concurrency controls of the worker queue library instead.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.