Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(reuse): stop pending operations upon reuse/disconnect #18997

Merged
merged 1 commit into from Nov 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/playwright-core/src/remote/playwrightConnection.ts
Expand Up @@ -208,6 +208,8 @@ export class PlaywrightConnection {
for (const context of browser.contexts()) {
if (!context.pages().length)
await context.close(serverSideCallMetadata());
else
await context.stopPendingOperations();
}
if (!browser.contexts())
await browser.close();
Expand Down
1 change: 1 addition & 0 deletions packages/playwright-core/src/server/browser.ts
Expand Up @@ -107,6 +107,7 @@ export abstract class Browser extends SdkObject {
this._contextForReuse = { context: await this.newContext(metadata, params), hash };
return { context: this._contextForReuse.context, needsReset: false };
}
await this._contextForReuse.context.stopPendingOperations();
return { context: this._contextForReuse.context, needsReset: true };
}

Expand Down
8 changes: 7 additions & 1 deletion packages/playwright-core/src/server/browserContext.ts
Expand Up @@ -26,7 +26,7 @@ import { helper } from './helper';
import * as network from './network';
import type { PageDelegate } from './page';
import { Page, PageBinding } from './page';
import type { Progress } from './progress';
import type { Progress, ProgressController } from './progress';
import type { Selectors } from './selectors';
import type * as types from './types';
import type * as channels from '@protocol/channels';
Expand Down Expand Up @@ -56,6 +56,7 @@ export abstract class BrowserContext extends SdkObject {

readonly _timeoutSettings = new TimeoutSettings();
readonly _pageBindings = new Map<string, PageBinding>();
readonly _activeProgressControllers = new Set<ProgressController>();
readonly _options: channels.BrowserNewContextParams;
_requestInterceptor?: network.RouteHandler;
private _isPersistentContext: boolean;
Expand Down Expand Up @@ -150,6 +151,11 @@ export abstract class BrowserContext extends SdkObject {
return true;
}

async stopPendingOperations() {
for (const controller of this._activeProgressControllers)
controller.abort(new Error(`Context was reset for reuse.`));
}

static reusableContextHash(params: channels.BrowserNewContextForReuseParams): string {
const paramsCopy = { ...params };

Expand Down
1 change: 1 addition & 0 deletions packages/playwright-core/src/server/fetch.ts
Expand Up @@ -78,6 +78,7 @@ export abstract class APIRequestContext extends SdkObject {
readonly fetchResponses: Map<string, Buffer> = new Map();
readonly fetchLog: Map<string, string[]> = new Map();
protected static allInstances: Set<APIRequestContext> = new Set();
readonly _activeProgressControllers = new Set<ProgressController>();

static findResponseBody(guid: string): Buffer | undefined {
for (const request of APIRequestContext.allInstances) {
Expand Down
6 changes: 6 additions & 0 deletions packages/playwright-core/src/server/progress.ts
Expand Up @@ -63,6 +63,10 @@ export class ProgressController {
return this._lastIntermediateResult;
}

abort(error: Error) {
this._forceAbortPromise.reject(error);
}

async run<T>(task: (progress: Progress) => Promise<T>, timeout?: number): Promise<T> {
if (timeout) {
this._timeout = timeout;
Expand All @@ -71,6 +75,7 @@ export class ProgressController {

assert(this._state === 'before');
this._state = 'running';
this.sdkObject.attribution.context?._activeProgressControllers.add(this);

const progress: Progress = {
log: message => {
Expand Down Expand Up @@ -117,6 +122,7 @@ export class ProgressController {
await Promise.all(this._cleanups.splice(0).map(runCleanup));
throw e;
} finally {
this.sdkObject.attribution.context?._activeProgressControllers.delete(this);
clearTimeout(timer);
}
}
Expand Down
24 changes: 24 additions & 0 deletions tests/playwright-test/playwright.reuse.spec.ts
Expand Up @@ -374,3 +374,27 @@ test('should reuse context with beforeunload', async ({ runInlineTest }) => {
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(2);
});

test('should cancel pending operations upon reuse', async ({ runInlineTest }) => {
const result = await runInlineTest({
'src/reuse.test.ts': `
const { test } = pwt;
test('one', async ({ page }) => {
await Promise.race([
page.getByText('click me').click().catch(e => {}),
page.waitForTimeout(2000),
]);
});

test('two', async ({ page }) => {
await page.setContent('<button onclick="window._clicked=true">click me</button>');
// Give it time to erroneously click.
await page.waitForTimeout(2000);
expect(await page.evaluate('window._clicked')).toBe(undefined);
});
`,
}, { workers: 1 }, { PW_TEST_REUSE_CONTEXT: '1' });

expect(result.exitCode).toBe(0);
expect(result.passed).toBe(2);
});