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.

Sign up for GitHub

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

Jump to bottom

page.close() and browser.close() should be in try-catch-finally block #90

Open
ropel opened this issue Sep 29, 2021 · 4 comments
Open

page.close() and browser.close() should be in try-catch-finally block #90

ropel opened this issue Sep 29, 2021 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@ropel
Copy link
Contributor

ropel commented Sep 29, 2021

Everytime something goes wrong capturing a screenshot (i.e. no response, timeouts, ...) the function exits without closing page and browser, so that chrome instances keep growing until server dies.
Putting page.close() and browser.close() in a "finally" block should avoid resources leaking
@sindresorhus
Copy link
Owner

I agree. Pull request welcome.

@sindresorhus sindresorhus added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Oct 2, 2021
@ropel
Copy link
Contributor Author

ropel commented Oct 6, 2021

Ok, pull request done

@jopemachine
Copy link
Contributor

Ok, pull request done

Is this issue resolved?

@AlejandroAkbal
Copy link

Was this ever merged? Because I'm having zombie processes and I imagine it's because of this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@sindresorhus @ropel @jopemachine @AlejandroAkbal