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

fix(plugin-user-data-dir): Use onDisconnect instead of onClose #530

Merged
merged 1 commit into from Sep 17, 2021

Conversation

dev-hyperweb
Copy link
Contributor

@dev-hyperweb dev-hyperweb commented Jul 22, 2021

puppeteer-extra-plugin-stealth
puppeteer-extra-plugin : user-agent-override

missing "onDisconnected" event will make application freeze
( Stuct in onClose event )

And can't use "onClose" because it's make freeze too

  • Deprecated: Since puppeteer v1.6.0 onDisconnected has been improved
    • and should be used instead of onClose.
    • In puppeteer < v1.6.0 onDisconnected was not catching all exit scenarios.

also relate problem has mention it on other relate problem too
pluginStealth.enabledEvasions.delete('user-agent-override');
this short fix, but we need "user-agent-override" to work better than disable

I test fixed by this code

const express = require('express'); // Adding Express
const crypto = require('crypto');
const app = express(); // Initializing Express
var bodyParser = require('body-parser');
app.use(bodyParser.urlencoded({ extended: false }));
const puppeteer = require('puppeteer-extra'); // Adding Puppeteer
const pluginStealth = require('puppeteer-extra-plugin-stealth')();
puppeteer.use(pluginStealth);

const app_dir = __dirname;

const PORT_USAGE = '8017';
var url_target = "https://google.com";
var gname = "test_session";
app.get('/', function(req, res) {
	;(async () => {
  // Launch the browser in headless mode and set up a page.
		puppeteer.launch(
		{
			args: ['--no-sandbox', '--disable-setuid-sandbox'],
			ignoreHTTPSErrors: true,
			/*executablePath: '/usr/bin/chromium-browser',*/
			dumpio: false,
			headless: true,
			userDataDir: app_dir + "/cookies_" + gname + ".cookie",
		}
		).then(async function(browser) {
			const page = await browser.newPage();
			await page.setUserAgent("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.87 Safari/537.36");
			await page.goto(url_target,{
				waitUntil: "networkidle2",
				timeout: 10 * 1000
			});

			await page.setRequestInterception(true);
			await browser.close();
			res.json({ status:process.pid })
			return res.end();

		});
})()

});
app.listen(PORT_USAGE, '127.0.0.1', function() {
	console.log(new Date().toString(),'Running on port ' + PORT_USAGE);
});

and request http to "127.0.0.1:8017"

It can close application without force killing

@github-actions github-actions bot added plugin: puppeteer-extra PuppeteerExtra Plugin related plugin: stealth ㊙️ Detection evasion related labels Jul 22, 2021
@dev-hyperweb dev-hyperweb marked this pull request as ready for review July 22, 2021 13:11
@dev-hyperweb dev-hyperweb changed the title fix freeze application fix freeze application puppeteer-extra-plugin-stealth Jul 22, 2021
@dev-hyperweb
Copy link
Contributor Author

dev-hyperweb commented Jul 23, 2021

