feat: APIRequestContext dispose reason (#30765)

Similarly to page.close, we pass test-runner specific reason to
facilitate better error messages.

```
  1) a.test.ts:10:11 › test

    Error: apiRequestContext.fetch: Fixture { request } from beforeAll cannot be reused in a test.
      - Recommended fix: use a separate { request } in the test.
      - Alternatively, manually create APIRequestContext in beforeAll and dispose it in afterAll.
    See https://playwright.dev/docs/api-testing#sending-api-requests-from-ui-tests for more details.

       9 |
      10 |       test('test', async () => {
    > 11 |         await context.fetch('http://example.com');
         |                       ^
      12 |       });
      13 |
```

Closes #29260.
This commit is contained in:
Dmitry Gozman 2024-05-13 18:51:30 -07:00 committed by GitHub
parent f2441eb4b5
commit 776b04e5ea
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 70 additions and 13 deletions

View File

@ -185,6 +185,12 @@ context cookies from the response. The method will automatically follow redirect
All responses returned by [`method: APIRequestContext.get`] and similar methods are stored in the memory, so that you can later call [`method: APIResponse.body`].This method discards all its resources, calling any method on disposed [APIRequestContext] will throw an exception.
### option: APIRequestContext.dispose.reason
* since: v1.45
- `reason` <[string]>
The reason to be reported to the operations interrupted by the context disposure.
## async method: APIRequestContext.fetch
* since: v1.16
- returns: <[APIResponse]>

View File

@ -85,6 +85,7 @@ export class APIRequest implements api.APIRequest {
export class APIRequestContext extends ChannelOwner<channels.APIRequestContextChannel> implements api.APIRequestContext {
_request?: APIRequest;
readonly _tracing: Tracing;
private _closeReason: string | undefined;
static from(channel: channels.APIRequestContextChannel): APIRequestContext {
return (channel as any)._object;
@ -99,9 +100,10 @@ export class APIRequestContext extends ChannelOwner<channels.APIRequestContextCh
await this.dispose();
}
async dispose(): Promise<void> {
async dispose(options: { reason?: string } = {}): Promise<void> {
this._closeReason = options.reason;
await this._instrumentation.onWillCloseRequestContext(this);
await this._channel.dispose();
await this._channel.dispose(options);
this._tracing._resetStackCounter();
this._request?._contexts.delete(this);
}
@ -156,6 +158,8 @@ export class APIRequestContext extends ChannelOwner<channels.APIRequestContextCh
async _innerFetch(options: FetchOptions & { url?: string, request?: api.Request } = {}): Promise<APIResponse> {
return await this._wrapApiCall(async () => {
if (this._closeReason)
throw new Error(this._closeReason);
assert(options.request || typeof options.url === 'string', 'First argument must be either URL string or Request');
assert((options.data === undefined ? 0 : 1) + (options.form === undefined ? 0 : 1) + (options.multipart === undefined ? 0 : 1) <= 1, `Only one of 'data', 'form' or 'multipart' can be specified`);
assert(options.maxRedirects === undefined || options.maxRedirects >= 0, `'maxRedirects' should be greater than or equal to '0'`);

View File

@ -207,7 +207,9 @@ scheme.APIRequestContextDisposeAPIResponseParams = tObject({
fetchUid: tString,
});
scheme.APIRequestContextDisposeAPIResponseResult = tOptional(tObject({}));
scheme.APIRequestContextDisposeParams = tOptional(tObject({}));
scheme.APIRequestContextDisposeParams = tObject({
reason: tOptional(tString),
});
scheme.APIRequestContextDisposeResult = tOptional(tObject({}));
scheme.APIResponse = tObject({
fetchUid: tString,

View File

@ -198,9 +198,9 @@ export class APIRequestContextDispatcher extends Dispatcher<APIRequestContext, c
return this._object.storageState();
}
async dispose(_: channels.APIRequestContextDisposeParams, metadata: CallMetadata): Promise<void> {
async dispose(params: channels.APIRequestContextDisposeParams, metadata: CallMetadata): Promise<void> {
metadata.potentiallyClosesScope = true;
await this._object.dispose();
await this._object.dispose(params);
this._dispose();
}

View File

@ -121,7 +121,7 @@ export abstract class APIRequestContext extends SdkObject {
abstract tracing(): Tracing;
abstract dispose(): Promise<void>;
abstract dispose(options: { reason?: string }): Promise<void>;
abstract _defaultOptions(): FetchRequestOptions;
abstract _addCookies(cookies: channels.NetworkCookie[]): Promise<void>;
@ -483,7 +483,8 @@ export class BrowserContextAPIRequestContext extends APIRequestContext {
return this._context.tracing;
}
override async dispose() {
override async dispose(options: { reason?: string }) {
this._closeReason = options.reason;
this.fetchResponses.clear();
}
@ -552,7 +553,8 @@ export class GlobalAPIRequestContext extends APIRequestContext {
return this._tracing;
}
override async dispose() {
override async dispose(options: { reason?: string }) {
this._closeReason = options.reason;
await this._tracing.flush();
await this._tracing.deleteTmpTracesDir();
this._disposeImpl();

View File

@ -15846,8 +15846,14 @@ export interface APIRequestContext {
* and similar methods are stored in the memory, so that you can later call
* [apiResponse.body()](https://playwright.dev/docs/api/class-apiresponse#api-response-body).This method discards all
* its resources, calling any method on disposed {@link APIRequestContext} will throw an exception.
* @param options
*/
dispose(): Promise<void>;
dispose(options?: {
/**
* The reason to be reported to the operations interrupted by the context disposure.
*/
reason?: string;
}): Promise<void>;
/**
* Sends HTTP(S) request and returns its response. The method will populate request cookies from the context and

View File

@ -392,7 +392,17 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
request: async ({ playwright }, use) => {
const request = await playwright.request.newContext();
await use(request);
await request.dispose();
const hook = (test.info() as TestInfoImpl)._currentHookType();
if (hook === 'beforeAll') {
await request.dispose({ reason: [
`Fixture { request } from beforeAll cannot be reused in a test.`,
` - Recommended fix: use a separate { request } in the test.`,
` - Alternatively, manually create APIRequestContext in beforeAll and dispose it in afterAll.`,
`See https://playwright.dev/docs/api-testing#sending-api-requests-from-ui-tests for more details.`,
].join('\n') });
} else {
await request.dispose();
}
},
});

View File

@ -309,7 +309,7 @@ export interface APIRequestContextChannel extends APIRequestContextEventTarget,
fetchLog(params: APIRequestContextFetchLogParams, metadata?: CallMetadata): Promise<APIRequestContextFetchLogResult>;
storageState(params?: APIRequestContextStorageStateParams, metadata?: CallMetadata): Promise<APIRequestContextStorageStateResult>;
disposeAPIResponse(params: APIRequestContextDisposeAPIResponseParams, metadata?: CallMetadata): Promise<APIRequestContextDisposeAPIResponseResult>;
dispose(params?: APIRequestContextDisposeParams, metadata?: CallMetadata): Promise<APIRequestContextDisposeResult>;
dispose(params: APIRequestContextDisposeParams, metadata?: CallMetadata): Promise<APIRequestContextDisposeResult>;
}
export type APIRequestContextFetchParams = {
url: string,
@ -372,8 +372,12 @@ export type APIRequestContextDisposeAPIResponseOptions = {
};
export type APIRequestContextDisposeAPIResponseResult = void;
export type APIRequestContextDisposeParams = {};
export type APIRequestContextDisposeOptions = {};
export type APIRequestContextDisposeParams = {
reason?: string,
};
export type APIRequestContextDisposeOptions = {
reason?: string,
};
export type APIRequestContextDisposeResult = void;
export interface APIRequestContextEvents {

View File

@ -330,6 +330,8 @@ APIRequestContext:
fetchUid: string
dispose:
parameters:
reason: string?
APIResponse:

View File

@ -827,3 +827,24 @@ test('should save trace in two APIRequestContexts', async ({ runInlineTest, serv
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(1);
});
test('should explain a failure when using a dispose APIRequestContext', async ({ runInlineTest, server }) => {
const result = await runInlineTest({
'a.test.ts': `
import { test } from '@playwright/test';
let context;
test.beforeAll(async ({ request }) => {
context = request;
});
test('test', async () => {
await context.fetch('http://example.com');
});
`,
}, { workers: 1 });
expect(result.exitCode).toBe(1);
expect(result.passed).toBe(0);
expect(result.output).toContain(`Recommended fix: use a separate { request } in the test`);
});