chore: rename ScopedRace to LongStandingScope, preserve Error instance in reject() (#24361)

This commit is contained in:
Dmitry Gozman 2023-07-23 21:00:07 -07:00 committed by GitHub
parent 2070861807
commit 2cb0622c39
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 73 additions and 65 deletions

View file

@ -23,7 +23,7 @@ import type { Headers, RemoteAddr, SecurityDetails, WaitForEventOptions } from '
import fs from 'fs'; import fs from 'fs';
import { mime } from '../utilsBundle'; import { mime } from '../utilsBundle';
import { assert, isString, headersObjectToArray, isRegExp } from '../utils'; import { assert, isString, headersObjectToArray, isRegExp } from '../utils';
import { ManualPromise, ScopedRace } from '../utils/manualPromise'; import { ManualPromise, LongStandingScope } from '../utils/manualPromise';
import { Events } from './events'; import { Events } from './events';
import type { Page } from './page'; import type { Page } from './page';
import { Waiter } from './waiter'; import { Waiter } from './waiter';
@ -270,8 +270,8 @@ export class Request extends ChannelOwner<channels.RequestChannel> implements ap
return this._fallbackOverrides; return this._fallbackOverrides;
} }
_targetClosedRace(): ScopedRace { _targetClosedScope(): LongStandingScope {
return this.serviceWorker()?._closedRace || this.frame()._page?._closedOrCrashedRace || new ScopedRace(); return this.serviceWorker()?._closedScope || this.frame()._page?._closedOrCrashedScope || new LongStandingScope();
} }
} }
@ -294,7 +294,7 @@ export class Route extends ChannelOwner<channels.RouteChannel> implements api.Ro
// When page closes or crashes, we catch any potential rejects from this Route. // When page closes or crashes, we catch any potential rejects from this Route.
// Note that page could be missing when routing popup's initial request that // Note that page could be missing when routing popup's initial request that
// does not have a Page initialized just yet. // does not have a Page initialized just yet.
return this.request()._targetClosedRace().safeRace(promise); return this.request()._targetClosedScope().safeRace(promise);
} }
_startHandling(): Promise<boolean> { _startHandling(): Promise<boolean> {
@ -522,7 +522,7 @@ export class Response extends ChannelOwner<channels.ResponseChannel> implements
} }
async finished(): Promise<null> { async finished(): Promise<null> {
return this.request()._targetClosedRace().race(this._finishedPromise); return this.request()._targetClosedScope().race(this._finishedPromise);
} }
async body(): Promise<Buffer> { async body(): Promise<Buffer> {

View file

@ -24,7 +24,7 @@ import { urlMatches } from '../utils/network';
import { TimeoutSettings } from '../common/timeoutSettings'; import { TimeoutSettings } from '../common/timeoutSettings';
import type * as channels from '@protocol/channels'; import type * as channels from '@protocol/channels';
import { parseError, serializeError } from '../protocol/serializers'; import { parseError, serializeError } from '../protocol/serializers';
import { assert, headersObjectToArray, isObject, isRegExp, isString, ScopedRace, urlMatchesEqual } from '../utils'; import { assert, headersObjectToArray, isObject, isRegExp, isString, LongStandingScope, urlMatchesEqual } from '../utils';
import { mkdirIfNeeded } from '../utils/fileUtils'; import { mkdirIfNeeded } from '../utils/fileUtils';
import { Accessibility } from './accessibility'; import { Accessibility } from './accessibility';
import { Artifact } from './artifact'; import { Artifact } from './artifact';
@ -78,7 +78,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
private _frames = new Set<Frame>(); private _frames = new Set<Frame>();
_workers = new Set<Worker>(); _workers = new Set<Worker>();
private _closed = false; private _closed = false;
readonly _closedOrCrashedRace = new ScopedRace(); readonly _closedOrCrashedScope = new LongStandingScope();
private _viewportSize: Size | null; private _viewportSize: Size | null;
private _routes: RouteHandler[] = []; private _routes: RouteHandler[] = [];
@ -141,8 +141,8 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
this.coverage = new Coverage(this._channel); this.coverage = new Coverage(this._channel);
this.once(Events.Page.Close, () => this._closedOrCrashedRace.scopeClosed(new Error(kBrowserOrContextClosedError))); this.once(Events.Page.Close, () => this._closedOrCrashedScope.close(kBrowserOrContextClosedError));
this.once(Events.Page.Crash, () => this._closedOrCrashedRace.scopeClosed(new Error(kBrowserOrContextClosedError))); this.once(Events.Page.Crash, () => this._closedOrCrashedScope.close(kBrowserOrContextClosedError));
this._setEventToSubscriptionMapping(new Map<string, channels.PageUpdateSubscriptionParams['event']>([ this._setEventToSubscriptionMapping(new Map<string, channels.PageUpdateSubscriptionParams['event']>([
[Events.Page.Console, 'console'], [Events.Page.Console, 'console'],
@ -686,7 +686,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
this._browserContext.setDefaultNavigationTimeout(0); this._browserContext.setDefaultNavigationTimeout(0);
this._browserContext.setDefaultTimeout(0); this._browserContext.setDefaultTimeout(0);
this._instrumentation?.onWillPause(); this._instrumentation?.onWillPause();
await this._closedOrCrashedRace.safeRace(this.context()._channel.pause()); await this._closedOrCrashedScope.safeRace(this.context()._channel.pause());
this._browserContext.setDefaultNavigationTimeout(defaultNavigationTimeout); this._browserContext.setDefaultNavigationTimeout(defaultNavigationTimeout);
this._browserContext.setDefaultTimeout(defaultTimeout); this._browserContext.setDefaultTimeout(defaultTimeout);
} }

View file

@ -27,7 +27,7 @@ export class Video implements api.Video {
constructor(page: Page, connection: Connection) { constructor(page: Page, connection: Connection) {
this._isRemote = connection.isRemote(); this._isRemote = connection.isRemote();
this._artifact = page._closedOrCrashedRace.safeRace(this._artifactReadyPromise); this._artifact = page._closedOrCrashedScope.safeRace(this._artifactReadyPromise);
} }
_artifactReady(artifact: Artifact) { _artifactReady(artifact: Artifact) {

View file

@ -22,13 +22,13 @@ import type { Page } from './page';
import type { BrowserContext } from './browserContext'; import type { BrowserContext } from './browserContext';
import type * as api from '../../types/types'; import type * as api from '../../types/types';
import type * as structs from '../../types/structs'; import type * as structs from '../../types/structs';
import { ScopedRace } from '../utils'; import { LongStandingScope } from '../utils';
import { kBrowserOrContextClosedError } from '../common/errors'; import { kBrowserOrContextClosedError } from '../common/errors';
export class Worker extends ChannelOwner<channels.WorkerChannel> implements api.Worker { export class Worker extends ChannelOwner<channels.WorkerChannel> implements api.Worker {
_page: Page | undefined; // Set for web workers. _page: Page | undefined; // Set for web workers.
_context: BrowserContext | undefined; // Set for service workers. _context: BrowserContext | undefined; // Set for service workers.
readonly _closedRace = new ScopedRace(); readonly _closedScope = new LongStandingScope();
static from(worker: channels.WorkerChannel): Worker { static from(worker: channels.WorkerChannel): Worker {
return (worker as any)._object; return (worker as any)._object;
@ -43,7 +43,7 @@ export class Worker extends ChannelOwner<channels.WorkerChannel> implements api.
this._context._serviceWorkers.delete(this); this._context._serviceWorkers.delete(this);
this.emit(Events.Worker.Close, this); this.emit(Events.Worker.Close, this);
}); });
this.once(Events.Worker.Close, () => this._closedRace.scopeClosed(new Error(kBrowserOrContextClosedError))); this.once(Events.Worker.Close, () => this._closedScope.close(kBrowserOrContextClosedError));
} }
url(): string { url(): string {

View file

@ -190,7 +190,7 @@ export abstract class BrowserContext extends SdkObject {
const [, ...otherPages] = this.pages(); const [, ...otherPages] = this.pages();
for (const p of otherPages) for (const p of otherPages)
await p.close(metadata); await p.close(metadata);
if (page && page._crashedRace.isDone()) { if (page && page._crashedScope.isClosed()) {
await page.close(metadata); await page.close(metadata);
page = undefined; page = undefined;
} }

View file

@ -29,7 +29,7 @@ import * as types from './types';
import { BrowserContext } from './browserContext'; import { BrowserContext } from './browserContext';
import type { Progress } from './progress'; import type { Progress } from './progress';
import { ProgressController } from './progress'; import { ProgressController } from './progress';
import { ScopedRace, assert, constructURLBasedOnBaseURL, makeWaitForNextTask, monotonicTime } from '../utils'; import { LongStandingScope, assert, constructURLBasedOnBaseURL, makeWaitForNextTask, monotonicTime } from '../utils';
import { ManualPromise } from '../utils/manualPromise'; import { ManualPromise } from '../utils/manualPromise';
import { debugLogger } from '../common/debugLogger'; import { debugLogger } from '../common/debugLogger';
import type { CallMetadata } from './instrumentation'; import type { CallMetadata } from './instrumentation';
@ -44,7 +44,7 @@ import { FrameSelectors } from './frameSelectors';
import { TimeoutError } from '../common/errors'; import { TimeoutError } from '../common/errors';
type ContextData = { type ContextData = {
contextPromise: ManualPromise<dom.FrameExecutionContext | Error>; contextPromise: ManualPromise<dom.FrameExecutionContext | { destroyedReason: string }>;
context: dom.FrameExecutionContext | null; context: dom.FrameExecutionContext | null;
}; };
@ -482,7 +482,7 @@ export class Frame extends SdkObject {
_inflightRequests = new Set<network.Request>(); _inflightRequests = new Set<network.Request>();
private _networkIdleTimer: NodeJS.Timer | undefined; private _networkIdleTimer: NodeJS.Timer | undefined;
private _setContentCounter = 0; private _setContentCounter = 0;
readonly _detachedRace = new ScopedRace(); readonly _detachedScope = new LongStandingScope();
private _raceAgainstEvaluationStallingEventsPromises = new Set<ManualPromise<any>>(); private _raceAgainstEvaluationStallingEventsPromises = new Set<ManualPromise<any>>();
readonly _redirectedNavigations = new Map<string, { url: string, gotoPromise: Promise<network.Response | null> }>(); // documentId -> data readonly _redirectedNavigations = new Map<string, { url: string, gotoPromise: Promise<network.Response | null> }>(); // documentId -> data
readonly selectors: FrameSelectors; readonly selectors: FrameSelectors;
@ -510,7 +510,7 @@ export class Frame extends SdkObject {
} }
isDetached(): boolean { isDetached(): boolean {
return this._detachedRace.isDone(); return this._detachedScope.isClosed();
} }
_onLifecycleEvent(event: RegularLifecycleEvent) { _onLifecycleEvent(event: RegularLifecycleEvent) {
@ -613,10 +613,10 @@ export class Frame extends SdkObject {
} }
async raceNavigationAction(progress: Progress, options: types.GotoOptions, action: () => Promise<network.Response | null>): Promise<network.Response | null> { async raceNavigationAction(progress: Progress, options: types.GotoOptions, action: () => Promise<network.Response | null>): Promise<network.Response | null> {
return ScopedRace.raceMultiple([ return LongStandingScope.raceMultiple([
this._detachedRace, this._detachedScope,
this._page._disconnectedRace, this._page._disconnectedScope,
this._page._crashedRace, this._page._crashedScope,
], action().catch(e => { ], action().catch(e => {
if (e instanceof NavigationAbortedError && e.documentId) { if (e instanceof NavigationAbortedError && e.documentId) {
const data = this._redirectedNavigations.get(e.documentId); const data = this._redirectedNavigations.get(e.documentId);
@ -727,10 +727,10 @@ export class Frame extends SdkObject {
} }
_context(world: types.World): Promise<dom.FrameExecutionContext> { _context(world: types.World): Promise<dom.FrameExecutionContext> {
return this._contextData.get(world)!.contextPromise.then(contextOrError => { return this._contextData.get(world)!.contextPromise.then(contextOrDestroyedReason => {
if (contextOrError instanceof js.ExecutionContext) if (contextOrDestroyedReason instanceof js.ExecutionContext)
return contextOrError; return contextOrDestroyedReason;
throw contextOrError; throw new Error(contextOrDestroyedReason.destroyedReason);
}); });
} }
@ -1053,10 +1053,10 @@ export class Frame extends SdkObject {
// Make sure we react immediately upon page close or frame detach. // Make sure we react immediately upon page close or frame detach.
// We need this to show expected/received values in time. // We need this to show expected/received values in time.
const actionPromise = new Promise(f => setTimeout(f, timeout)); const actionPromise = new Promise(f => setTimeout(f, timeout));
await ScopedRace.raceMultiple([ await LongStandingScope.raceMultiple([
this._page._disconnectedRace, this._page._disconnectedScope,
this._page._crashedRace, this._page._crashedScope,
this._detachedRace, this._detachedScope,
], actionPromise); ], actionPromise);
} }
progress.throwIfAborted(); progress.throwIfAborted();
@ -1533,12 +1533,11 @@ export class Frame extends SdkObject {
_onDetached() { _onDetached() {
this._stopNetworkIdleTimer(); this._stopNetworkIdleTimer();
const error = new Error('Frame was detached'); this._detachedScope.close('Frame was detached');
this._detachedRace.scopeClosed(error);
for (const data of this._contextData.values()) { for (const data of this._contextData.values()) {
if (data.context) if (data.context)
data.context.contextDestroyed(error); data.context.contextDestroyed('Frame was detached');
data.contextPromise.resolve(error); data.contextPromise.resolve({ destroyedReason: 'Frame was detached' });
} }
if (this._parentFrame) if (this._parentFrame)
this._parentFrame._childFrames.delete(this); this._parentFrame._childFrames.delete(this);
@ -1591,7 +1590,7 @@ export class Frame extends SdkObject {
// connections so we might end up creating multiple isolated worlds. // connections so we might end up creating multiple isolated worlds.
// We can use either. // We can use either.
if (data.context) { if (data.context) {
data.context.contextDestroyed(new Error('Execution context was destroyed, most likely because of a navigation')); data.context.contextDestroyed('Execution context was destroyed, most likely because of a navigation');
this._setContext(world, null); this._setContext(world, null);
} }
this._setContext(world, context); this._setContext(world, context);
@ -1600,9 +1599,9 @@ export class Frame extends SdkObject {
_contextDestroyed(context: dom.FrameExecutionContext) { _contextDestroyed(context: dom.FrameExecutionContext) {
// Sometimes we get this after detach, in which case we should not reset // Sometimes we get this after detach, in which case we should not reset
// our already destroyed contexts to something that will never resolve. // our already destroyed contexts to something that will never resolve.
if (this._detachedRace.isDone()) if (this._detachedScope.isClosed())
return; return;
context.contextDestroyed(new Error('Execution context was destroyed, most likely because of a navigation')); context.contextDestroyed('Execution context was destroyed, most likely because of a navigation');
for (const [world, data] of this._contextData) { for (const [world, data] of this._contextData) {
if (data.context === context) if (data.context === context)
this._setContext(world, null); this._setContext(world, null);
@ -1614,7 +1613,7 @@ export class Frame extends SdkObject {
// We should not start a timer and report networkidle in detached frames. // We should not start a timer and report networkidle in detached frames.
// This happens at least in Firefox for child frames, where we may get requestFinished // This happens at least in Firefox for child frames, where we may get requestFinished
// after the frame was detached - probably a race in the Firefox itself. // after the frame was detached - probably a race in the Firefox itself.
if (this._firedLifecycleEvents.has('networkidle') || this._detachedRace.isDone()) if (this._firedLifecycleEvents.has('networkidle') || this._detachedScope.isClosed())
return; return;
this._networkIdleTimer = setTimeout(() => { this._networkIdleTimer = setTimeout(() => {
this._firedNetworkIdleSelf = true; this._firedNetworkIdleSelf = true;
@ -1703,10 +1702,10 @@ class SignalBarrier {
this._progress.log(` navigated to "${frame._url}"`); this._progress.log(` navigated to "${frame._url}"`);
return true; return true;
}); });
await ScopedRace.raceMultiple([ await LongStandingScope.raceMultiple([
frame._page._disconnectedRace, frame._page._disconnectedScope,
frame._page._crashedRace, frame._page._crashedScope,
frame._detachedRace, frame._detachedScope,
], waiter.promise).catch(() => {}); ], waiter.promise).catch(() => {});
waiter.dispose(); waiter.dispose();
this.release(); this.release();

View file

@ -19,7 +19,7 @@ import * as utilityScriptSource from '../generated/utilityScriptSource';
import { serializeAsCallArgument } from './isomorphic/utilityScriptSerializers'; import { serializeAsCallArgument } from './isomorphic/utilityScriptSerializers';
import type { UtilityScript } from './injected/utilityScript'; import type { UtilityScript } from './injected/utilityScript';
import { SdkObject } from './instrumentation'; import { SdkObject } from './instrumentation';
import { ScopedRace } from '../utils/manualPromise'; import { LongStandingScope } from '../utils/manualPromise';
export type ObjectId = string; export type ObjectId = string;
export type RemoteObject = { export type RemoteObject = {
@ -63,7 +63,7 @@ export interface ExecutionContextDelegate {
export class ExecutionContext extends SdkObject { export class ExecutionContext extends SdkObject {
private _delegate: ExecutionContextDelegate; private _delegate: ExecutionContextDelegate;
private _utilityScriptPromise: Promise<JSHandle> | undefined; private _utilityScriptPromise: Promise<JSHandle> | undefined;
private _contextDestroyedRace = new ScopedRace(); private _contextDestroyedScope = new LongStandingScope();
readonly worldNameForTest: string; readonly worldNameForTest: string;
constructor(parent: SdkObject, delegate: ExecutionContextDelegate, worldNameForTest: string) { constructor(parent: SdkObject, delegate: ExecutionContextDelegate, worldNameForTest: string) {
@ -72,12 +72,12 @@ export class ExecutionContext extends SdkObject {
this._delegate = delegate; this._delegate = delegate;
} }
contextDestroyed(error: Error) { contextDestroyed(reason: string) {
this._contextDestroyedRace.scopeClosed(error); this._contextDestroyedScope.close(reason);
} }
async _raceAgainstContextDestroyed<T>(promise: Promise<T>): Promise<T> { async _raceAgainstContextDestroyed<T>(promise: Promise<T>): Promise<T> {
return this._contextDestroyedRace.race(promise); return this._contextDestroyedScope.race(promise);
} }
rawEvaluateJSON(expression: string): Promise<any> { rawEvaluateJSON(expression: string): Promise<any> {

View file

@ -31,7 +31,7 @@ import * as accessibility from './accessibility';
import { FileChooser } from './fileChooser'; import { FileChooser } from './fileChooser';
import type { Progress } from './progress'; import type { Progress } from './progress';
import { ProgressController } from './progress'; import { ProgressController } from './progress';
import { ScopedRace, assert, isError } from '../utils'; import { LongStandingScope, assert, isError } from '../utils';
import { ManualPromise } from '../utils/manualPromise'; import { ManualPromise } from '../utils/manualPromise';
import { debugLogger } from '../common/debugLogger'; import { debugLogger } from '../common/debugLogger';
import type { ImageComparatorOptions } from '../utils/comparators'; import type { ImageComparatorOptions } from '../utils/comparators';
@ -142,8 +142,8 @@ export class Page extends SdkObject {
private _disconnected = false; private _disconnected = false;
private _initialized = false; private _initialized = false;
private _eventsToEmitAfterInitialized: { event: string | symbol, args: any[] }[] = []; private _eventsToEmitAfterInitialized: { event: string | symbol, args: any[] }[] = [];
readonly _disconnectedRace = new ScopedRace(); readonly _disconnectedScope = new LongStandingScope();
readonly _crashedRace = new ScopedRace(); readonly _crashedScope = new LongStandingScope();
readonly _browserContext: BrowserContext; readonly _browserContext: BrowserContext;
readonly keyboard: input.Keyboard; readonly keyboard: input.Keyboard;
readonly mouse: input.Mouse; readonly mouse: input.Mouse;
@ -285,7 +285,7 @@ export class Page extends SdkObject {
this._frameManager.dispose(); this._frameManager.dispose();
this._frameThrottler.dispose(); this._frameThrottler.dispose();
this.emit(Page.Events.Crash); this.emit(Page.Events.Crash);
this._crashedRace.scopeClosed(new Error('Page crashed')); this._crashedScope.close('Page crashed');
this.instrumentation.onPageClose(this); this.instrumentation.onPageClose(this);
} }
@ -294,7 +294,7 @@ export class Page extends SdkObject {
this._frameThrottler.dispose(); this._frameThrottler.dispose();
assert(!this._disconnected, 'Page disconnected twice'); assert(!this._disconnected, 'Page disconnected twice');
this._disconnected = true; this._disconnected = true;
this._disconnectedRace.scopeClosed(new Error('Page closed')); this._disconnectedScope.close('Page closed');
} }
async _onFileChooserOpened(handle: dom.ElementHandle) { async _onFileChooserOpened(handle: dom.ElementHandle) {
@ -632,7 +632,7 @@ export class Page extends SdkObject {
} }
isClosedOrClosingOrCrashed() { isClosedOrClosingOrCrashed() {
return this._closedState !== 'open' || this._crashedRace.isDone(); return this._closedState !== 'open' || this._crashedScope.isClosed();
} }
_addWorker(workerId: string, worker: Worker) { _addWorker(workerId: string, worker: Worker) {
@ -737,7 +737,7 @@ export class Worker extends SdkObject {
didClose() { didClose() {
if (this._existingExecutionContext) if (this._existingExecutionContext)
this._existingExecutionContext.contextDestroyed(new Error('Worker was closed')); this._existingExecutionContext.contextDestroyed('Worker was closed');
this.emit(Worker.Events.Close, this); this.emit(Worker.Events.Close, this);
} }

View file

@ -56,25 +56,33 @@ export class ManualPromise<T = void> extends Promise<T> {
} }
} }
export class ScopedRace { export class LongStandingScope {
private _terminateError: Error | undefined; private _terminateError: Error | undefined;
private _terminateErrorMessage: string | undefined;
private _terminatePromises = new Map<ManualPromise<Error>, Error>(); private _terminatePromises = new Map<ManualPromise<Error>, Error>();
private _isDone = false; private _isClosed = false;
scopeClosed(error: Error) { reject(error: Error) {
this._isDone = true; this._isClosed = true;
this._terminateError = error; this._terminateError = error;
for (const p of this._terminatePromises.keys())
p.resolve(error);
}
close(errorMessage: string) {
this._isClosed = true;
this._terminateErrorMessage = errorMessage;
for (const [p, e] of this._terminatePromises) { for (const [p, e] of this._terminatePromises) {
rewriteErrorMessage(e, error.message); rewriteErrorMessage(e, errorMessage);
p.resolve(e); p.resolve(e);
} }
} }
isDone() { isClosed() {
return this._isDone; return this._isClosed;
} }
static async raceMultiple<T>(scopes: ScopedRace[], promise: Promise<T>): Promise<T> { static async raceMultiple<T>(scopes: LongStandingScope[], promise: Promise<T>): Promise<T> {
return Promise.race(scopes.map(s => s.race(promise))); return Promise.race(scopes.map(s => s.race(promise)));
} }
@ -90,8 +98,9 @@ export class ScopedRace {
const terminatePromise = new ManualPromise<Error>(); const terminatePromise = new ManualPromise<Error>();
if (this._terminateError) if (this._terminateError)
terminatePromise.resolve(this._terminateError); terminatePromise.resolve(this._terminateError);
const error = new Error(''); if (this._terminateErrorMessage)
this._terminatePromises.set(terminatePromise, error); terminatePromise.resolve(new Error(this._terminateErrorMessage));
this._terminatePromises.set(terminatePromise, new Error(''));
try { try {
return await Promise.race([ return await Promise.race([
terminatePromise.then(e => safe ? defaultValue : Promise.reject(e)), terminatePromise.then(e => safe ? defaultValue : Promise.reject(e)),