Skip to content

Commit

Permalink
Logs readability and other smaller corrections.
Browse files Browse the repository at this point in the history
  • Loading branch information
PaulDalek committed Oct 18, 2024
1 parent 5a2f159 commit 0a25484
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 36 deletions.
38 changes: 26 additions & 12 deletions lib/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,15 @@ async function reconnect() {
logWithStack(
1,
error,
'[browser] Could not close the browser before relaunch.'
'[browser] Could not close the browser before relaunching (probably is already closed).'
);
}

// Try to relaunch the browser
await create(getOptions().puppeteer.args || []);

// Log the success message
log(3, `[browser] Browser relaunched successfully.`);
}
}

Expand Down Expand Up @@ -260,9 +263,9 @@ export async function newPage(poolResource) {

log(
3,
`[pool] Successfully created a worker ${poolResource.id} - took ${
`[pool] Pool resource [${poolResource.id}] - Successfully created a worker, took ${
new Date().getTime() - startDate
} ms.`
}ms.`
);

// Return the resource with a ready to use page
Expand Down Expand Up @@ -303,8 +306,10 @@ export async function clearPage(poolResource, hardReset = false) {
logWithStack(
2,
error,
`[browser] Worker with ID ${poolResource.id}: could not clear the content of the page.`
`[pool] Pool resource [${poolResource.id}] - Content of the page could not be cleared.`
);
// Set the `workLimit` to exceeded in order to recreate the resource
poolResource.workCount = getOptions().pool.workLimit + 1;
}
}

Expand Down Expand Up @@ -422,11 +427,11 @@ export async function addPageResources(page, options) {
* to be cleared.
*/
export async function clearPageResources(page, injectedResources) {
for (const resource of injectedResources) {
await resource.dispose();
}

try {
for (const resource of injectedResources) {
await resource.dispose();
}

// Destroy old charts after export is done and reset all CSS and script tags
await page.evaluate(() => {
// We are not guaranteed that Highcharts is loaded, e,g, when doing SVG
Expand Down Expand Up @@ -529,7 +534,10 @@ function setPageEvents(poolResource) {
mainFrame.detached &&
poolResource.workCount <= pool.workLimit
) {
log(3, `[browser] The page's frame detached.`);
log(
3,
`[browser] Pool resource [${poolResource.id}] - Page's frame detached.`
);
try {
// Try to connect to a new page using exponential backoff strategy
expBackoff(
Expand All @@ -542,7 +550,7 @@ function setPageEvents(poolResource) {
} catch (error) {
log(
3,
`[browser] Could not close the page with a detached frame. Page already closed, id: ${poolResourceId}.`
`[browser] Pool resource [${poolResourceId}] - Could not close the page with a detached frame.`
);
}

Expand All @@ -554,8 +562,14 @@ function setPageEvents(poolResource) {
poolResource
);
} catch (error) {
log(3, '[browser] Could not create a new page.');
poolResource.page = null;
logWithStack(
3,
error,
`[browser] Pool resource [${poolResource.id}] - Could not create a new page.`
);

// Set the `workLimit` to exceeded in order to recreate the resource
poolResource.workCount = pool.workLimit + 1;
}
}
});
Expand Down
52 changes: 29 additions & 23 deletions lib/pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,42 +73,42 @@ const factory = {
* Validates a worker page in the export pool, checking if it has exceeded
* the work limit.
*
* @param {Object} workerHandle - The handle to the worker, containing the
* @param {Object} poolResource - The handle to the worker, containing the
* worker's ID, a reference to the browser page, and work count.
*
* @returns {boolean} - Returns true if the worker is valid and within
* the work limit; otherwise, returns false.
*/
validate: async (workerHandle) => {
validate: async (poolResource) => {
let validated = true;

// Check if the `workLimit` is exceeded
if (
poolConfig.workLimit &&
++workerHandle.workCount > poolConfig.workLimit
++poolResource.workCount > poolConfig.workLimit
) {
log(
3,
`[pool] Worker with ID ${workerHandle.id} failed validation: exceeded work limit (limit is ${poolConfig.workLimit}).`
`[pool] Pool resource [${poolResource.id}] - Validation failed (exceeded the ${poolConfig.workLimit} works limit).`
);
validated = false;
}

// Check if the `page` is not valid
if (!workerHandle.page) {
if (!poolResource.page) {
// Check if the `page` is closed
if (workerHandle.page.isClosed()) {
if (poolResource.page.isClosed()) {
log(
3,
`[pool] Worker with ID ${workerHandle.id} failed validation: page is closed or invalid.`
`[pool] Pool resource [${poolResource.id}] - Validation failed (page is closed or invalid).`
);
}

// Check if the `mainFrame` is detached
if (workerHandle.page.mainFrame().detached) {
if (poolResource.page.mainFrame().detached) {
log(
3,
`[pool] Worker with ID ${workerHandle.id} failed validation: page's frame is detached.`
`[pool] Pool resource [${poolResource.id}] - Validation failed (page's frame is detached).`
);
}
validated = false;
Expand All @@ -133,25 +133,25 @@ const factory = {
/**
* Destroys a worker entry in the export pool, closing its associated page.
*
* @param {Object} workerHandle - The handle to the worker, containing
* @param {Object} poolResource - The handle to the worker, containing
* the worker's ID and a reference to the browser page.
*/
destroy: async (workerHandle) => {
log(3, `[pool] Destroying worker with ID ${workerHandle.id}.`);
destroy: async (poolResource) => {
log(3, `[pool] Pool resource [${poolResource.id}] - Destroying a worker.`);

if (workerHandle.page) {
if (poolResource.page) {
try {
// Remove all attached event listeners from the resource
workerHandle.page.removeAllListeners('pageerror');
workerHandle.page.removeAllListeners('console');
workerHandle.page.removeAllListeners('framedetached');
poolResource.page.removeAllListeners('pageerror');
poolResource.page.removeAllListeners('console');
poolResource.page.removeAllListeners('framedetached');

// We need to wait around for this
await workerHandle.page.close();
await poolResource.page.close();
} catch (error) {
log(
3,
`[pool] Page of a worker with ID ${workerHandle.id} could not be closed upon destroying.`
`[pool] Pool resource [${poolResource.id}] - Page could not be closed upon destroying.`
);
}
}
Expand Down Expand Up @@ -206,12 +206,15 @@ export const initPool = async (config) => {

// Set events
pool.on('release', async (resource) => {
log(4, `[pool] Releasing a worker with ID ${resource.id}.`);
log(4, `[pool] Pool resource [${resource.id}] - Releasing a worker.`);
await clearPage(resource, false);
});

pool.on('destroySuccess', (_eventId, resource) => {
log(4, `[pool] Destroyed a worker with ID ${resource.id}.`);
log(
4,
`[pool] Pool resource [${resource.id}] - Destroyed a worker successfully.`
);
resource.page = null;
});

Expand Down Expand Up @@ -335,7 +338,7 @@ export const postWork = async (chart, options) => {
log(4, '[pool] Acquired a worker handle.');

if (!workerHandle.page) {
// Release the resource with exceeded `workLimit` in order to recreate
// Set the `workLimit` to exceeded in order to recreate the resource
workerHandle.workCount = poolConfig.workLimit + 1;
throw new ExportError(
'Resolved worker page is invalid: the pool setup is wonky.',
Expand All @@ -346,7 +349,10 @@ export const postWork = async (chart, options) => {
// Save the start time
let workStart = new Date().getTime();

log(4, `[pool] Starting work on pool entry with ID ${workerHandle.id}.`);
log(
4,
`[pool] Pool resource [${workerHandle.id}] - Starting work on this pool entry.`
);

// Perform an export on a puppeteer level
const exportCounter = measureTime();
Expand All @@ -355,7 +361,7 @@ export const postWork = async (chart, options) => {
// Check if it's an error
if (result instanceof Error) {
if (result.message === 'Rasterization timeout') {
// Release the resource with exceeded `workLimit` in order to recreate
// Set the `workLimit` to exceeded in order to recreate the resource
workerHandle.workCount = poolConfig.workLimit + 1;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export const expBackoff = async (fn, attempt = 0, ...args) => {
await new Promise((response) => setTimeout(response, delayInMs));
log(
3,
`[pool] Waited ${delayInMs}ms until next call for the resource id: ${args[0]}.`
`[pool] Waited ${delayInMs}ms until next call for the resource of ID: ${args[0]}.`
);

// Try again
Expand Down

0 comments on commit 0a25484

Please sign in to comment.