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

Wrap individual puppeteer navigations in try/catch block #175

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

westonruter
Copy link
Collaborator

@westonruter westonruter commented Jan 29, 2025

When running benchmark-web-vitals with high numbers (e.g. 250) I would eventually encounter errors like:

Network.enable timed out. Increase the 'protocolTimeout' setting in launch/connect calls for a higher timeout if needed.

or

Navigation timeout of 30000 ms exceeded.

or

Timed out after waiting 30000ms.

When these occur, the entire process quit early without completing all of the expected iterations over the URLs. I found that this was because there was a missing try/catch block around each individual iteration. This meant it would move on to process the next URL which is where the existing exception handling is located:

// Catch Puppeteer errors to prevent the process from getting stuck.
try {
const { completeRequests, metrics } = await benchmarkURL(
url,
browser,
metricsDefinition,
params,
logIterationsProgress
);

By adding this additional try/catch block, if there is a Puppeteer error, it will be the same as if the HTTP response returned with a failure. Such errors will then be reflected in the overall success rate.

Review this PR with whitespace changes ignored.

@westonruter westonruter added the bug Something isn't working label Jan 29, 2025
@westonruter westonruter force-pushed the fix/benchmark-web-vitals-exception-catching branch from dca5646 to c9ea68b Compare January 29, 2025 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant