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

[Bug] My software stucks on browser.close with plugin-stealth 2.7.x version #421

Closed
Dam998 opened this issue Feb 4, 2021 · 56 comments
Closed
Labels
issue: bug report A bug has been reported needs triage work-in-progress This is currently being worked on

Comments

@Dam998
Copy link

Dam998 commented Feb 4, 2021

My software stucks on browser.close with last version (puppeteer-extra-plugin-stealth)

I'm using more than 1 browser open at the same time, and when I'm trying to close 1 my software stucks.
this has never happened with version 2.6.7 or before

my code:

var pages = await this.browser.pages()
for (let i = 0; i < pages.length; i++) {
    await pages[i].close();
}
await this.browser.close();

Tested with 2.7.x version

if i downgrade to 2.6.7 everything works fine

@Dam998 Dam998 added issue: bug report A bug has been reported needs triage labels Feb 4, 2021
@Dam998 Dam998 changed the title [Bug] My software stucks on browser.close with 2.7.x version [Bug] My software stucks on browser.close with plugin-stealth 2.7.x version Feb 4, 2021
@berstend
Copy link
Owner

berstend commented Feb 4, 2021

How are you opening the browsers, .launch or .connect?

@Dam998
Copy link
Author

Dam998 commented Feb 4, 2021

.launch

@berstend
Copy link
Owner

berstend commented Feb 4, 2021

Could you remove the user-agent-override evasion and test if it works then?

const pluginStealth = require('puppeteer-extra-plugin-stealth')()
pluginStealth.enabledEvasions.delete('user-agent-override')
puppeteer.use(pluginStealth)

@berstend
Copy link
Owner

berstend commented Feb 4, 2021

The issue might be related to #413 - we save some state there and added functionality depending on how the browser is launched

@Dam998
Copy link
Author

Dam998 commented Feb 4, 2021

if I remove user-agent-override is working

can I ask you why? if this is correct and nothing change at the end
or if you suggest me to launch browser in a different way

@berstend
Copy link
Owner

berstend commented Feb 4, 2021

So it works without the user-agent-override evasion?

The reason is to narrow down the problem, we made a larger update to the evasion so it makes sense the issue is somewhere there.

If removing user-agent-override fixes the issue in 2.7.x then you just need to wait for us to develop a fix, until then you need to use the older version for your use-case.

@berstend
Copy link
Owner

berstend commented Feb 4, 2021

@Dam998 did you mean opening multiple browsers or multiple pages in a single browser?

Your words indicate the former, your code the latter

@Dam998
Copy link
Author

Dam998 commented Feb 4, 2021

opening multiple browsers (more browsers, each browser with 2 pages)

I created a class that on start() open a browser and then close it at the end, this is why

@Niek
Copy link
Collaborator

Niek commented Feb 4, 2021

@Dam998 do you also mind sharing a minimal reproducible code sample so we can check?

@berstend
Copy link
Owner

berstend commented Feb 4, 2021

Are you able to make a small self-contained test file? this helps us reproduce the issue quickly. Thanks!

@Dam998
Copy link
Author

Dam998 commented Feb 4, 2021

I'll try, give me some minutes

@Dam998
Copy link
Author

Dam998 commented Feb 5, 2021

I reproduced it with this simple script:

const puppeteer = require('puppeteer-extra');
const StealthPlugin = require('puppeteer-extra-plugin-stealth');
const locateChrome = require('locate-chrome');
const delay = require('delay');

(async () => {
    console.log('start')
    for (var i = 0; i < 20; i++) {
        (async () => {
            console.log('start')
            try {
                var chromeOptions = {
                    headless: false,
                    ignoreDefaultArgs: ['--disable-extensions'],
                    ignoreHTTPSErrors: true,
                    args: [
                        '--disable-web-security',
                        '--disable-dev-shm-usage',
                        '--no-sandbox',
                        '--disable-setuid-sandbox',
                        '--disable-gpu',
                        '--ignore-certificate-errors',
                        '--enable-features=NetworkService'
                    ],
                };

                let chromePath = await locateChrome()
                chromeOptions.executablePath = chromePath;

                puppeteer.use(StealthPlugin())
                const browser = await puppeteer.launch(chromeOptions);
                const page = await browser.newPage();
                
                await page.goto('https://github.com/berstend/puppeteer-extra/issues/421', { 'waitUntil': 'domcontentloaded', timeout: 60000 });
                await delay(13000);

                var pages = await browser.pages()
                for (let j = 0; j < pages.length; j++) {
                    await pages[j].close();
                }
                await browser.close();
            }
            catch (err) {
                console.log(err)
            }
        })();
        await delay(10000);
    }
})();

this script just open more browsers and then close them continusly
with 2.6.7 version everything works good, with 2.7.x after the first browser close, the script freezes

and with 2.7.x if you try to do CTRL + C in terminal it not wll close, with 2.6.7 yes

