fix: skip proxy-chain (#53)

szmarczak · web-flow · commit 6e4cd09c39bc · 2021-10-17T23:07:37.000+02:00
diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml
@@ -15,7 +15,7 @@ jobs:
         strategy:
             matrix:
                 os: [ubuntu-latest, windows-latest]
-                node-version: [10, 12, 14]
+                node-version: [15, 16]
 
         steps:
             -
@@ -43,10 +43,10 @@ jobs:
             -
                 uses: actions/checkout@v2
             -
-                name: Use Node.js 14
+                name: Use Node.js 16
                 uses: actions/setup-node@v1
                 with:
-                    node-version: 14
+                    node-version: 16
             -
                 name: Install Dependencies
                 run: npm install
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -19,7 +19,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest, windows-latest]
-        node-version: [10, 12, 14]
+        node-version: [15, 16]
 
     steps:
       -
@@ -47,10 +47,10 @@ jobs:
       -
         uses: actions/checkout@v2
       -
-        name: Use Node.js 14
+        name: Use Node.js 16
         uses: actions/setup-node@v1
         with:
-          node-version: 14
+          node-version: 16
       -
         name: Install Dependencies
         run: npm install
@@ -70,7 +70,7 @@ jobs:
       -
         uses: actions/setup-node@v1
         with:
-          node-version: 14
+          node-version: 16
           registry-url: https://registry.npmjs.org/
       -
         name: Install Dependencies
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+3.0.0 / 2021/10/17
+====================
+- fix: skip proxy-chain #53
+
 2.0.4 / 2021/10/05
 ====================
 - fix `proxyUrl` in incognito context (#51), closes apify/apify-js#1195
diff --git a/package.json b/package.json
@@ -1,7 +1,10 @@
 {
 	"name": "browser-pool",
-	"version": "2.0.4",
+	"version": "3.0.0",
 	"description": "Rotate multiple browsers using popular automation libraries such as Playwright or Puppeteer.",
+    "engines": {
+        "node": ">=15.10.0"
+    },
 	"main": "dist/index.js",
 	"module": "dist/index.mjs",
 	"types": "dist/index.d.ts",
@@ -45,7 +48,6 @@
 		"nanoid": "^3.1.25",
 		"ow": "^0.27.0",
 		"p-limit": "^3.1.0",
-		"proxy-chain": "^1.0.2",
 		"tiny-typed-emitter": "^2.1.0",
 		"tslib": "^2.3.1"
 	},
@@ -66,6 +68,7 @@
 		"jsdoc-to-markdown": "^7.0.1",
 		"markdown-toc": "^1.2.0",
 		"playwright": "^1.13.1",
+		"proxy-chain": "^2.0.0-beta.0",
 		"puppeteer": "^10.4.0",
 		"rimraf": "^3.0.2",
 		"ts-jest": "^27.0.4",
diff --git a/src/abstract-classes/browser-controller.ts b/src/abstract-classes/browser-controller.ts
@@ -112,8 +112,6 @@ export abstract class BrowserController<
 
     isActive = false;
 
-    supportsPageOptions = false;
-
     activePages = 0;
 
     totalPages = 0;
diff --git a/src/abstract-classes/browser-plugin.ts b/src/abstract-classes/browser-plugin.ts
@@ -1,13 +1,9 @@
 import merge from 'lodash.merge';
-import { log } from '../logger';
 import { LaunchContext, LaunchContextOptions } from '../launch-context';
 import { BrowserController } from './browser-controller';
 import { throwImplementationNeeded } from './utils';
 import { UnwrapPromise } from '../utils';
 
-// eslint-disable-next-line @typescript-eslint/no-var-requires -- TODO: Type this module, and convert import
-const proxyChain = require('proxy-chain');
-
 /**
  * Each plugin expects an instance of the object with the `.launch()` property.
  * For Puppeteer, it is the `puppeteer` module itself, whereas for Playwright
@@ -177,44 +173,4 @@ export abstract class BrowserPlugin<
     protected abstract _createController(): BrowserController<Library, LibraryOptions, LaunchResult, NewPageOptions, NewPageResult> {
         throwImplementationNeeded('_createController');
     }
-
-    /**
-     * Starts proxy-chain server - https://www.npmjs.com/package/proxy-chain#anonymizeproxyproxyurl-callback
-     * @param proxyUrl Proxy URL with username and password.
-     * @return URL of the anonymization proxy server that needs to be closed after the proxy is not used anymore.
-     * @private
-     */
-    protected async _getAnonymizedProxyUrl(proxyUrl: string): Promise<string> {
-        try {
-            return await proxyChain.anonymizeProxy(proxyUrl);
-        } catch (e) {
-            throw new Error(`BrowserPool: Could not anonymize proxyUrl: ${proxyUrl}. Reason: ${(e as Error).message}.`);
-        }
-    }
-
-    /**
-     * @param proxyUrl Anonymized proxy URL of a running proxy server.
-     * @private
-     */
-    protected async _closeAnonymizedProxy(proxyUrl: string): Promise<boolean> {
-        return proxyChain.closeAnonymizedProxy(proxyUrl, true).catch((err: Error) => {
-            log.debug(`Could not close anonymized proxy server.\nCause:${err.message}`);
-        });
-    }
-
-    /**
-     * Checks if proxy URL should be anonymized.
-     * @private
-     */
-    protected _shouldAnonymizeProxy(proxyUrl: string): boolean {
-        const parsedProxyUrl = proxyChain.parseUrl(proxyUrl);
-        if (parsedProxyUrl.username || parsedProxyUrl.password) {
-            if (parsedProxyUrl.scheme !== 'http') {
-                throw new Error('Invalid "proxyUrl" option: authentication is only supported for HTTP proxy type.');
-            }
-            return true;
-        }
-
-        return false;
-    }
 }
