feat(trace): split API requests into a separate network stream - #42073
feat(trace): split API requests into a separate network stream#42073dcrousso wants to merge 1 commit into
Conversation
pavelfeldman
left a comment
There was a problem hiding this comment.
On a high level, I expect the fetch context to have its own trace file and its own network file. No apiNetworkFile in the recording state/ no version bump. API request context is just another context with its own trace and network.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as spam.
This comment was marked as spam.
browser HAR recording listened to the context request client and mixed API calls with page traffic make API request listener registration opt out for browser HAR while preserving tracing and standalone request HAR
2fad1cf to
9ff658f
Compare
|
i did a bunch of digging and i think this is an even simpler approach as it just prevents API requests from being included in HAR while not doing anything to tracing |
Test results for "tests 1"3 flaky51186 passed, 1190 skipped Merge workflow run. |
Test results for "MCP"2 failed 7970 passed, 1284 skipped Merge workflow run. |
🟡 Neither failure is clearly on this PR, but I can't fully clear oneHi, I'm the Playwright bot and I took a first look at the CI failures. Two MCP tests failed in the latest run, both far from what this PR touches (it only splits API requests out of HAR recording in DetailsOverall: the diff is confined to server-side HAR recording. Neither failing test exercises that path (one is Pre-existing flake / infra
Uncertain
Triaged by the Playwright bot - agent run |
| const content = options.content || 'embed'; | ||
| this._tracer = new HarTracer(context, page, this, { | ||
| content, | ||
| includeAPIRequests: context instanceof APIRequestContext, |
There was a problem hiding this comment.
I don't quite understand the logic that "enables" api requests when api request context is passed.
There was a problem hiding this comment.
this is to handle things like playwright.request.newContext().tracing.startHar() vs a "regular" BrowserContext (i.e. the latter will not include API requests in the HAR whereas the former will only include API requests in the HAR)
| ]; | ||
| this._eventListeners = []; | ||
| if (this._options.includeAPIRequests !== false) { | ||
| const apiRequest = this._context instanceof APIRequestContext ? this._context : this._context.fetchRequest; |
There was a problem hiding this comment.
Also unclear why we would switch to fetch context in this case.
There was a problem hiding this comment.
note that this check existed before this PR
a standalone APIRequestContext emits these events directly, whereas for a BrowserContext they're emitted by the owned BrowserContextAPIRequestContext
the existing
.networkstream mixed browser traffic withAPIRequestContexttrafficrecord
_apiRequestentries in.api.network, archive it astrace.api.network, and ingest it alongside.networkbump the trace format to
9so older viewers reject traces that have the new stream