@berstend
Copy link
Owner

berstend commented Feb 5, 2021

Thanks! can't test right now but does it make a difference if you move puppeteer.use(StealthPlugin()) outside of the loop? It's not necessary to do that 20 times, once is enough. :-)

@Dam998
Copy link
Author

Dam998 commented Feb 5, 2021

nothing change

this is only an example, I made this little mistake here but nothing change if I move it outside of the loop

@berstend
Copy link
Owner

berstend commented Feb 5, 2021

Gotcha. Thanks for the test case, we'll look into it. :-)

@Niek
Copy link
Collaborator

Niek commented Feb 18, 2021

I cannot reproduce this issue, your test script works fine for me @Dam998. I only got some memleak warnings but that's to be expected:

$ node test.js
start
start
start
start
start
start
start
start
start
start
(node:70151) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 exit listeners added to [process]. Use emitter.setMaxListeners() to increase limit
(Use `node --trace-warnings ...` to show where the warning was created)
(node:70151) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGINT listeners added to [process]. Use emitter.setMaxListeners() to increase limit
(node:70151) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGTERM listeners added to [process]. Use emitter.setMaxListeners() to increase limit
(node:70151) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGHUP listeners added to [process]. Use emitter.setMaxListeners() to increase limit
start
start
start
start
start
start
start
start
start
start
start

@Dam998 can you run the following script and paste the output in a comment?

npx envinfo@latest --system --binaries --npmPackages '*(puppeteer*|playwright*|automation-extra*|@extra*)'

@Dam998
Copy link
Author

Dam998 commented Feb 18, 2021

@Niek

image

@Dam998
Copy link
Author

Dam998 commented Feb 18, 2021

I tested again my script with 2.7.4 version and I'm getting same error
with 2.6.7 works fine

@berstend
Copy link
Owner

@Dam998 pretty sure this is related to you using Windows in one way or another - our core team is all on POSIX.

Are you using WSL to run this stuff?

@berstend
Copy link
Owner

The issue is most likely somewhere in the user-data-dir plugin:
https://github.com/berstend/puppeteer-extra/blob/master/packages/puppeteer-extra-plugin-user-data-dir/index.js

Can you run your script with DEBUG=puppeteer-extra-plugin:user-data-dir myscript.js?

@Dam998
Copy link
Author

Dam998 commented Feb 19, 2021

with version 2.7.4:

$ DEBUG=puppeteer-extra-plugin:user-data-dir node testBug.js

start
start
  puppeteer-extra-plugin:user-data-dir initialized {
  deleteTemporary: true,
  deleteExisting: false,
  files: [],
  folderPath: 'C:\\Users\\dpapi\\AppData\\Local\\Temp',
  folderPrefix: 'puppeteer_dev_profile-'
} +0ms
  puppeteer-extra-plugin:user-data-dir created custom dir C:\Users\dpapi\AppData\Local\Temp\puppeteer_dev_profile-sCe9ey +5ms
  puppeteer-extra-plugin:user-data-dir Wrote file C:\Users\dpapi\AppData\Local\Temp\puppeteer_dev_profile-sCe9ey\Default\Preferences +12ms
start
  puppeteer-extra-plugin:user-data-dir created custom dir C:\Users\dpapi\AppData\Local\Temp\puppeteer_dev_profile-O8kLVd +10s
  puppeteer-extra-plugin:user-data-dir Wrote file C:\Users\dpapi\AppData\Local\Temp\puppeteer_dev_profile-O8kLVd\Default\Preferences +1ms
  puppeteer-extra-plugin:user-data-dir onClose +5s
  puppeteer-extra-plugin:user-data-dir removeUserDataDir +1ms

with version 2.7.4 everything stuck, the terminal too doesn't responde, I need always to force close it
but only if I reproduce this situation, if I use only 1 browser at a time there are no problems

@Niek
Copy link
Collaborator

Niek commented Feb 20, 2021

@berstend what do you think about bumping fs-extra to @9? See also: https://github.com/jprichardson/node-fs-extra/blob/master/CHANGELOG.md

I think this might be the main cause: jprichardson/node-fs-extra#804

@berstend
Copy link
Owner

Yeah, bumping fs-extra might fix this issue

@Niek
Copy link
Collaborator

Niek commented Mar 14, 2021

@Dam998 Please check with the latest master branch.

@KhalilMohammad
Copy link

I get this issue if I use puppeteer-extra with puppeteer-page-proxy.
It freezes on browser.close()

@alamothe
Copy link

alamothe commented Sep 2, 2021

@k-funk @KhalilMohammad this issue is Windows specific, please create a separate issue. Thanks!

@LinkTree3
Copy link

LinkTree3 commented Sep 8, 2021

I am seeing the same issue right now.
I am on windows and when I add 'puppeteer-extra-plugin-stealth' and use puppeteer.use(StealthPlugin()) I can't use ctrl+c to terminate the process. :(

Is there any solution for this?

I ended up reverting to 2.6.7 as well. It works for me for now. :)

@berstend
Copy link
Owner

berstend commented Sep 13, 2021

Alright, to summarize what we know so far:

Can someone experiencing this issue monkey patch their ./node_modules/puppeteer-extra-plugin-user-data-dir/index.js file and comment out this onClose handler to see if this fixes it?

@berstend
Copy link
Owner

more specifically the onClose handler will result in this sync call:

deleteUserDataDir() {
debug('removeUserDataDir')
try {
// We're doing it sync to improve chances to cleanup
// correctly in the event of ultimate disaster.
fse.rmdirSync(this._userDataDir, { recursive: true })
} catch (e) {
debug(e)
}
}

@LinkTree3
Copy link

Alright, to summarize what we know so far:

Can someone experiencing this issue monkey patch their ./node_modules/puppeteer-extra-plugin-user-data-dir/index.js file and comment out this onClose handler to see if this fixes it?

Hey thank you so much for looking into this! Yes I'll gladly check this and report. :)

@LinkTree3
Copy link

LinkTree3 commented Sep 13, 2021

Yes your suggestion worked! :)

Now how do we fix it permanently without needing to resort to monkey patching?

@berstend
Copy link
Owner

@LinkTree3 Cool, thanks for testing :)

Would you be able to test if this change makes a difference (commenting the onClose handler back in)?
https://github.com/berstend/puppeteer-extra/pull/502/files

(basically replacing rmdirSync with rmSync)

@dev-hyperweb
Copy link
Contributor

it clearly. this issued stuck in onClose event

so if im not wrong, even empty event still stuck
in now, I fixed with onDisconnected event to use for my project

@LinkTree3
Copy link

rmSync

This did not work.

@berstend
Copy link
Owner

rmSync

This did not work.

Ok, one last test :)

fs.rm(this._userDataDir, { recursive: true, force: true }, console.log)

@LinkTree3
Copy link

LinkTree3 commented Sep 13, 2021

YUP This did the trick! :)

@berstend
Copy link
Owner

YUP This did the trick! :)

Sweet! Thanks for testing 👍 So we know that (for whatever reason) removing the temporary profile folder synchronously can cause issues on windows, whereas doing it asynchronously works fine.

I'm gonna prepare a fix + new version within the next days, as time permits.

@LinkTree3
Copy link

YUP This did the trick! :)

Sweet! Thanks for testing 👍 So we know that (for whatever reason) removing the temporary profile folder synchronously can cause issues on windows, whereas doing it asynchronously works fine.

I'm gonna prepare a fix + new version within the next days, as time permits.

Thank you so much! looking forward to it :)

@berstend berstend added work-in-progress This is currently being worked on and removed ongoing-research labels Sep 13, 2021
@LinkTree3
Copy link

LinkTree3 commented Sep 13, 2021

berstend I am so sorry, but it seems that I might have had a mistake while testing it with
fs.rm(this._userDataDir, { recursive: true, force: true }, console.log)

Now when I try it again it seems to not work :(

I am getting an EBUSY error and a bunch of "null" when I try to exit

[Error: EBUSY: resource busy or locked, unlink 'C:\Users[username]\AppData\Local\Temp\puppeteer_dev_profile-1BHM2o\lockfile'] {
errno: -4082,
code: 'EBUSY',
syscall: 'unlink',
path: 'C:\Users\[username]\AppData\Local\Temp\puppeteer_dev_profile-1BHM2o\lockfile'
}

image

@berstend
Copy link
Owner

@LinkTree3 no worries :) could you try if changing onClose to onDisconnected (with the original fse.rmdirSync code) does make a difference in your case?
https://github.com/berstend/puppeteer-extra/pull/530/files

@LinkTree3
Copy link

LinkTree3 commented Sep 13, 2021

@LinkTree3 no worries :) could you try if changing onClose to onDisconnected (with the original fse.rmdirSync code) does make a difference in your case?
https://github.com/berstend/puppeteer-extra/pull/530/files

Yes this one seems to work! I tested it a couple times to make sure :)

I also added a console log right before and after the fse.rmdirSync call and they both fired after the puppeteer browser closed.

image

@berstend
Copy link
Owner

@LinkTree3 awesome, we're gonna go with this fix then

@berstend
Copy link
Owner

This should be fixed now through #530 (thanks @dev-hyperweb 👍 )

Successfully published:
 - puppeteer-extra-plugin-stealth@2.7.9
 - puppeteer-extra-plugin-user-data-dir@2.2.13
 - puppeteer-extra-plugin-user-preferences@2.2.13

cc @LinkTree3 @Dam998

@berstend
Copy link
Owner

berstend commented Jul 8, 2022

More properly fixed in #656

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug report A bug has been reported needs triage work-in-progress This is currently being worked on
Projects
None yet
Development

No branches or pull requests

10 participants