feat(har): include WebSocket in .har#41015
Conversation
|
@microsoft-github-policy-service agree company="Microsoft" |
| } | ||
|
|
||
| private _addPageEventListeners(page: Page) { | ||
| if (this._page && page !== this._page) |
There was a problem hiding this comment.
don't we want to listen to all pages?
There was a problem hiding this comment.
it seems like it's possible to create a HarTracer that only looks at stuff for a particular Page
for example, you can see this check also in _createPageEntryIfNeeded and _onRequest
| this._requestTimestamp = timestamp; | ||
| } | ||
|
|
||
| private _normalizeTimestamp(timestamp: number | undefined): number { |
| } | ||
|
|
||
| setTimestampBaseline(wallTime: number, timestamp: number) { | ||
| this._requestWallTime = wallTime; |
There was a problem hiding this comment.
In createHarEntry we only use startedDateTime and _monotonicTime both of which are computed on the playwright end at the moment the entry is created. Maybe it's good enough to have the same for the websocket frames?
There was a problem hiding this comment.
i figured since both Chromium and WebKit already have these values we might as well use them
but yeah if you feel strongly that we shouldn't then i absolutely can remove
| const wsEntry = log.entries.find(e => e.request.url === wsUrl)!; | ||
| expect(wsEntry).toBeTruthy(); | ||
| expect(wsEntry.response.status).toBe(101); | ||
| expect((wsEntry as any)._resourceType).toBe('websocket'); |
There was a problem hiding this comment.
cast the entry once to the har type to avoid repetitive as any
yury-s
left a comment
yury-s
left a comment
There was a problem hiding this comment.
In case of content: 'attach' we need to store the content as external resources, let's also add a test for that.
| return; | ||
|
|
||
| const pageEntry = this._createPageEntryIfNeeded(page); | ||
| const harEntry = createHarEntry(pageEntry?.id, 'GET', url, page.mainFrame().guid, this._options); |
There was a problem hiding this comment.
we'll need to populate headers too
There was a problem hiding this comment.
yeah i realized that after i uploaded this PR 😅
working on that right now
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
dcrousso
force-pushed
the
drousso/feat-har-websocket
branch
from
91ad605 to
9f3d97c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
dcrousso
force-pushed
the
drousso/feat-har-websocket
branch
from
9f3d97c to
b423303
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
add an entry for each `WebSocket` when generating a `.har` also include data for each sent/received frame in a custom property `_webSocketMessages` (for more info see <https://developer.chrome.com/blog/new-in-devtools-76#websocket>) both Chrome and WebKit already capture the `wallTime` for the initial request and a `timestamp` for each subsequent message (i.e. diff the `timestamp` relative to the initial `timestamp` and add the `wallTime` in order to determine the current `wallTime`) unfortunately Firefox does not have this so fall back to `Date.now() / 1000`
dcrousso
force-pushed
the
drousso/feat-har-websocket
branch
from
b423303 to
92e554b
Compare
Test results for "MCP"7181 passed, 1113 skipped Merge workflow run. |
Test results for "tests 1"1 flaky43952 passed, 864 skipped Merge workflow run. |
2 participants
add an entry for each
WebSocketwhen generating a.haralso include data for each sent/received frame in a custom property
_webSocketMessages(for more info see https://developer.chrome.com/blog/new-in-devtools-76#websocket)both Chrome and WebKit already capture the
wallTimefor the initial request and atimestampfor each subsequent message (i.e. diff thetimestamprelative to the initialtimestampand add thewallTimein order to determine the currentwallTime)unfortunately Firefox does not have this so fall back to
Date.now() / 1000fixes #30315