Why fail test, but in my fork it pass :(
Test

look like I forgot remove unused workflow in my fork, Let's try again

this cause by browser.close() event
missing "onDisconnected" event will make application freeze

And can't use "onClose" because it's make freeze too
* **Deprecated:** Since puppeteer v1.6.0 `onDisconnected` has been improved
   * and should be used instead of `onClose`.
   *
   * In puppeteer < v1.6.0 `onDisconnected` was not catching all exit scenarios.

also relate problem has mention it'
pluginStealth.enabledEvasions.delete('user-agent-override');

I test fixed by this code
```
const express = require('express'); // Adding Express
const crypto = require('crypto');
const app = express(); // Initializing Express
var bodyParser = require('body-parser');
app.use(bodyParser.urlencoded({ extended: false }));
const puppeteer = require('puppeteer-extra'); // Adding Puppeteer
const pluginStealth = require('puppeteer-extra-plugin-stealth')();
puppeteer.use(pluginStealth);

const app_dir = __dirname;

const PORT_USAGE = '8017';
var url_target = "https://google.com";
var gname = "test_session";
app.get('/', function(req, res) {
	;(async () => {
  // Launch the browser in headless mode and set up a page.
		puppeteer.launch(
		{
			args: ['--no-sandbox', '--disable-setuid-sandbox'],
			ignoreHTTPSErrors: true,
			/*executablePath: '/usr/bin/chromium-browser',*/
			dumpio: false,
			headless: true,
			userDataDir: app_dir + "/cookies_" + gname + ".cookie",
		}
		).then(async function(browser) {
			const page = await browser.newPage();
			await page.setUserAgent("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.87 Safari/537.36");
			await page.goto(url_target,{
				waitUntil: "networkidle2",
				timeout: 10 * 1000
			});

			await page.setRequestInterception(true);
			await browser.close();
			res.json({ status:process.pid })
			return res.end();

		});
})()

});
app.listen(8017, '127.0.0.1', function() {
	console.log(new Date().toString(),'Running on port ' + PORT_USAGE);
});
```

It can close application without force killing

Update index.js

3252d5c

maybe not need onDisconnected event handle in this file
@Niek Niek requested a review from berstend August 9, 2021 09:58
@Niek Niek changed the title fix freeze application puppeteer-extra-plugin-stealth Fix 421: fix freeze application puppeteer-extra-plugin-stealth Aug 9, 2021
@Niek Niek changed the title Fix 421: fix freeze application puppeteer-extra-plugin-stealth Fix #421: fix freeze application puppeteer-extra-plugin-stealth Aug 9, 2021
@Niek
Copy link
Collaborator

Niek commented Aug 9, 2021

@dev-hyperweb can you look at the failing tests?

Edit: seems like it was a fluke, this PR LGTM now

@alamothe
Copy link

alamothe commented Sep 9, 2021

Any updates?

@dev-hyperweb
Copy link
Contributor Author

maybe this solution need test with other plugins to makesure it's not conflic

but I don't have time to test.

@dev-hyperweb
Copy link
Contributor Author

got solves with this solution
#421 (comment)

maybe closed this PR

@berstend
Copy link
Owner

got solves with this solution
#421 (comment)

maybe closed this PR

Apparently it's not solved with the fix in this comment, changing to onDisconnect might be the right way - onClose is pretty aggressively hooking into a lot of events which might be related to the issues on windows:

if (opts.context === 'launch' && this._hasChildClassMember('onClose')) {
// The disconnect event has been improved since puppeteer v1.6.0
// onClose is being kept mostly for legacy reasons
if (this.onClose) {
process.on('exit', this.onClose.bind(this))
browser.on('disconnected', this.onClose.bind(this))
if (opts.options.handleSIGINT !== false) {
process.on('SIGINT', this.onClose.bind(this))
}
if (opts.options.handleSIGTERM !== false) {
process.on('SIGTERM', this.onClose.bind(this))
}
if (opts.options.handleSIGHUP !== false) {
process.on('SIGHUP', this.onClose.bind(this))
}
}
}

@dev-hyperweb
Copy link
Contributor Author

I don't know why fail test
evasions/iframe.contentWindow/index.test.js:289

maybe cause by evasion ?
I don't have time to check more issued about fail test.

Copy link
Owner

@berstend berstend left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We confirmed in testing this seems to fix the windows bug, the failing test seems like a fluke to me

@berstend berstend changed the title Fix #421: fix freeze application puppeteer-extra-plugin-stealth fix(plugin-user-data-dir): Use onDisconnect instead of onClose, fixes #421 Sep 17, 2021
@berstend berstend changed the title fix(plugin-user-data-dir): Use onDisconnect instead of onClose, fixes #421 fix(plugin-user-data-dir): Use onDisconnect instead of onClose Sep 17, 2021
@berstend berstend merged commit 713f9fa into berstend:master Sep 17, 2021
@berstend
Copy link
Owner

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

thanks @dev-hyperweb 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: puppeteer-extra PuppeteerExtra Plugin related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants