chore: review feedback

This commit is contained in:
Max Schmitt 2024-07-12 11:12:29 +02:00
parent 37ab55c318
commit 60d235e25c
6 changed files with 21 additions and 18 deletions

View file

@ -531,7 +531,6 @@ Using Client Certificates in combination with Proxy Servers is not supported.
:::note :::note
When using WebKit on macOS, accessing `localhost` will not pick up client certificates. You can make it work by replacing `localhost` with `local.playwright`. When using WebKit on macOS, accessing `localhost` will not pick up client certificates. You can make it work by replacing `localhost` with `local.playwright`.
Instead, use `playwright.local` as the hostname.
::: :::
## context-option-useragent ## context-option-useragent

View file

@ -85,7 +85,13 @@ export abstract class Browser extends SdkObject {
async newContext(metadata: CallMetadata, options: channels.BrowserNewContextParams): Promise<BrowserContext> { async newContext(metadata: CallMetadata, options: channels.BrowserNewContextParams): Promise<BrowserContext> {
validateBrowserContextOptions(options, this.options); validateBrowserContextOptions(options, this.options);
const clientCertificatesProxy = await createClientCertificatesProxyIfNeeded(options, this.options); const clientCertificatesProxy = await createClientCertificatesProxyIfNeeded(options, this.options);
const context = await this.doCreateNewContext(options); let context;
try {
context = await this.doCreateNewContext(options);
} catch (error) {
await clientCertificatesProxy?.close();
throw error;
}
context._clientCertificatesProxy = clientCertificatesProxy; context._clientCertificatesProxy = clientCertificatesProxy;
if (options.storageState) if (options.storageState)
await context.setStorageState(metadata, options.storageState); await context.setStorageState(metadata, options.storageState);

View file

@ -659,6 +659,7 @@ export async function createClientCertificatesProxyIfNeeded(options: channels.Br
return; return;
if (options.proxy?.server || browserOptions?.proxy?.server) if (options.proxy?.server || browserOptions?.proxy?.server)
throw new Error('Cannot specify both proxy and clientCertificates'); throw new Error('Cannot specify both proxy and clientCertificates');
verifyClientCertificates(options.clientCertificates);
const clientCertificatesProxy = new ClientCertificatesProxy(options); const clientCertificatesProxy = new ClientCertificatesProxy(options);
options.proxy = { server: await clientCertificatesProxy.listen() }; options.proxy = { server: await clientCertificatesProxy.listen() };
options.ignoreHTTPSErrors = true; options.ignoreHTTPSErrors = true;
@ -702,7 +703,6 @@ export function validateBrowserContextOptions(options: channels.BrowserNewContex
options.proxy = normalizeProxySettings(options.proxy); options.proxy = normalizeProxySettings(options.proxy);
} }
verifyGeolocation(options.geolocation); verifyGeolocation(options.geolocation);
verifyClientCertificates(options.clientCertificates);
} }
export function verifyGeolocation(geolocation?: types.Geolocation) { export function verifyGeolocation(geolocation?: types.Geolocation) {

View file

@ -79,18 +79,16 @@ export abstract class BrowserType extends SdkObject {
const controller = new ProgressController(metadata, this); const controller = new ProgressController(metadata, this);
const persistent: channels.BrowserNewContextParams = { ...options }; const persistent: channels.BrowserNewContextParams = { ...options };
controller.setLogName('browser'); controller.setLogName('browser');
const browser = await controller.run(async progress => {
// Note: Any initial TLS requests will fail since we rely on the Page/Frames initialize which sets ignoreHTTPSErrors. // Note: Any initial TLS requests will fail since we rely on the Page/Frames initialize which sets ignoreHTTPSErrors.
const clientCertificatesProxy = await createClientCertificatesProxyIfNeeded(persistent); const clientCertificatesProxy = await createClientCertificatesProxyIfNeeded(persistent);
if (clientCertificatesProxy) if (clientCertificatesProxy)
options.proxy = persistent.proxy; options.proxy = persistent.proxy;
const browser = await controller.run(progress => {
progress.cleanupWhenAborted(() => clientCertificatesProxy?.close()); progress.cleanupWhenAborted(() => clientCertificatesProxy?.close());
return this._innerLaunchWithRetries(progress, options, persistent, helper.debugProtocolLogger(), userDataDir).catch(e => { throw this._rewriteStartupLog(e); }); const browser = await this._innerLaunchWithRetries(progress, options, persistent, helper.debugProtocolLogger(), userDataDir).catch(e => { throw this._rewriteStartupLog(e); });
browser._defaultContext!._clientCertificatesProxy = clientCertificatesProxy;
return browser;
}, TimeoutSettings.launchTimeout(options)); }, TimeoutSettings.launchTimeout(options));
browser._defaultContext!._clientCertificatesProxy = clientCertificatesProxy;
return browser._defaultContext!; return browser._defaultContext!;
} }

View file

@ -13230,7 +13230,7 @@ export interface BrowserType<Unused = {}> {
* **NOTE** Using Client Certificates in combination with Proxy Servers is not supported. * **NOTE** Using Client Certificates in combination with Proxy Servers is not supported.
* *
* **NOTE** When using WebKit on macOS, accessing `localhost` will not pick up client certificates. You can make it * **NOTE** When using WebKit on macOS, accessing `localhost` will not pick up client certificates. You can make it
* work by replacing `localhost` with `local.playwright`. Instead, use `playwright.local` as the hostname. * work by replacing `localhost` with `local.playwright`.
*/ */
clientCertificates?: Array<{ clientCertificates?: Array<{
/** /**
@ -15642,7 +15642,7 @@ export interface APIRequest {
* **NOTE** Using Client Certificates in combination with Proxy Servers is not supported. * **NOTE** Using Client Certificates in combination with Proxy Servers is not supported.
* *
* **NOTE** When using WebKit on macOS, accessing `localhost` will not pick up client certificates. You can make it * **NOTE** When using WebKit on macOS, accessing `localhost` will not pick up client certificates. You can make it
* work by replacing `localhost` with `local.playwright`. Instead, use `playwright.local` as the hostname. * work by replacing `localhost` with `local.playwright`.
*/ */
clientCertificates?: Array<{ clientCertificates?: Array<{
/** /**
@ -16836,7 +16836,7 @@ export interface Browser extends EventEmitter {
* **NOTE** Using Client Certificates in combination with Proxy Servers is not supported. * **NOTE** Using Client Certificates in combination with Proxy Servers is not supported.
* *
* **NOTE** When using WebKit on macOS, accessing `localhost` will not pick up client certificates. You can make it * **NOTE** When using WebKit on macOS, accessing `localhost` will not pick up client certificates. You can make it
* work by replacing `localhost` with `local.playwright`. Instead, use `playwright.local` as the hostname. * work by replacing `localhost` with `local.playwright`.
*/ */
clientCertificates?: Array<{ clientCertificates?: Array<{
/** /**
@ -20311,7 +20311,7 @@ export interface BrowserContextOptions {
* **NOTE** Using Client Certificates in combination with Proxy Servers is not supported. * **NOTE** Using Client Certificates in combination with Proxy Servers is not supported.
* *
* **NOTE** When using WebKit on macOS, accessing `localhost` will not pick up client certificates. You can make it * **NOTE** When using WebKit on macOS, accessing `localhost` will not pick up client certificates. You can make it
* work by replacing `localhost` with `local.playwright`. Instead, use `playwright.local` as the hostname. * work by replacing `localhost` with `local.playwright`.
*/ */
clientCertificates?: Array<{ clientCertificates?: Array<{
/** /**

View file

@ -5210,7 +5210,7 @@ export interface PlaywrightTestOptions {
* **NOTE** Using Client Certificates in combination with Proxy Servers is not supported. * **NOTE** Using Client Certificates in combination with Proxy Servers is not supported.
* *
* **NOTE** When using WebKit on macOS, accessing `localhost` will not pick up client certificates. You can make it * **NOTE** When using WebKit on macOS, accessing `localhost` will not pick up client certificates. You can make it
* work by replacing `localhost` with `local.playwright`. Instead, use `playwright.local` as the hostname. * work by replacing `localhost` with `local.playwright`.
* *
* **Usage** * **Usage**
* *