diff --git a/src/browser-pool.ts b/src/browser-pool.ts
@@ -451,7 +451,7 @@ export class BrowserPool<
         // eslint-disable-next-line dot-notation -- accessing private property
         await browserController['isActivePromise'];
 
-        const finalPageOptions = browserController.supportsPageOptions ? pageOptions : undefined;
+        const finalPageOptions = browserController.launchContext.useIncognitoPages ? pageOptions : undefined;
         await this._executeHooks(this.prePageCreateHooks, pageId, browserController, finalPageOptions);
 
         let page: PageReturn;
diff --git a/src/playwright/playwright-controller.ts b/src/playwright/playwright-controller.ts
@@ -2,10 +2,12 @@ import type { Browser, BrowserType, Page } from 'playwright';
 import { BrowserController, Cookie } from '../abstract-classes/browser-controller';
 
 export class PlaywrightController extends BrowserController<BrowserType, Parameters<BrowserType['launch']>[0], Browser> {
-    override supportsPageOptions = true;
+    protected async _newPage(contextOptions?: Parameters<Browser['newPage']>[0]): Promise<Page> {
+        if (contextOptions !== undefined && !this.launchContext.useIncognitoPages) {
+            throw new Error('A new page can be created with provided context only when using incognito pages.');
+        }
 
-    protected async _newPage(pageOptions?: Parameters<Browser['newPage']>[0]): Promise<Page> {
-        const page = await this.browser.newPage(pageOptions);
+        const page = await this.browser.newPage(contextOptions);
 
         page.once('close', () => {
             this.activePages--;
diff --git a/src/playwright/playwright-plugin.ts b/src/playwright/playwright-plugin.ts
@@ -12,7 +12,6 @@ export class PlaywrightPlugin extends BrowserPlugin<BrowserType, Parameters<Brow
     protected async _launch(launchContext: LaunchContext<BrowserType>): Promise<PlaywrightBrowser> {
         const {
             launchOptions,
-            anonymizedProxyUrl,
             useIncognitoPages,
             userDataDir,
         } = launchContext;
@@ -36,12 +35,6 @@ export class PlaywrightPlugin extends BrowserPlugin<BrowserType, Parameters<Brow
             browser = new PlaywrightBrowserWithPersistentContext({ browserContext, version: this._browserVersion });
         }
 
-        if (anonymizedProxyUrl) {
-            browser.once('disconnected', () => {
-                this._closeAnonymizedProxy(anonymizedProxyUrl as string);
-            });
-        }
-
         return browser;
     }
 
@@ -55,13 +48,13 @@ export class PlaywrightPlugin extends BrowserPlugin<BrowserType, Parameters<Brow
         const { launchOptions, proxyUrl } = launchContext;
 
         if (proxyUrl) {
-            if (this._shouldAnonymizeProxy(proxyUrl)) {
-                const anonymizedProxyUrl = await this._getAnonymizedProxyUrl(proxyUrl);
-                launchContext.anonymizedProxyUrl = anonymizedProxyUrl;
-                launchOptions.proxy = { server: anonymizedProxyUrl };
-            } else {
-                launchOptions.proxy = { server: proxyUrl };
-            }
+            const url = new URL(proxyUrl);
+
+            launchOptions.proxy = {
+                server: url.origin,
+                username: decodeURIComponent(url.username),
+                password: decodeURIComponent(url.password),
+            };
         }
     }
 }
diff --git a/src/puppeteer/puppeteer-controller.ts b/src/puppeteer/puppeteer-controller.ts
@@ -1,10 +1,44 @@
-import type * as Puppeteer from 'puppeteer';
+import type Puppeteer from 'puppeteer';
 import { BrowserController, Cookie } from '../abstract-classes/browser-controller';
+import { log } from '../logger';
 
 const PROCESS_KILL_TIMEOUT_MILLIS = 5000;
 
+interface ContextOptions extends Puppeteer.BrowserContextOptions {
+    proxyUsername?: string;
+    proxyPassword?: string;
+}
+
 export class PuppeteerController extends BrowserController<typeof Puppeteer> {
-    protected async _newPage(): Promise<Puppeteer.Page> {
+    protected async _newPage(contextOptions?: ContextOptions): Promise<Puppeteer.Page> {
+        if (contextOptions !== undefined) {
+            if (!this.launchContext.useIncognitoPages) {
+                throw new Error('A new page can be created with provided context only when using incognito pages.');
+            }
+
+            const context = await this.browser.createIncognitoBrowserContext(contextOptions);
+            const page = await context.newPage();
+
+            if (contextOptions.proxyUsername || contextOptions.proxyPassword) {
+                await page.authenticate({
+                    username: contextOptions.proxyUsername ?? '',
+                    password: contextOptions.proxyPassword ?? '',
+                });
+            }
+
+            page.once('close', async () => {
+                this.activePages--;
+
+                try {
+                    await context.close();
+                } catch (error: any) {
+                    log.exception(error, 'Failed to close context.');
+                }
+            });
+
+            return page;
+        }
+
         const page = await this.browser.newPage();
 
         page.once('close', () => {
diff --git a/src/puppeteer/puppeteer-plugin.ts b/src/puppeteer/puppeteer-plugin.ts
@@ -1,4 +1,7 @@
-import type * as Puppeteer from 'puppeteer';
+// eslint isn't compatible with `import type`
+/* eslint-disable import/no-duplicates */
+import type Puppeteer from 'puppeteer';
+import type { Browser, Credentials, Target } from 'puppeteer';
 import { BrowserController } from '../abstract-classes/browser-controller';
 import { BrowserPlugin } from '../abstract-classes/browser-plugin';
 import { LaunchContext } from '../launch-context';
@@ -9,24 +12,23 @@ import { PuppeteerController } from './puppeteer-controller';
 const PROXY_SERVER_ARG = '--proxy-server=';
 
 export class PuppeteerPlugin extends BrowserPlugin<typeof Puppeteer> {
-    protected async _launch(launchContext: LaunchContext<typeof Puppeteer>): Promise<Puppeteer.Browser> {
+    protected async _launch(launchContext: LaunchContext<typeof Puppeteer>): Promise<Browser> {
         const {
             launchOptions,
-            anonymizedProxyUrl,
             userDataDir,
             useIncognitoPages,
+            proxyCredentials,
+            proxyUrl,
         } = launchContext;
 
-        const typedAnonymizedProxyUrl = anonymizedProxyUrl as string | undefined;
-
         const finalLaunchOptions = {
             ...launchOptions,
             userDataDir: launchOptions?.userDataDir ?? userDataDir,
         };
 
         let browser = await this.library.launch(finalLaunchOptions);
 
-        browser.on('targetcreated', async (target: Puppeteer.Target) => {
+        browser.on('targetcreated', async (target: Target) => {
             try {
                 const page = await target.page();
 
@@ -41,29 +43,35 @@ export class PuppeteerPlugin extends BrowserPlugin<typeof Puppeteer> {
             }
         });
 
-        if (useIncognitoPages) {
-            browser = new Proxy(browser, {
-                get: (target, property: keyof typeof browser) => {
-                    if (property === 'newPage') {
-                        return (async (...args) => {
-                            const incognitoContext = await browser.createIncognitoBrowserContext({
-                                proxyServer: typedAnonymizedProxyUrl || launchContext.proxyUrl,
+        const newPage = browser.newPage.bind(browser);
+
+        browser = new Proxy(browser, {
+            get: (target, property: keyof typeof browser) => {
+                if (property === 'newPage') {
+                    return (async (...args) => {
+                        let page: Puppeteer.Page;
+
+                        if (useIncognitoPages) {
+                            const context = await browser.createIncognitoBrowserContext({
+                                proxyServer: proxyUrl,
                             });
 
-                            return incognitoContext.newPage(...args);
-                        }) as typeof browser.newPage;
-                    }
+                            page = await context.newPage(...args);
+                        } else {
+                            page = await newPage(...args);
+                        }
 
-                    return target[property];
-                },
-            });
-        }
+                        if (proxyCredentials) {
+                            await page.authenticate(proxyCredentials as Credentials);
+                        }
 
-        if (typedAnonymizedProxyUrl) {
-            browser.once('disconnected', () => {
-                this._closeAnonymizedProxy(typedAnonymizedProxyUrl);
-            });
-        }
+                        return page;
+                    }) as typeof browser.newPage;
+                }
+
+                return target[property];
+            },
+        });
 
         return browser;
     }
@@ -78,19 +86,24 @@ export class PuppeteerPlugin extends BrowserPlugin<typeof Puppeteer> {
         launchContext.launchOptions ??= {};
 
         const { launchOptions, proxyUrl } = launchContext;
-        let finalProxyUrl = proxyUrl;
 
-        if (proxyUrl && this._shouldAnonymizeProxy(proxyUrl)) {
-            finalProxyUrl = await this._getAnonymizedProxyUrl(proxyUrl);
-            launchContext.anonymizedProxyUrl = finalProxyUrl;
-        }
+        if (proxyUrl) {
+            const url = new URL(proxyUrl);
 
-        const proxyArg = `${PROXY_SERVER_ARG}${finalProxyUrl}`;
+            if (url.username || url.password) {
+                launchContext.proxyCredentials = {
+                    username: decodeURIComponent(url.username),
+                    password: decodeURIComponent(url.password),
+                };
+            }
+
+            const proxyArg = `${PROXY_SERVER_ARG}${url.origin}`;
 
-        if (Array.isArray(launchOptions.args)) {
-            launchOptions.args.push(proxyArg);
-        } else {
-            launchOptions.args = [proxyArg];
+            if (Array.isArray(launchOptions.args)) {
+                launchOptions.args.push(proxyArg);
+            } else {
+                launchOptions.args = [proxyArg];
+            }
         }
     }
 }
diff --git a/test/browser-plugins/create-proxy-server.ts b/test/browser-plugins/create-proxy-server.ts
diff --git a/test/browser-plugins/plugins.test.ts b/test/browser-plugins/plugins.test.ts
diff --git a/test/browser-pool.test.ts b/test/browser-pool.test.ts

Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ jobs:`
`15`	`15`	`strategy:`
`16`	`16`	`matrix:`
`17`	`17`	`os: [ubuntu-latest, windows-latest]`
`18`		`- node-version: [10, 12, 14]`
	`18`	`+ node-version: [15, 16]`
`19`	`19`
`20`	`20`	`steps:`
`21`	`21`	`-`
`@@ -43,10 +43,10 @@ jobs:`
`43`	`43`	`-`
`44`	`44`	`uses: actions/checkout@v2`
`45`	`45`	`-`
`46`		`- name: Use Node.js 14`
	`46`	`+ name: Use Node.js 16`
`47`	`47`	`uses: actions/setup-node@v1`
`48`	`48`	`with:`
`49`		`- node-version: 14`
	`49`	`+ node-version: 16`
`50`	`50`	`-`
`51`	`51`	`name: Install Dependencies`
`52`	`52`	`run: npm install`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ jobs:`
`19`	`19`	`strategy:`
`20`	`20`	`matrix:`
`21`	`21`	`os: [ubuntu-latest, windows-latest]`
`22`		`- node-version: [10, 12, 14]`
	`22`	`+ node-version: [15, 16]`
`23`	`23`
`24`	`24`	`steps:`
`25`	`25`	`-`
`@@ -47,10 +47,10 @@ jobs:`
`47`	`47`	`-`
`48`	`48`	`uses: actions/checkout@v2`
`49`	`49`	`-`
`50`		`- name: Use Node.js 14`
	`50`	`+ name: Use Node.js 16`
`51`	`51`	`uses: actions/setup-node@v1`
`52`	`52`	`with:`
`53`		`- node-version: 14`
	`53`	`+ node-version: 16`
`54`	`54`	`-`
`55`	`55`	`name: Install Dependencies`
`56`	`56`	`run: npm install`
`@@ -70,7 +70,7 @@ jobs:`
`70`	`70`	`-`
`71`	`71`	`uses: actions/setup-node@v1`
`72`	`72`	`with:`
`73`		`- node-version: 14`
	`73`	`+ node-version: 16`
`74`	`74`	`registry-url: https://registry.npmjs.org/`
`75`	`75`	`-`
`76`	`76`	`name: Install Dependencies`