chore: only throw the proxy on launch required on win/CR (#6350)

microsoft · Apr 29, 2021 · 1c40c94 · 1c40c94
1 parent 263a0fd
commit 1c40c94
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 12 deletions.
diff --git a/docs/src/api/params.md b/docs/src/api/params.md
@@ -447,9 +447,13 @@ Actual picture of each page will be scaled down if necessary to fit the specifie
   - `username` <[string]> Optional username to use if HTTP proxy requires authentication.
   - `password` <[string]> Optional password to use if HTTP proxy requires authentication.
 
-Network proxy settings to use with this context. Note that browser needs to be launched with the global proxy for this
-option to work. If all contexts override the proxy, global proxy will be never used and can be any string, for example
-`launch({ proxy: { server: 'per-context' } })`.
+Network proxy settings to use with this context.
+
+:::note
+For Chromium on Windows the browser needs to be launched with the global proxy for this option to work. If all
+contexts override the proxy, global proxy will be never used and can be any string, for example
+`launch({ proxy: { server: 'http://per-context' } })`.
+:::
 
 ## select-options-values
 * langs: java, js

diff --git a/src/server/browserContext.ts b/src/server/browserContext.ts
@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import * as os from 'os';
 import { TimeoutSettings } from '../utils/timeoutSettings';
 import { debugMode, mkdirIfNeeded, createGuid } from '../utils/utils';
 import { Browser, BrowserOptions } from './browser';
@@ -405,8 +406,8 @@ export function validateBrowserContextOptions(options: types.BrowserContextOptio
     options.recordVideo.size!.height &= ~1;
   }
   if (options.proxy) {
-    if (!browserOptions.proxy)
-      throw new Error(`Browser needs to be launched with the global proxy. If all contexts override the proxy, global proxy will be never used and can be any string, for example "launch({ proxy: { server: 'per-context' } })"`);
+    if (!browserOptions.proxy && browserOptions.isChromium && os.platform() === 'win32')
+      throw new Error(`Browser needs to be launched with the global proxy. If all contexts override the proxy, global proxy will be never used and can be any string, for example "launch({ proxy: { server: 'http://per-context' } })"`);
     options.proxy = normalizeProxySettings(options.proxy);
   }
   if (debugMode() === 'inspector')

diff --git a/tests/browsercontext-proxy.spec.ts b/tests/browsercontext-proxy.spec.ts
@@ -26,14 +26,37 @@ it.afterAll(async () => {
   await browser.close();
 });
 
-it('should throw for missing global proxy', async ({ browserType, browserOptions, server }) => {
+it('should throw for missing global proxy on Chromium Windows', async ({ browserName, platform, browserType, browserOptions, server }) => {
+  it.skip(browserName !== 'chromium' || platform !== 'win32');
+
   delete browserOptions.proxy;
   const browser = await browserType.launch(browserOptions);
   const error = await browser.newContext({ proxy: { server: `localhost:${server.PORT}` } }).catch(e => e);
   expect(error.toString()).toContain('Browser needs to be launched with the global proxy');
   await browser.close();
 });
 
+it('should work when passing the proxy only on the context level', async ({browserName, platform, browserType, browserOptions, contextOptions, server}) => {
+  // Currently an upstream bug in the network stack of Chromium which leads that
+  // the wrong proxy gets used in the BrowserContext.
+  it.fixme(browserName === 'chromium' && platform === 'win32');
+
+  server.setRoute('/target.html', async (req, res) => {
+    res.end('<html><title>Served by the proxy</title></html>');
+  });
+  delete browserOptions.proxy;
+  const browserWithoutProxyInLaunch = await browserType.launch(browserOptions);
+  const context = await browserWithoutProxyInLaunch.newContext({
+    ...contextOptions,
+    proxy: { server: `localhost:${server.PORT}` }
+  });
+
+  const page = await context.newPage();
+  await page.goto('http://non-existent.com/target.html');
+  expect(await page.title()).toBe('Served by the proxy');
+  await browserWithoutProxyInLaunch.close();
+});
+
 it('should throw for bad server value', async ({ contextOptions }) => {
   const error = await browser.newContext({
     ...contextOptions,

diff --git a/types/types.d.ts b/types/types.d.ts
@@ -8694,9 +8694,11 @@ export interface Browser extends EventEmitter {
     permissions?: Array<string>;
 
     /**
-     * Network proxy settings to use with this context. Note that browser needs to be launched with the global proxy for this
-     * option to work. If all contexts override the proxy, global proxy will be never used and can be any string, for example
-     * `launch({ proxy: { server: 'per-context' } })`.
+     * Network proxy settings to use with this context.
+     * 
+     * > NOTE: For Chromium on Windows the browser needs to be launched with the global proxy for this option to work. If all
+     * contexts override the proxy, global proxy will be never used and can be any string, for example `launch({ proxy: {
+     * server: 'http://per-context' } })`.
      */
     proxy?: {
       /**
@@ -10545,9 +10547,11 @@ export interface BrowserContextOptions {
   permissions?: Array<string>;
 
   /**
-   * Network proxy settings to use with this context. Note that browser needs to be launched with the global proxy for this
-   * option to work. If all contexts override the proxy, global proxy will be never used and can be any string, for example
-   * `launch({ proxy: { server: 'per-context' } })`.
+   * Network proxy settings to use with this context.
+   * 
+   * > NOTE: For Chromium on Windows the browser needs to be launched with the global proxy for this option to work. If all
+   * contexts override the proxy, global proxy will be never used and can be any string, for example `launch({ proxy: {
+   * server: 'http://per-context' } })`.
    */
   proxy?: {
     /**