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

improvement: close browser before throwing error #201

Merged
merged 1 commit into from
Mar 6, 2023

Conversation

ClassAxion
Copy link
Contributor

We should catch errors then close browser and throw the error because otherwise people like me who have this script in loop with own error handling will have memory leak because old browser stays in background.

Please check before mergeing.

@pierreminiggio
Copy link
Contributor

pierreminiggio commented Mar 6, 2023

In some other parts of the code, we already have checks for some errors that will kill the browser before throwing again.
But can't hurt to have another checker higher in the loop like you added to make up for the other errors that aren't caught.
I've one question though : what does browser.close() does if the browser happens to be already closed ?

@ClassAxion
Copy link
Contributor Author

Ok, I didn't know about it. So browser.close() in catch should probably be in individual try ... catch
Sometimes I have internet problems so I've got Navigation timeout and then the browser doesn't close.

@fawazahmed0
Copy link
Owner

what does browser.close() does if the browser happens to be already closed ?

It doesn't do anything

import puppeteer from 'puppeteer';

(async () => {
  const browser = await puppeteer.launch({headless:false});
  await browser.close();
  await browser.close();

  console.log('hi')
})();

Output:

hi

@fawazahmed0
Copy link
Owner

Thanks for PR

@fawazahmed0 fawazahmed0 merged commit c4d0daa into fawazahmed0:main Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants