feat(websocket): expand diagnostics channel payloads by islandryu · Pull Request #4888 · nodejs/undici

islandryu · 2026-03-13T08:56:49Z

This relates to...

Rationale

I would like to extend the WebSocket events in order to send CDP data.

Changes

Introduces undici:websocket:created, undici:websocket:handshakeRequest, frameSent, frameReceived, and frameError coverage.

Features

Bug Fixes

N/A

Breaking Changes and Deprecations

Status

codecov-commenter · 2026-03-13T09:14:24Z

Codecov Report

❌ Patch coverage is 71.22302% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.84%. Comparing base (0d7ec33) to head (69af2db).
⚠️ Report is 17 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/core/diagnostics.js	14.28%	30 Missing ⚠️
lib/web/websocket/sender.js	86.95%	6 Missing ⚠️
lib/web/websocket/receiver.js	89.47%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4888      +/-   ##
==========================================
- Coverage   92.89%   92.84%   -0.06%     
==========================================
  Files         112      112              
  Lines       35665    35771     +106     
==========================================
+ Hits        33131    33211      +80     
- Misses       2534     2560      +26

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

mcollina

lgtm

mcollina · 2026-03-13T21:33:00Z

lib/web/websocket/sender.js

+
+  for (let i = 0; i < payloadData.length; ++i) {
+    payloadData[i] = frame[payloadOffset + i] ^ maskKey[i & 3]
+  }


diagnostics channels are meant to be low overhead. This is the opposite of that

Changed to use the data before masking.

KhafraDev

I don't like the changes. Diagnostics channels should be unobtrusive.

KhafraDev · 2026-03-13T21:33:28Z

lib/web/websocket/sender.js

          this.#socket.uncork()
        } else {
          // direct writing
+          publishFrame(this.#websocket, hint === sendHints.text ? opcodes.TEXT : opcodes.BINARY, true, toBuffer(item, hint))


the toBuffer will make WebSocket slower for the majority of people not using diagnostics channels

Moved the toBuffer call after the hasSubscribers check.

KhafraDev · 2026-03-13T21:33:38Z

lib/web/websocket/sender.js

    const node = {
      promise: item.arrayBuffer().then((ab) => {
        node.promise = null
+        publishFrame(this.#websocket, opcodes.BINARY, true, new Uint8Array(ab))


islandryu · 2026-03-20T02:50:21Z

@mcollina @KhafraDev
Could you take a look?

tsctx · 2026-03-20T12:57:40Z

lib/web/websocket/receiver.js

        const rsv3 = buffer[0] & 0x10

        if (!isValidOpcode(opcode)) {
+          this.publishFrameError(new Error('Invalid opcode received'))


Isn't there a cleaner way to handle this? Repeating these same lines before every failWebsocketConnection call seems like it could lead to maintenance issues.

I've made the function common.

tsctx · 2026-03-20T12:58:27Z

lib/web/websocket/stream/websocketstream.js

-        channels.socketError.publish(err)
+        channels.socketError.publish({
+          error: err,
+          websocket: undefined
+        })


What was the reason for this change? Wouldn't this be considered a breaking change?

As you pointed out, there was no need to pass undefined.
Fixed
As you pointed out, this would be a breaking change, so I’ll exclude it from this PR for now.

However, in places like the following, the error is tied to a specific WebSocket, so we need a way to let the subscriber know which WebSocket the error occurred on. I may send this in a separate PR.

https://github.com/islandryu/undici/blob/main/lib/web/websocket/websocket.js#L78

lib/web/websocket/sender.js

tsctx · 2026-03-20T13:00:30Z

lib/web/websocket/sender.js

+        publishFrame(this.#websocket, opcodes.BINARY, true, ab, hint)
+        node.diagnosticInfo.published = true


Why is it published when resolved? Wouldn't that be called before it's actually sent? Also, wouldn't that mess up the order?

Exactly, it should be published when socket writes
Fixed.

tsctx · 2026-03-20T13:02:54Z

lib/web/websocket/sender.js

+  channels.frameSent.publish({
+    websocket,
+    opcode,
+    mask,


Since this is client-side only, masked is always true. Is this really necessary?

Indeed, it didn’t seem like there are cases where the mask changes in Undici.
Fixed

tsctx · 2026-03-20T13:10:52Z

lib/web/websocket/receiver.js

+          channels.frameReceived.publish({
+            websocket: this.#handler.websocket,
+            opcode: this.#info.opcode,
+            mask: this.#info.masked,


Since this is client-side only, masked is always false. Is this really necessary?

Co-authored-by: tsctx <91457664+tsctx@users.noreply.github.com>

docs/docs/api/DiagnosticsChannel.md

lib/core/diagnostics.js

Co-authored-by: tsctx <91457664+tsctx@users.noreply.github.com>

feat(websocket): expand diagnostics channel payloads

b8b8fec

update websocket debug expectations

639d09f

metcoder95 requested a review from KhafraDev March 13, 2026 11:21

mcollina approved these changes Mar 13, 2026

View reviewed changes

mcollina requested changes Mar 13, 2026

View reviewed changes

KhafraDev requested changes Mar 13, 2026

View reviewed changes

reduce frameSent diagnostics overhead

aa50095

islandryu requested review from KhafraDev and mcollina March 20, 2026 02:46

tsctx requested changes Mar 20, 2026

View reviewed changes

islandryu and others added 6 commits March 21, 2026 21:37

Update lib/web/websocket/sender.js

b8d7ae5

Co-authored-by: tsctx <91457664+tsctx@users.noreply.github.com>

remove websocket from socket_error

0f5ae89

unify failWebsocketConnection and remove mask from frameSent

99d34aa

remove mask from frameSent

30d5a53

adjust the timing of publishFrame

0b44030

fix test

98c9429

islandryu requested a review from tsctx March 21, 2026 13:46

ryuhei shima added 2 commits March 21, 2026 22:50

remove websocket from socket_error

ed80a36

remove websocketError type

0419a1b

tsctx reviewed Mar 21, 2026

View reviewed changes

docs/docs/api/DiagnosticsChannel.md Outdated Show resolved Hide resolved

docs/docs/api/DiagnosticsChannel.md Outdated Show resolved Hide resolved

lib/core/diagnostics.js Outdated Show resolved Hide resolved

islandryu and others added 3 commits March 22, 2026 00:21

Update docs/docs/api/DiagnosticsChannel.md

805449c

Co-authored-by: tsctx <91457664+tsctx@users.noreply.github.com>

Update docs/docs/api/DiagnosticsChannel.md

381a54f

Co-authored-by: tsctx <91457664+tsctx@users.noreply.github.com>

Update lib/core/diagnostics.js

69af2db

Co-authored-by: tsctx <91457664+tsctx@users.noreply.github.com>

		publishFrame(this.#websocket, opcodes.BINARY, true, ab, hint)
		node.diagnosticInfo.published = true

Uh oh!

Conversation

islandryu commented Mar 13, 2026

This relates to...

Rationale

Changes

Features

Bug Fixes

Breaking Changes and Deprecations

Status

Uh oh!

codecov-commenter commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KhafraDev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

islandryu commented Mar 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

islandryu Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Mar 13, 2026 •

edited

Loading

islandryu Mar 21, 2026 •

edited

Loading