feat(har): adjust capturing WebSocket data based on content config#41124
feat(har): adjust capturing WebSocket data based on content config#41124dcrousso wants to merge 1 commit into
WebSocket data based on content config#41124Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| expect(messages[0].time).toBeLessThanOrEqual(messages[1].time); | ||
| }); | ||
|
|
||
| it('should attach binary websocket messages', async ({ contextFactory, server }, testInfo) => { |
There was a problem hiding this comment.
Somehow it feels like we should combine this and previous tests together.
| expect(wsEntry.response.content._file).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('should omit binary websocket messages', async ({ contextFactory, server }, testInfo) => { |
|
|
||
| let saveMessages: (() => void) | undefined; | ||
| if (this._options.content === 'attach') { | ||
| saveMessages = () => { |
There was a problem hiding this comment.
If we want to support websockets in the trace, we'll have to do this separately in HarRecorder vs Tracing. Let's move this out to HarRecorder through HarTracerDelegate right away?
There was a problem hiding this comment.
if it's OK with you, id rather handle this when i actually work on supporting WebSocket in tracing as right now i dont really know what the best approach would be (i.e. i'd hate to implement something for it now and then have to completely change it once i start working on tracing)
i plan to do that next so this shouldn't be "stale" for too long
| }), | ||
| eventsHelper.addEventListener(webSocket, network.WebSocket.Events.FrameSent, ({ opcode, data, wallTimeMs }: { opcode: number, data: string, wallTimeMs: number }) => { | ||
| harEntry._webSocketMessages!.push({ type: 'send', time: this._options.omitTiming ? -1 : wallTimeMs, opcode, data }); | ||
| if (this._options.content !== 'omit') |
There was a problem hiding this comment.
Shouldn't this be a separate option? I can imagine not everyone would like websocket traffic (that is sometimes very heavy) to be recorder. Perhaps overall, an option for (not) recording websockets?
There was a problem hiding this comment.
we discussed offline and felt that it's probably not supper common for a developer to want the network metadata (e.g. headers, timing, etc.) without also wanting the content for a WebSocket, so in addition to the existing content settings they could easily add a urlFilter to ignore all ws:///wss:// instead
if needed we could add an environment variable, but that might be overeager
dcrousso
force-pushed
the
feat-har-WebSocket-content
branch
from
5bac817 to
1e68c65
Compare
Test results for "MCP"7266 passed, 1119 skipped Merge workflow run. |
Test results for "tests 1"6 flaky39557 passed, 775 skipped Merge workflow run. |
2 participants
only set
_webSocketMessagesifcontent: 'embed'for
content: 'attach', save the array as a.jsonalso, unlike other requests that have a single response, a
WebSocketcan receive multiple frames, meaning that we don't finish theEntryuntil theWebSocketis closed, which delays when the captured frames are saved, so make sure to at least save what's already there if the recording is stopped before theWebSocketis closed