From 852a5c234bdb3a9095768cf44f3eabd8518546b8 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 19 Oct 2022 13:06:35 -0700 Subject: [PATCH] feat(install): connection timeout (#18161) - `PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT` for custom timeout. - Bumped default timeout from 10s to 30s. - Inlined `download.ts` to avoid extra plumbing. - Removed optional arguments - we always pass them. - Updated installation docs. Fixes #18156. --- docs/src/browsers.md | 89 +++++++++++++++--- .../src/server/registry/browserFetcher.ts | 51 ++++++++-- .../src/server/registry/download.ts | 92 ------------------- .../src/server/registry/index.ts | 4 +- .../src/server/registry/oopDownloadMain.ts | 29 +++--- ...right-cdn-should-race-with-timeout.spec.ts | 12 ++- 6 files changed, 145 insertions(+), 132 deletions(-) delete mode 100644 packages/playwright-core/src/server/registry/download.ts diff --git a/docs/src/browsers.md b/docs/src/browsers.md index e5560b80a7..cc7ed956fb 100644 --- a/docs/src/browsers.md +++ b/docs/src/browsers.md @@ -125,8 +125,6 @@ organization that uses such policies, it is the easiest to use bundled Chromium you can still opt into stable channels on the bots that are typically free of such restrictions. ## Installing browsers - -### Prerequisites for .NET * langs: csharp To invoke Playwright CLI commands, you need to invoke a PowerShell script: @@ -219,17 +217,17 @@ playwright install ``` ```bash tab=bash-bash lang=java -PLAYWRIGHT_BROWSERS_PATH=$HOME/pw-browsers mvn test +PLAYWRIGHT_BROWSERS_PATH=$HOME/pw-browsers mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" ``` ```batch tab=bash-batch lang=java set PLAYWRIGHT_BROWSERS_PATH=%USERPROFILE%\pw-browsers -mvn test +mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" ``` ```powershell tab=bash-powershell lang=java $env:PLAYWRIGHT_BROWSERS_PATH="$env:USERPROFILE\pw-browsers" -mvn test +mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" ``` ```bash tab=bash-bash lang=csharp @@ -313,7 +311,7 @@ Playwright keeps track of packages that need those browsers and will garbage col Developers can opt-in in this mode via exporting `PLAYWRIGHT_BROWSERS_PATH=$HOME/pw-browsers` in their `.bashrc`. ::: -### Managing browser binaries +### Hermetic install * langs: js You can opt into the hermetic install and place binaries in the local folder: @@ -389,17 +387,17 @@ playwright install ``` ```bash tab=bash-bash lang=java -HTTPS_PROXY=https://192.0.2.1 mvn test +HTTPS_PROXY=https://192.0.2.1 mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" ``` ```batch tab=bash-batch lang=java set HTTPS_PROXY=https://192.0.2.1 -mvn test +mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" ``` ```powershell tab=bash-powershell lang=java $env:HTTPS_PROXY="https://192.0.2.1" -mvn test +mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" ``` ```bash tab=bash-bash lang=csharp @@ -430,6 +428,67 @@ set NODE_EXTRA_CA_CERTS="C:\certs\root.crt" $env:NODE_EXTRA_CA_CERTS="C:\certs\root.crt" ``` +If your network is slow to connect to Playwright browser archive, you can increase the connection timeout in milliseconds with `PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT` environment variable: + +```bash tab=bash-bash lang=js +PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT=120000 npx playwright install +``` + +```batch tab=bash-batch lang=js +set PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT=120000 +npx playwright install +``` + +```powershell tab=bash-powershell lang=js +$env:PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT="120000" +npx playwright install +``` + +```bash tab=bash-bash lang=python +pip install playwright +PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT=120000 playwright install +``` + +```batch tab=bash-batch lang=python +set PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT=120000 +pip install playwright +playwright install +``` + +```powershell tab=bash-powershell lang=python +$env:PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT="120000" +pip install playwright +playwright install +``` + +```bash tab=bash-bash lang=java +PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT=120000 mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" +``` + +```batch tab=bash-batch lang=java +set PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT=120000 +mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" +``` + +```powershell tab=bash-powershell lang=java +$env:PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT="120000" +mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" +``` + +```bash tab=bash-bash lang=csharp +PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT=120000 pwsh bin/Debug/netX/playwright.ps1 install +``` + +```batch tab=bash-batch lang=csharp +set PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT=120000 +pwsh bin/Debug/netX/playwright.ps1 install +``` + +```powershell tab=bash-powershell lang=csharp +$env:PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT="120000" +pwsh bin/Debug/netX/playwright.ps1 install +``` + ## Download from artifact repository By default, Playwright downloads browsers from Microsoft CDN. @@ -484,17 +543,17 @@ playwright install ``` ```bash tab=bash-bash lang=java -PLAYWRIGHT_DOWNLOAD_HOST=192.0.2.1 mvn test +PLAYWRIGHT_DOWNLOAD_HOST=192.0.2.1 mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" ``` ```batch tab=bash-batch lang=java set PLAYWRIGHT_DOWNLOAD_HOST=192.0.2.1 -mvn test +mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" ``` ```powershell tab=bash-powershell lang=java $env:PLAYWRIGHT_DOWNLOAD_HOST="192.0.2.1" -mvn test +mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" ``` ```bash tab=bash-bash lang=csharp @@ -566,19 +625,19 @@ playwright install ``` ```bash tab=bash-bash lang=java -PLAYWRIGHT_FIREFOX_DOWNLOAD_HOST=203.0.113.3 PLAYWRIGHT_DOWNLOAD_HOST=192.0.2.1 mvn test +PLAYWRIGHT_FIREFOX_DOWNLOAD_HOST=203.0.113.3 PLAYWRIGHT_DOWNLOAD_HOST=192.0.2.1 mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" ``` ```batch tab=bash-batch lang=java set PLAYWRIGHT_FIREFOX_DOWNLOAD_HOST=203.0.113.3 set PLAYWRIGHT_DOWNLOAD_HOST=192.0.2.1 -mvn test +mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" ``` ```powershell tab=bash-powershell lang=java $env:PLAYWRIGHT_FIREFOX_DOWNLOAD_HOST="203.0.113.3" $env:PLAYWRIGHT_DOWNLOAD_HOST="192.0.2.1" -mvn test +mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install" ``` ```bash tab=bash-bash lang=csharp diff --git a/packages/playwright-core/src/server/registry/browserFetcher.ts b/packages/playwright-core/src/server/registry/browserFetcher.ts index f312a5eb13..343a528efc 100644 --- a/packages/playwright-core/src/server/registry/browserFetcher.ts +++ b/packages/playwright-core/src/server/registry/browserFetcher.ts @@ -18,13 +18,14 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; +import childProcess from 'child_process'; import { getUserAgent } from '../../common/userAgent'; import { existsAsync } from '../../utils/fileUtils'; import { debugLogger } from '../../common/debugLogger'; -import { download } from './download'; import { extract } from '../../zipBundle'; +import { ManualPromise } from '../../utils/manualPromise'; -export async function downloadBrowserWithProgressBar(title: string, browserDirectory: string, executablePath: string, downloadURLs: string[], downloadFileName: string): Promise { +export async function downloadBrowserWithProgressBar(title: string, browserDirectory: string, executablePath: string, downloadURLs: string[], downloadFileName: string, downloadConnectionTimeout: number): Promise { if (await existsAsync(browserDirectory)) { // Already downloaded. debugLogger.log('install', `${title} is already downloaded.`); @@ -33,11 +34,20 @@ export async function downloadBrowserWithProgressBar(title: string, browserDirec const zipPath = path.join(os.tmpdir(), downloadFileName); try { - await download(downloadURLs, zipPath, { - progressBarName: title, - log: debugLogger.log.bind(debugLogger, 'install'), - userAgent: getUserAgent(), - }); + const retryCount = 3; + for (let attempt = 1; attempt <= retryCount; ++attempt) { + debugLogger.log('install', `downloading ${title} - attempt #${attempt}`); + const url = downloadURLs[(attempt - 1) % downloadURLs.length]; + const { error } = await downloadFileOutOfProcess(url, zipPath, title, getUserAgent(), downloadConnectionTimeout); + if (!error) { + debugLogger.log('install', `SUCCESS downloading ${title}`); + break; + } + const errorMessage = error?.message || ''; + debugLogger.log('install', `attempt #${attempt} - ERROR: ${errorMessage}`); + if (attempt >= retryCount) + throw error; + } debugLogger.log('install', `extracting archive`); debugLogger.log('install', `-- zip: ${zipPath}`); debugLogger.log('install', `-- location: ${browserDirectory}`); @@ -56,6 +66,33 @@ export async function downloadBrowserWithProgressBar(title: string, browserDirec return true; } +/** + * Node.js has a bug where the process can exit with 0 code even though there was an uncaught exception. + * Thats why we execute it in a separate process and check manually if the destination file exists. + * https://github.com/microsoft/playwright/issues/17394 + */ +function downloadFileOutOfProcess(url: string, destinationPath: string, progressBarName: string, userAgent: string, downloadConnectionTimeout: number): Promise<{ error: Error | null }> { + const cp = childProcess.fork(path.join(__dirname, 'oopDownloadMain.js'), [url, destinationPath, progressBarName, userAgent, String(downloadConnectionTimeout)]); + const promise = new ManualPromise<{ error: Error | null }>(); + cp.on('message', (message: any) => { + if (message?.method === 'log') + debugLogger.log('install', message.params.message); + }); + cp.on('exit', code => { + if (code !== 0) { + promise.resolve({ error: new Error(`Download failure, code=${code}`) }); + return; + } + if (!fs.existsSync(destinationPath)) + promise.resolve({ error: new Error(`Download failure, ${destinationPath} does not exist`) }); + else + promise.resolve({ error: null }); + }); + cp.on('error', error => { + promise.resolve({ error }); + }); + return promise; +} export function logPolitely(toBeLogged: string) { const logLevel = process.env.npm_config_loglevel; diff --git a/packages/playwright-core/src/server/registry/download.ts b/packages/playwright-core/src/server/registry/download.ts deleted file mode 100644 index 9174439a2f..0000000000 --- a/packages/playwright-core/src/server/registry/download.ts +++ /dev/null @@ -1,92 +0,0 @@ -/** - * Copyright (c) Microsoft Corporation. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import fs from 'fs'; -import path from 'path'; -import childProcess from 'child_process'; -import { ManualPromise } from '../../utils/manualPromise'; - -type DownloadFileLogger = (message: string) => void; -type DownloadFileOptions = { - progressBarName?: string, - log?: DownloadFileLogger, - userAgent?: string -}; - -/** - * Node.js has a bug where the process can exit with 0 code even though there was an uncaught exception. - * Thats why we execute it in a separate process and check manually if the destination file exists. - * https://github.com/microsoft/playwright/issues/17394 - */ -function downloadFileOutOfProcess(url: string, destinationPath: string, options: DownloadFileOptions = {}): Promise<{ error: Error | null }> { - const cp = childProcess.fork(path.join(__dirname, 'oopDownloadMain.js'), [url, destinationPath, options.progressBarName || '', options.userAgent || '']); - const promise = new ManualPromise<{ error: Error | null }>(); - cp.on('message', (message: any) => { - if (message?.method === 'log') - options.log?.(message.params.message); - }); - cp.on('exit', code => { - if (code !== 0) { - promise.resolve({ error: new Error(`Download failure, code=${code}`) }); - return; - } - if (!fs.existsSync(destinationPath)) - promise.resolve({ error: new Error(`Download failure, ${destinationPath} does not exist`) }); - else - promise.resolve({ error: null }); - }); - cp.on('error', error => { - promise.resolve({ error }); - }); - return promise; -} - -type DownloadOptions = { - progressBarName?: string, - retryCount?: number - log?: DownloadFileLogger - userAgent?: string -}; - -export async function download( - urls: string | string[], - destination: string, - options: DownloadOptions = {} -) { - const { progressBarName = 'file', retryCount = 3, log = () => { }, userAgent } = options; - for (let attempt = 1; attempt <= retryCount; ++attempt) { - log( - `downloading ${progressBarName} - attempt #${attempt}` - ); - if (!Array.isArray(urls)) - urls = [urls]; - const url = urls[(attempt - 1) % urls.length]; - - const { error } = await downloadFileOutOfProcess(url, destination, { - progressBarName, - log, - userAgent, - }); - if (!error) { - log(`SUCCESS downloading ${progressBarName}`); - break; - } - const errorMessage = error?.message || ''; - log(`attempt #${attempt} - ERROR: ${errorMessage}`); - if (attempt >= retryCount) - throw error; - } -} diff --git a/packages/playwright-core/src/server/registry/index.ts b/packages/playwright-core/src/server/registry/index.ts index 8e016413aa..ff008f5873 100644 --- a/packages/playwright-core/src/server/registry/index.ts +++ b/packages/playwright-core/src/server/registry/index.ts @@ -778,7 +778,9 @@ export class Registry { : `${displayName} playwright build v${descriptor.revision}`; const downloadFileName = `playwright-download-${descriptor.name}-${hostPlatform}-${descriptor.revision}.zip`; - await downloadBrowserWithProgressBar(title, descriptor.dir, executablePath, downloadURLs, downloadFileName).catch(e => { + const downloadConnectionTimeoutEnv = getFromENV('PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT'); + const downloadConnectionTimeout = +(downloadConnectionTimeoutEnv || '0') || 30_000; + await downloadBrowserWithProgressBar(title, descriptor.dir, executablePath, downloadURLs, downloadFileName, downloadConnectionTimeout).catch(e => { throw new Error(`Failed to download ${title}, caused by\n${e.stack}`); }); await fs.promises.writeFile(markerFilePath(descriptor.dir), ''); diff --git a/packages/playwright-core/src/server/registry/oopDownloadMain.ts b/packages/playwright-core/src/server/registry/oopDownloadMain.ts index 75972792bf..31465f98f6 100644 --- a/packages/playwright-core/src/server/registry/oopDownloadMain.ts +++ b/packages/playwright-core/src/server/registry/oopDownloadMain.ts @@ -22,12 +22,13 @@ import { ManualPromise } from '../../utils/manualPromise'; type OnProgressCallback = (downloadedBytes: number, totalBytes: number) => void; type DownloadFileLogger = (message: string) => void; type DownloadFileOptions = { - progressCallback?: OnProgressCallback, - log?: DownloadFileLogger, - userAgent?: string + progressCallback: OnProgressCallback, + log: DownloadFileLogger, + userAgent: string, + connectionTimeout: number, }; -function downloadFile(url: string, destinationPath: string, options: DownloadFileOptions = {}): Promise { +function downloadFile(url: string, destinationPath: string, options: DownloadFileOptions): Promise { const { progressCallback, log = () => { }, @@ -42,10 +43,10 @@ function downloadFile(url: string, destinationPath: string, options: DownloadFil httpRequest({ url, - headers: options.userAgent ? { + headers: { 'User-Agent': options.userAgent, - } : undefined, - timeout: 10_000, + }, + timeout: options.connectionTimeout, }, response => { log(`-- response status code: ${response.statusCode}`); if (response.statusCode !== 200) { @@ -68,8 +69,7 @@ function downloadFile(url: string, destinationPath: string, options: DownloadFil response.pipe(file); totalBytes = parseInt(response.headers['content-length'] || '0', 10); log(`-- total bytes: ${totalBytes}`); - if (progressCallback) - response.on('data', onData); + response.on('data', onData); }, (error: any) => promise.reject(error)); return promise; @@ -81,11 +81,11 @@ function downloadFile(url: string, destinationPath: string, options: DownloadFil function getDownloadProgress(progressBarName: string): OnProgressCallback { if (process.stdout.isTTY) - return _getAnimatedDownloadProgress(progressBarName); - return _getBasicDownloadProgress(progressBarName); + return getAnimatedDownloadProgress(progressBarName); + return getBasicDownloadProgress(progressBarName); } -function _getAnimatedDownloadProgress(progressBarName: string): OnProgressCallback { +function getAnimatedDownloadProgress(progressBarName: string): OnProgressCallback { let progressBar: ProgressBar; let lastDownloadedBytes = 0; @@ -109,7 +109,7 @@ function _getAnimatedDownloadProgress(progressBarName: string): OnProgressCallba }; } -function _getBasicDownloadProgress(progressBarName: string): OnProgressCallback { +function getBasicDownloadProgress(progressBarName: string): OnProgressCallback { // eslint-disable-next-line no-console console.log(`Downloading ${progressBarName}...`); const totalRows = 10; @@ -133,11 +133,12 @@ function toMegabytes(bytes: number) { } async function main() { - const [url, destination, progressBarName, userAgent] = process.argv.slice(2); + const [url, destination, progressBarName, userAgent, downloadConnectionTimeout] = process.argv.slice(2); await downloadFile(url, destination, { progressCallback: getDownloadProgress(progressBarName), userAgent, log: message => process.send?.({ method: 'log', params: { message } }), + connectionTimeout: +downloadConnectionTimeout, }); } diff --git a/tests/installation/playwright-cdn-should-race-with-timeout.spec.ts b/tests/installation/playwright-cdn-should-race-with-timeout.spec.ts index f5db1ada8f..e24f18328b 100644 --- a/tests/installation/playwright-cdn-should-race-with-timeout.spec.ts +++ b/tests/installation/playwright-cdn-should-race-with-timeout.spec.ts @@ -18,12 +18,18 @@ import type { AddressInfo } from 'net'; import { test, expect } from './npmTest'; test(`playwright cdn should race with a timeout`, async ({ exec }) => { - test.slow(); // This test will timeout on all the 3 fallback CDNs -> 30 seconds duration. const server = http.createServer(() => {}); await new Promise(resolve => server.listen(0, resolve)); try { - const result = await exec('npm i --foreground-scripts playwright', { env: { PLAYWRIGHT_DOWNLOAD_HOST: `http://127.0.0.1:${(server.address() as AddressInfo).port}`, DEBUG: 'pw:install' }, expectToExitWithError: true }); - expect(result).toContain(`timed out after 10000ms`); + const result = await exec('npm i --foreground-scripts playwright', { + env: { + PLAYWRIGHT_DOWNLOAD_HOST: `http://127.0.0.1:${(server.address() as AddressInfo).port}`, + DEBUG: 'pw:install', + PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT: '1000', + }, + expectToExitWithError: true + }); + expect(result).toContain(`timed out after 1000ms`); } finally { await new Promise(resolve => server.close(resolve)); }