cherry-pick(#21104): chore: consolidate http/https fetching (#21284)

Fixes #21227 Fixes #20784 Supersedes #21076
microsoft · Mar 1, 2023 · e9fe663 · e9fe663
1 parent 249825f
commit e9fe663
Show file tree

Hide file tree

Showing 7 changed files with 54 additions and 40 deletions.
diff --git a/packages/playwright-core/src/server/chromium/chromium.ts b/packages/playwright-core/src/server/chromium/chromium.ts
@@ -33,25 +33,22 @@ import { Browser } from '../browser';
 import type * as types from '../types';
 import type * as channels from '@protocol/channels';
 import type { HTTPRequestParams } from '../../utils/network';
-import { NET_DEFAULT_TIMEOUT } from '../../utils/network';
 import { fetchData } from '../../utils/network';
 import { getUserAgent } from '../../utils/userAgent';
 import { wrapInASCIIBox } from '../../utils/ascii';
-import { debugMode, headersArrayToObject,  } from '../../utils';
+import { debugMode, headersArrayToObject, } from '../../utils';
 import { removeFolders } from '../../utils/fileUtils';
 import { RecentLogsCollector } from '../../common/debugLogger';
 import type { Progress } from '../progress';
 import { ProgressController } from '../progress';
 import { TimeoutSettings } from '../../common/timeoutSettings';
 import { helper } from '../helper';
 import type { CallMetadata } from '../instrumentation';
-import http from 'http';
-import https from 'https';
+import type http from 'http';
 import { registry } from '../registry';
 import { ManualPromise } from '../../utils/manualPromise';
 import { validateBrowserContextOptions } from '../browserContext';
 import { chromiumSwitches } from './chromiumSwitches';
-import { httpHappyEyeballsAgent, httpsHappyEyeballsAgent } from '../happy-eyeballs';
 
 const ARTIFACTS_FOLDER = path.join(os.tmpdir(), 'playwright-artifacts-');
 
@@ -338,21 +335,11 @@ async function urlToWSEndpoint(progress: Progress, endpointURL: string) {
     return endpointURL;
   progress.log(`<ws preparing> retrieving websocket url from ${endpointURL}`);
   const httpURL = endpointURL.endsWith('/') ? `${endpointURL}json/version/` : `${endpointURL}/json/version/`;
-  const isHTTPS = endpointURL.startsWith('https://');
-  const json = await new Promise<string>((resolve, reject) => {
-    (isHTTPS ? https : http).get(httpURL, {
-      timeout: NET_DEFAULT_TIMEOUT,
-      agent: isHTTPS ? httpsHappyEyeballsAgent : httpHappyEyeballsAgent,
-    }, resp => {
-      if (resp.statusCode! < 200 || resp.statusCode! >= 400) {
-        reject(new Error(`Unexpected status ${resp.statusCode} when connecting to ${httpURL}.\n` +
-        `This does not look like a DevTools server, try connecting via ws://.`));
-      }
-      let data = '';
-      resp.on('data', chunk => data += chunk);
-      resp.on('end', () => resolve(data));
-    }).on('error', reject);
-  });
+  const json = await fetchData({
+    url: httpURL,
+  }, async (_, resp) => new Error(`Unexpected status ${resp.statusCode} when connecting to ${httpURL}.\n` +
+    `This does not look like a DevTools server, try connecting via ws://.`)
+  );
   return JSON.parse(json).webSocketDebuggerUrl;
 }
 

diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts
@@ -30,7 +30,7 @@ import { HttpsProxyAgent, SocksProxyAgent } from '../utilsBundle';
 import { BrowserContext } from './browserContext';
 import { CookieStore, domainMatches } from './cookieStore';
 import { MultipartFormData } from './formData';
-import { httpHappyEyeballsAgent, httpsHappyEyeballsAgent } from './happy-eyeballs';
+import { httpHappyEyeballsAgent, httpsHappyEyeballsAgent } from '../utils/happy-eyeballs';
 import type { CallMetadata } from './instrumentation';
 import { SdkObject } from './instrumentation';
 import type { Playwright } from './playwright';
@@ -69,7 +69,7 @@ export type APIRequestFinishedEvent = {
   body?: Buffer;
 };
 
-export type SendRequestOptions = https.RequestOptions & {
+type SendRequestOptions = https.RequestOptions & {
   maxRedirects: number,
   deadline: number,
   __testHookLookup?: (hostname: string) => LookupAddress[]

diff --git a/packages/playwright-core/src/server/transport.ts b/packages/playwright-core/src/server/transport.ts
@@ -20,7 +20,7 @@ import type { WebSocket } from '../utilsBundle';
 import type { ClientRequest, IncomingMessage } from 'http';
 import type { Progress } from './progress';
 import { makeWaitForNextTask } from '../utils';
-import { httpHappyEyeballsAgent, httpsHappyEyeballsAgent } from './happy-eyeballs';
+import { httpHappyEyeballsAgent, httpsHappyEyeballsAgent } from '../utils/happy-eyeballs';
 
 export type ProtocolRequest = {
   id: number;

diff --git a/...ywright-core/src/server/happy-eyeballs.ts → ...aywright-core/src/utils/happy-eyeballs.ts b/...ywright-core/src/server/happy-eyeballs.ts → ...aywright-core/src/utils/happy-eyeballs.ts
@@ -19,8 +19,7 @@ import * as http from 'http';
 import * as https from 'https';
 import * as net from 'net';
 import * as tls from 'tls';
-import { ManualPromise } from '../utils/manualPromise';
-import type { SendRequestOptions } from './fetch';
+import { ManualPromise } from './manualPromise';
 
 // Implementation(partial) of Happy Eyeballs 2 algorithm described in
 // https://www.rfc-editor.org/rfc/rfc8305
@@ -50,7 +49,7 @@ export const httpsHappyEyeballsAgent = new HttpsHappyEyeballsAgent();
 export const httpHappyEyeballsAgent = new HttpHappyEyeballsAgent();
 
 async function createConnectionAsync(options: http.ClientRequestArgs, oncreate: ((err: Error | null, socket?: net.Socket) => void) | undefined, useTLS: boolean) {
-  const lookup = (options as SendRequestOptions).__testHookLookup || lookupAddresses;
+  const lookup = (options as any).__testHookLookup || lookupAddresses;
   const hostname = clientRequestArgsToHostName(options);
   const addresses = await lookup(hostname);
   const sockets = new Set<net.Socket>();

diff --git a/packages/playwright-core/src/utils/network.ts b/packages/playwright-core/src/utils/network.ts
@@ -24,6 +24,7 @@ import * as URL from 'url';
 import type { URLMatch } from '../common/types';
 import { isString, isRegExp } from './rtti';
 import { globToRegex } from './glob';
+import { httpHappyEyeballsAgent, httpsHappyEyeballsAgent } from './happy-eyeballs';
 
 export async function createSocket(host: string, port: number): Promise<net.Socket> {
   return new Promise((resolve, reject) => {
@@ -39,15 +40,22 @@ export type HTTPRequestParams = {
   headers?: http.OutgoingHttpHeaders,
   data?: string | Buffer,
   timeout?: number,
+  rejectUnauthorized?: boolean,
 };
 
 export const NET_DEFAULT_TIMEOUT = 30_000;
 
 export function httpRequest(params: HTTPRequestParams, onResponse: (r: http.IncomingMessage) => void, onError: (error: Error) => void) {
   const parsedUrl = URL.parse(params.url);
-  let options: https.RequestOptions = { ...parsedUrl };
-  options.method = params.method || 'GET';
-  options.headers = params.headers;
+  let options: https.RequestOptions = {
+    ...parsedUrl,
+    agent: parsedUrl.protocol === 'https:' ? httpsHappyEyeballsAgent : httpHappyEyeballsAgent,
+    method: params.method || 'GET',
+    headers: params.headers,
+  };
+  if (params.rejectUnauthorized !== undefined)
+    options.rejectUnauthorized = params.rejectUnauthorized;
+
   const timeout = params.timeout ?? NET_DEFAULT_TIMEOUT;
 
   const proxyURL = getProxyForUrl(params.url);

diff --git a/packages/playwright-test/src/plugins/webServerPlugin.ts b/packages/playwright-test/src/plugins/webServerPlugin.ts
@@ -13,13 +13,11 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import http from 'http';
-import https from 'https';
 import path from 'path';
 import net from 'net';
 
 import { debug } from 'playwright-core/lib/utilsBundle';
-import { raceAgainstTimeout, launchProcess } from 'playwright-core/lib/utils';
+import { raceAgainstTimeout, launchProcess, httpRequest } from 'playwright-core/lib/utils';
 
 import type { FullConfig, Reporter } from '../../types/testReporter';
 import type { TestRunnerPlugin } from '.';
@@ -159,20 +157,18 @@ async function isURLAvailable(url: URL, ignoreHTTPSErrors: boolean, onStdErr: Re
 }
 
 async function httpStatusCode(url: URL, ignoreHTTPSErrors: boolean, onStdErr: Reporter['onStdErr']): Promise<number> {
-  const commonRequestOptions = { headers: { Accept: '*/*' } };
-  const isHttps = url.protocol === 'https:';
-  const requestOptions = isHttps ? {
-    ...commonRequestOptions,
-    rejectUnauthorized: !ignoreHTTPSErrors,
-  } : commonRequestOptions;
   return new Promise(resolve => {
     debugWebServer(`HTTP GET: ${url}`);
-    (isHttps ? https : http).get(url, requestOptions, res => {
+    httpRequest({
+      url: url.toString(),
+      headers: { Accept: '*/*' },
+      rejectUnauthorized: !ignoreHTTPSErrors
+    }, res => {
       res.resume();
       const statusCode = res.statusCode ?? 0;
       debugWebServer(`HTTP Status: ${statusCode}`);
       resolve(statusCode);
-    }).on('error', error => {
+    }, error => {
       if ((error as NodeJS.ErrnoException).code === 'DEPTH_ZERO_SELF_SIGNED_CERT')
         onStdErr?.(`[WebServer] Self-signed certificate detected. Try adding ignoreHTTPSErrors: true to config.webServer.`);
       debugWebServer(`Error while checking if ${url} is available: ${error.message}`);

diff --git a/tests/playwright-test/web-server.spec.ts b/tests/playwright-test/web-server.spec.ts
@@ -608,3 +608,27 @@ test('should treat 3XX as available server', async ({ runInlineTest }, { workerI
   expect(result.output).toContain('[WebServer] listening');
   expect(result.output).toContain('[WebServer] error from server');
 });
+
+test('should check ipv4 and ipv6 with happy eyeballs when URL is passed', async ({ runInlineTest }, { workerIndex }) => {
+  test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/20784' });
+  const port = workerIndex * 2 + 10500;
+  const result = await runInlineTest({
+    'test.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      test('pass', async ({}) => {});
+    `,
+    'playwright.config.ts': `
+      module.exports = {
+        webServer: {
+          command: 'node -e "require(\\'http\\').createServer((req, res) => res.end()).listen(${port}, \\'127.0.0.1\\')"',
+          url: 'http://localhost:${port}/',
+        }
+      };
+    `,
+  }, {}, { DEBUG: 'pw:webserver' });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(1);
+  expect(result.output).toContain('Process started');
+  expect(result.output).toContain(`HTTP GET: http://localhost:${port}/`);
+  expect(result.output).toContain('WebServer available');
+});