feat: do not record route calls in the trace (#32723)

These are represented in the network pane instead.
This commit is contained in:
Dmitry Gozman 2024-09-21 10:17:59 -07:00 committed by GitHub
parent 17ed944a84
commit b3a82bef46
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 38 additions and 164 deletions

View file

@ -220,7 +220,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
}
// If the page is closed or unrouteAll() was called without waiting and interception disabled,
// the method will throw an error - silence it.
await route._innerContinue(true).catch(() => {});
await route._innerContinue(true /* isFallback */).catch(() => {});
}
async _onWebSocketRoute(webSocketRoute: network.WebSocketRoute) {

View file

@ -40,6 +40,7 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
_logger: Logger | undefined;
readonly _instrumentation: ClientInstrumentation;
private _eventToSubscriptionMapping: Map<string, string> = new Map();
private _isInternalType = false;
_wasCollected: boolean = false;
constructor(parent: ChannelOwner | Connection, type: string, guid: string, initializer: channels.InitializerTraits<T>) {
@ -61,6 +62,10 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
this._initializer = initializer;
}
protected markAsInternalType() {
this._isInternalType = true;
}
_setEventToSubscriptionMapping(mapping: Map<string, string>) {
this._eventToSubscriptionMapping = mapping;
}
@ -173,7 +178,7 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
let apiName: string | undefined = stackTrace.apiName;
const frames: channels.StackFrame[] = stackTrace.frames;
isInternal = isInternal || this._type === 'LocalUtils';
isInternal = isInternal || this._isInternalType;
if (isInternal)
apiName = undefined;

View file

@ -33,6 +33,7 @@ export class LocalUtils extends ChannelOwner<channels.LocalUtilsChannel> {
constructor(parent: ChannelOwner, type: string, guid: string, initializer: channels.LocalUtilsInitializer) {
super(parent, type, guid, initializer);
this.markAsInternalType();
this.devices = {};
for (const { name, descriptor } of initializer.deviceDescriptors)
this.devices[name] = descriptor;

View file

@ -299,6 +299,7 @@ export class Route extends ChannelOwner<channels.RouteChannel> implements api.Ro
constructor(parent: ChannelOwner, type: string, guid: string, initializer: channels.RouteInitializer) {
super(parent, type, guid, initializer);
this.markAsInternalType();
}
request(): Request {
@ -325,7 +326,7 @@ export class Route extends ChannelOwner<channels.RouteChannel> implements api.Ro
async abort(errorCode?: string) {
await this._handleRoute(async () => {
await this._raceWithTargetClose(this._channel.abort({ requestUrl: this.request()._initializer.url, errorCode }));
await this._raceWithTargetClose(this._channel.abort({ errorCode }));
});
}
@ -409,7 +410,6 @@ export class Route extends ChannelOwner<channels.RouteChannel> implements api.Ro
headers['content-length'] = String(length);
await this._raceWithTargetClose(this._channel.fulfill({
requestUrl: this.request()._initializer.url,
status: statusOption || 200,
headers: headersObjectToArray(headers),
body,
@ -421,7 +421,7 @@ export class Route extends ChannelOwner<channels.RouteChannel> implements api.Ro
async continue(options: FallbackOverrides = {}) {
await this._handleRoute(async () => {
this.request()._applyFallbackOverrides(options);
await this._innerContinue();
await this._innerContinue(false /* isFallback */);
});
}
@ -436,18 +436,15 @@ export class Route extends ChannelOwner<channels.RouteChannel> implements api.Ro
chain.resolve(done);
}
async _innerContinue(internal = false) {
async _innerContinue(isFallback: boolean) {
const options = this.request()._fallbackOverridesForContinue();
return await this._wrapApiCall(async () => {
await this._raceWithTargetClose(this._channel.continue({
requestUrl: this.request()._initializer.url,
return await this._raceWithTargetClose(this._channel.continue({
url: options.url,
method: options.method,
headers: options.headers ? headersObjectToArray(options.headers) : undefined,
postData: options.postDataBuffer,
isFallback: internal,
isFallback,
}));
}, !!internal);
}
}

View file

@ -31,20 +31,18 @@ export class Tracing extends ChannelOwner<channels.TracingChannel> implements ap
constructor(parent: ChannelOwner, type: string, guid: string, initializer: channels.TracingInitializer) {
super(parent, type, guid, initializer);
this.markAsInternalType();
}
async start(options: { name?: string, title?: string, snapshots?: boolean, screenshots?: boolean, sources?: boolean, _live?: boolean } = {}) {
this._includeSources = !!options.sources;
const traceName = await this._wrapApiCall(async () => {
await this._channel.tracingStart({
name: options.name,
snapshots: options.snapshots,
screenshots: options.screenshots,
live: options._live,
});
const response = await this._channel.tracingStartChunk({ name: options.name, title: options.title });
return response.traceName;
}, true);
const { traceName } = await this._channel.tracingStartChunk({ name: options.name, title: options.title });
await this._startCollectingStacks(traceName);
}
@ -63,16 +61,12 @@ export class Tracing extends ChannelOwner<channels.TracingChannel> implements ap
}
async stopChunk(options: { path?: string } = {}) {
await this._wrapApiCall(async () => {
await this._doStopChunk(options.path);
}, true);
}
async stop(options: { path?: string } = {}) {
await this._wrapApiCall(async () => {
await this._doStopChunk(options.path);
await this._channel.tracingStop();
}, true);
}
private async _doStopChunk(filePath: string | undefined) {

View file

@ -2116,7 +2116,6 @@ scheme.RouteRedirectNavigationRequestParams = tObject({
scheme.RouteRedirectNavigationRequestResult = tOptional(tObject({}));
scheme.RouteAbortParams = tObject({
errorCode: tOptional(tString),
requestUrl: tString,
});
scheme.RouteAbortResult = tOptional(tObject({}));
scheme.RouteContinueParams = tObject({
@ -2124,7 +2123,6 @@ scheme.RouteContinueParams = tObject({
method: tOptional(tString),
headers: tOptional(tArray(tType('NameValue'))),
postData: tOptional(tBinary),
requestUrl: tString,
isFallback: tBoolean,
});
scheme.RouteContinueResult = tOptional(tObject({}));
@ -2134,7 +2132,6 @@ scheme.RouteFulfillParams = tObject({
body: tOptional(tString),
isBase64: tOptional(tBoolean),
fetchResponseUid: tOptional(tString),
requestUrl: tString,
});
scheme.RouteFulfillResult = tOptional(tObject({}));
scheme.WebSocketRouteInitializer = tObject({

View file

@ -525,7 +525,7 @@ export abstract class BrowserContext extends SdkObject {
const internalMetadata = serverSideCallMetadata();
const page = await this.newPage(internalMetadata);
await page._setServerRequestInterceptor(handler => {
handler.fulfill({ body: '<html></html>', requestUrl: handler.request().url() }).catch(() => {});
handler.fulfill({ body: '<html></html>' }).catch(() => {});
return true;
});
for (const origin of originsToSave) {
@ -559,7 +559,7 @@ export abstract class BrowserContext extends SdkObject {
isServerSide: false,
});
await page._setServerRequestInterceptor(handler => {
handler.fulfill({ body: '<html></html>', requestUrl: handler.request().url() }).catch(() => {});
handler.fulfill({ body: '<html></html>' }).catch(() => {});
return true;
});
@ -594,7 +594,7 @@ export abstract class BrowserContext extends SdkObject {
const internalMetadata = serverSideCallMetadata();
const page = await this.newPage(internalMetadata);
await page._setServerRequestInterceptor(handler => {
handler.fulfill({ body: '<html></html>', requestUrl: handler.request().url() }).catch(() => {});
handler.fulfill({ body: '<html></html>' }).catch(() => {});
return true;
});
for (const originState of state.origins) {

View file

@ -81,7 +81,6 @@ export class RecorderApp extends EventEmitter implements IRecorderApp {
const file = require.resolve('../../vite/recorder/' + uri);
fs.promises.readFile(file).then(buffer => {
route.fulfill({
requestUrl: route.request().url(),
status: 200,
headers: [
{ name: 'Content-Type', value: mime.getType(path.extname(file)) || 'application/octet-stream' }

View file

@ -3771,7 +3771,6 @@ export type RouteRedirectNavigationRequestOptions = {
export type RouteRedirectNavigationRequestResult = void;
export type RouteAbortParams = {
errorCode?: string,
requestUrl: string,
};
export type RouteAbortOptions = {
errorCode?: string,
@ -3782,7 +3781,6 @@ export type RouteContinueParams = {
method?: string,
headers?: NameValue[],
postData?: Binary,
requestUrl: string,
isFallback: boolean,
};
export type RouteContinueOptions = {
@ -3798,7 +3796,6 @@ export type RouteFulfillParams = {
body?: string,
isBase64?: boolean,
fetchResponseUid?: string,
requestUrl: string,
};
export type RouteFulfillOptions = {
status?: number,

View file

@ -2946,7 +2946,6 @@ Route:
abort:
parameters:
errorCode: string?
requestUrl: string
continue:
parameters:
@ -2956,7 +2955,6 @@ Route:
type: array?
items: NameValue
postData: binary?
requestUrl: string
isFallback: boolean
fulfill:
@ -2969,7 +2967,6 @@ Route:
body: string?
isBase64: boolean?
fetchResponseUid: string?
requestUrl: string
WebSocketRoute:

View file

@ -105,7 +105,6 @@ export const UIModeView: React.FC<{}> = ({
const [singleWorker, setSingleWorker] = React.useState(queryParams.workers === '1');
const [showBrowser, setShowBrowser] = React.useState(queryParams.headed);
const [updateSnapshots, setUpdateSnapshots] = React.useState(queryParams.updateSnapshots === 'all');
const [showRouteActions, setShowRouteActions] = useSetting('show-route-actions', true);
const [darkMode, setDarkMode] = useDarkModeSetting();
const [showScreenshot, setShowScreenshot] = useSetting('screenshot-instead-of-snapshot', false);
@ -526,7 +525,6 @@ export const UIModeView: React.FC<{}> = ({
</Toolbar>
{settingsVisible && <SettingsView settings={[
{ value: darkMode, set: setDarkMode, title: 'Dark mode' },
{ value: showRouteActions, set: setShowRouteActions, title: 'Show route actions' },
{ value: showScreenshot, set: setShowScreenshot, title: 'Show screenshot instead of snapshot' },
]} />}
</div>

View file

@ -24,7 +24,6 @@ import type { ErrorDescription } from './errorsTab';
import type { ConsoleEntry } from './consoleTab';
import { ConsoleTab, useConsoleTabModel } from './consoleTab';
import type * as modelUtil from './modelUtil';
import { isRouteAction } from './modelUtil';
import { NetworkTab, useNetworkTabModel } from './networkTab';
import { SnapshotTab } from './snapshotTab';
import { SourceTab } from './sourceTab';
@ -71,13 +70,8 @@ export const Workbench: React.FunctionComponent<{
const [highlightedLocator, setHighlightedLocator] = React.useState<string>('');
const [selectedTime, setSelectedTime] = React.useState<Boundaries | undefined>();
const [sidebarLocation, setSidebarLocation] = useSetting<'bottom' | 'right'>('propertiesSidebarLocation', 'bottom');
const [showRouteActions, setShowRouteActions] = useSetting('show-route-actions', true);
const [showScreenshot, setShowScreenshot] = useSetting('screenshot-instead-of-snapshot', false);
const filteredActions = React.useMemo(() => {
return (model?.actions || []).filter(action => showRouteActions || !isRouteAction(action));
}, [model, showRouteActions]);
const setSelectedAction = React.useCallback((action: modelUtil.ActionTraceEventInContext | undefined) => {
setSelectedCallId(action?.callId);
setRevealedError(undefined);
@ -292,7 +286,7 @@ export const Workbench: React.FunctionComponent<{
</div>}
<ActionList
sdkLanguage={sdkLanguage}
actions={filteredActions}
actions={model?.actions || []}
selectedAction={model ? selectedAction : undefined}
selectedTime={selectedTime}
setSelectedTime={setSelectedTime}
@ -312,7 +306,6 @@ export const Workbench: React.FunctionComponent<{
id: 'settings',
title: 'Settings',
component: <SettingsView settings={[
{ value: showRouteActions, set: setShowRouteActions, title: 'Show route actions' },
{ value: showScreenshot, set: setShowScreenshot, title: 'Show screenshot instead of snapshot' }
]}/>,
};

View file

@ -71,10 +71,12 @@ class TraceViewerPage {
return await this.page.waitForSelector(`.list-view-entry:has-text("${action}") .action-icons`);
}
@step
async selectAction(title: string, ordinal: number = 0) {
await this.page.locator(`.action-title:has-text("${title}")`).nth(ordinal).click();
}
@step
async selectSnapshot(name: string) {
await this.page.click(`.snapshot-tab .tabbed-pane-tab-label:has-text("${name}")`);
}

View file

@ -966,28 +966,6 @@ test('should open two trace files of the same test', async ({ context, page, req
]);
});
test('should include requestUrl in route.fulfill', async ({ page, runAndTrace, browserName }) => {
await page.route('**/*', route => {
void route.fulfill({
status: 200,
headers: {
'content-type': 'text/html'
},
body: 'Hello there!'
});
});
const traceViewer = await runAndTrace(async () => {
await page.goto('http://test.com');
});
// Render snapshot, check expectations.
await traceViewer.selectAction('route.fulfill');
await traceViewer.page.locator('.tabbed-pane-tab-label', { hasText: 'Call' }).click();
const callLine = traceViewer.page.locator('.call-line');
await expect(callLine.getByText('status')).toContainText('200');
await expect(callLine.getByText('requestUrl')).toContainText('http://test.com');
});
test('should not crash with broken locator', async ({ page, runAndTrace, server }) => {
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/21832' });
const traceViewer = await runAndTrace(async () => {
@ -1001,37 +979,6 @@ test('should not crash with broken locator', async ({ page, runAndTrace, server
await expect(header).toBeVisible();
});
test('should include requestUrl in route.continue', async ({ page, runAndTrace, server }) => {
await page.route('**/*', route => {
void route.continue({ url: server.EMPTY_PAGE });
});
const traceViewer = await runAndTrace(async () => {
await page.goto('http://test.com');
});
// Render snapshot, check expectations.
await traceViewer.selectAction('route.continue');
await traceViewer.page.locator('.tabbed-pane-tab-label', { hasText: 'Call' }).click();
const callLine = traceViewer.page.locator('.call-line');
await expect(callLine.getByText('requestUrl')).toContainText('http://test.com');
await expect(callLine.getByText(/^url:.*/)).toContainText(server.EMPTY_PAGE);
});
test('should include requestUrl in route.abort', async ({ page, runAndTrace, server }) => {
await page.route('**/*', route => {
void route.abort();
});
const traceViewer = await runAndTrace(async () => {
await page.goto('http://test.com').catch(() => {});
});
// Render snapshot, check expectations.
await traceViewer.selectAction('route.abort');
await traceViewer.page.locator('.tabbed-pane-tab-label', { hasText: 'Call' }).click();
const callLine = traceViewer.page.locator('.call-line');
await expect(callLine.getByText('requestUrl')).toContainText('http://test.com');
});
test('should serve overridden request', async ({ page, runAndTrace, server }) => {
server.setRoute('/custom.css', (req, res) => {
res.writeHead(200, {
@ -1418,7 +1365,7 @@ test('should show correct request start time', {
expect(parseMillis(start)).toBeLessThan(1000);
});
test('should allow hiding route actions', {
test('should not record route actions', {
annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/30970' },
}, async ({ page, runAndTrace, server }) => {
const traceViewer = await runAndTrace(async () => {
@ -1428,28 +1375,9 @@ test('should allow hiding route actions', {
await page.goto(server.EMPTY_PAGE);
});
// Routes are visible by default.
await expect(traceViewer.actionTitles).toHaveText([
/page.route/,
/page.goto.*empty.html/,
/route.fulfill/,
]);
await traceViewer.page.getByText('Settings').click();
await expect(traceViewer.page.getByRole('checkbox', { name: 'Show route actions' })).toBeChecked();
await traceViewer.page.getByRole('checkbox', { name: 'Show route actions' }).uncheck();
await traceViewer.page.getByText('Actions', { exact: true }).click();
await expect(traceViewer.actionTitles).toHaveText([
/page.goto.*empty.html/,
]);
await traceViewer.page.getByText('Settings').click();
await traceViewer.page.getByRole('checkbox', { name: 'Show route actions' }).check();
await traceViewer.page.getByText('Actions', { exact: true }).click();
await expect(traceViewer.actionTitles).toHaveText([
/page.route/,
/page.goto.*empty.html/,
/route.fulfill/,
]);
});

View file

@ -66,10 +66,9 @@ test('should collect trace with resources, but no js', async ({ context, page, s
test('should use the correct apiName for event driven callbacks', async ({ context, page, server }, testInfo) => {
await context.tracing.start();
// route.* calls should not be included in the trace
await page.route('**/empty.html', route => route.continue());
// page.goto -> page.route should be included in the trace since its handled.
await page.goto(server.PREFIX + '/empty.html');
// page.route -> internalContinue should not be included in the trace since it was handled by Playwright internally.
await page.goto(server.PREFIX + '/grid.html');
// The default internal dialog handler should not provide an action.
@ -87,7 +86,6 @@ test('should use the correct apiName for event driven callbacks', async ({ conte
expect(actions).toEqual([
'page.route',
'page.goto',
'route.continue',
'page.goto',
'page.evaluate',
'page.reload',

View file

@ -1216,7 +1216,6 @@ test('should not nest top level expect into unfinished api calls ', {
' browserContext.newPage',
'page.route',
'page.goto',
'route.fetch',
'expect.toBeVisible',
'page.unrouteAll',
'After Hooks',

View file

@ -546,37 +546,6 @@ fixture | fixture: browser
`);
});
test('should not nest page.continue inside page.goto steps', async ({ runInlineTest }) => {
const result = await runInlineTest({
'reporter.ts': stepIndentReporter,
'playwright.config.ts': `module.exports = { reporter: './reporter', };`,
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('pass', async ({ page }) => {
await page.route('**/*', route => route.fulfill('<html></html>'));
await page.goto('http://localhost:1234');
});
`
}, { reporter: '' });
expect(result.exitCode).toBe(0);
expect(result.output).toBe(`
hook |Before Hooks
fixture | fixture: browser
pw:api | browserType.launch
fixture | fixture: context
pw:api | browser.newContext
fixture | fixture: page
pw:api | browserContext.newPage
pw:api |page.route @ a.test.ts:4
pw:api |page.goto(http://localhost:1234) @ a.test.ts:5
pw:api |route.fulfill @ a.test.ts:4
hook |After Hooks
fixture | fixture: page
fixture | fixture: context
`);
});
test('should not propagate errors from within toPass', async ({ runInlineTest }) => {
const result = await runInlineTest({
'reporter.ts': stepIndentReporter,