Node.js: share connection object throughout the application

Question

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.

Answer 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.
  }
}