quic: apply multiple quality improvements to js side

jasnell · jasnell · commit f0af79ef2115 · 2026-04-09T17:59:36.000-07:00
diff --git a/doc/api/quic.md b/doc/api/quic.md
@@ -815,7 +815,7 @@ added: v23.8.0
 
 * Type: {bigint}
 
-### `sessionStats.maxBytesInFlights`
+### `sessionStats.maxBytesInFlight`
 
 <!-- YAML
 added: v23.8.0
diff --git a/lib/internal/quic/quic.js b/lib/internal/quic/quic.js
@@ -637,6 +637,51 @@ function validateBody(body) {
   ], body);
 }
 
+/**
+ * Parses an alternating [name, value, name, value, ...] array from C++
+ * into a plain header object. Multi-value headers become arrays.
+ * @param {string[]} pairs
+ * @returns {object}
+ */
+function parseHeaderPairs(pairs) {
+  assert(ArrayIsArray(pairs));
+  assert(pairs.length % 2 === 0);
+  const block = { __proto__: null };
+  for (let n = 0; n + 1 < pairs.length; n += 2) {
+    if (block[pairs[n]] !== undefined) {
+      block[pairs[n]] = [block[pairs[n]], pairs[n + 1]];
+    } else {
+      block[pairs[n]] = pairs[n + 1];
+    }
+  }
+  return block;
+}
+
+/**
+ * Applies session and stream callbacks from an options object to a session.
+ * @param {QuicSession} session
+ * @param {object} cbs
+ */
+function applyCallbacks(session, cbs) {
+  if (cbs.onstream) session.onstream = cbs.onstream;
+  if (cbs.ondatagram) session.ondatagram = cbs.ondatagram;
+  if (cbs.ondatagramstatus) session.ondatagramstatus = cbs.ondatagramstatus;
+  if (cbs.onpathvalidation) session.onpathvalidation = cbs.onpathvalidation;
+  if (cbs.onsessionticket) session.onsessionticket = cbs.onsessionticket;
+  if (cbs.onversionnegotiation) session.onversionnegotiation = cbs.onversionnegotiation;
+  if (cbs.onhandshake) session.onhandshake = cbs.onhandshake;
+  if (cbs.onnewtoken) session.onnewtoken = cbs.onnewtoken;
+  if (cbs.onorigin) session.onorigin = cbs.onorigin;
+  if (cbs.onheaders || cbs.ontrailers || cbs.onwanttrailers) {
+    session[kStreamCallbacks] = {
+      __proto__: null,
+      onheaders: cbs.onheaders,
+      ontrailers: cbs.ontrailers,
+      onwanttrailers: cbs.onwanttrailers,
+    };
+  }
+}
+
 // Functions used specifically for internal or assertion purposes only.
 let getQuicStreamState;
 let getQuicSessionState;
@@ -713,6 +758,13 @@ class QuicStream {
     }
   }
 
+  #assertHeadersSupported() {
+    if (getQuicSessionState(this.#session).headersSupported === 2) {
+      throw new ERR_INVALID_STATE(
+        'The negotiated QUIC application protocol does not support headers');
+    }
+  }
+
   /**
    * @param {symbol} privateSymbol
    * @param {object} handle
@@ -810,10 +862,7 @@ class QuicStream {
       this.#onheaders = undefined;
       this.#state[kWantsHeaders] = false;
     } else {
-      if (getQuicSessionState(this.#session).headersSupported === 2) {
-        throw new ERR_INVALID_STATE(
-          'The negotiated QUIC application protocol does not support headers');
-      }
+      this.#assertHeadersSupported();
       validateFunction(fn, 'onheaders');
       this.#onheaders = FunctionPrototypeBind(fn, this);
       this.#state[kWantsHeaders] = true;
@@ -832,10 +881,7 @@ class QuicStream {
       this.#ontrailers = undefined;
       this.#state[kWantsTrailers] = false;
     } else {
-      if (getQuicSessionState(this.#session).headersSupported === 2) {
-        throw new ERR_INVALID_STATE(
-          'The negotiated QUIC application protocol does not support headers');
-      }
+      this.#assertHeadersSupported();
       validateFunction(fn, 'ontrailers');
       this.#ontrailers = FunctionPrototypeBind(fn, this);
       this.#state[kWantsTrailers] = true;
@@ -853,10 +899,7 @@ class QuicStream {
     if (fn === undefined) {
       this.#onwanttrailers = undefined;
     } else {
-      if (getQuicSessionState(this.#session).headersSupported === 2) {
-        throw new ERR_INVALID_STATE(
-          'The negotiated QUIC application protocol does not support headers');
-      }
+      this.#assertHeadersSupported();
       validateFunction(fn, 'onwanttrailers');
       this.#onwanttrailers = FunctionPrototypeBind(fn, this);
     }
@@ -940,7 +983,7 @@ class QuicStream {
 
   /**
    * True if the stream has been destroyed.
-   * @returns {boolean}
+   * @type {boolean}
    */
   get destroyed() {
     QuicStream.#assertIsQuicStream(this);
@@ -1186,18 +1229,7 @@ class QuicStream {
 
   [kHeaders](headers, kind) {
     assert(this.#onheaders, 'Unexpected stream headers event');
-    assert(ArrayIsArray(headers));
-    assert(headers.length % 2 === 0);
-    const block = {
-      __proto__: null,
-    };
-    for (let n = 0; n + 1 < headers.length; n += 2) {
-      if (block[headers[n]] !== undefined) {
-        block[headers[n]] = [block[headers[n]], headers[n + 1]];
-      } else {
-        block[headers[n]] = headers[n + 1];
-      }
-    }
+    const block = parseHeaderPairs(headers);
 
     // Buffer initial headers so stream.headers returns them.
     if (kind === 'initial' && this.#headers === undefined) {
@@ -1213,17 +1245,7 @@ class QuicStream {
     // If we received trailers from the peer, dispatch them.
     if (headers !== undefined) {
       if (this.#ontrailers) {
-        assert(ArrayIsArray(headers));
-        assert(headers.length % 2 === 0);
-        const block = { __proto__: null };
-        for (let n = 0; n + 1 < headers.length; n += 2) {
-          if (block[headers[n]] !== undefined) {
-            block[headers[n]] = [block[headers[n]], headers[n + 1]];
-          } else {
-            block[headers[n]] = headers[n + 1];
-          }
-        }
-        this.#ontrailers(block);
+        this.#ontrailers(parseHeaderPairs(headers));
       }
       return;
     }
@@ -1248,7 +1270,7 @@ class QuicStream {
       depth: options.depth == null ? null : options.depth - 1,
     };
 
-    return `Stream ${inspect({
+    return `QuicStream ${inspect({
       __proto__: null,
       id: this.id,
       direction: this.direction,
@@ -1792,7 +1814,7 @@ class QuicSession {
 
       debug('gracefully closing the session');
 
-      this.#handle?.gracefulClose();
+      this.#handle.gracefulClose();
       if (onSessionClosingChannel.hasSubscribers) {
         onSessionClosingChannel.publish({
           session: this,
@@ -1890,7 +1912,6 @@ class QuicSession {
     this.#peerCertificate = undefined;
     this.#ephemeralKeyInfo = undefined;
 
-
     // Destroy the underlying C++ handle
     this.#handle.destroy();
     this.#handle = undefined;
@@ -1920,29 +1941,20 @@ class QuicSession {
     debug('finishing closing the session with an error', errorType, code, reason);
     // Otherwise, errorType indicates the type of error that occurred, code indicates
     // the specific error, and reason is an optional string describing the error.
+    // code !== 0n here (the early return above handles code === 0n)
     switch (errorType) {
       case 0: /* Transport Error */
-        if (code === 0n) {
-          this.destroy();
-        } else {
-          this.destroy(new ERR_QUIC_TRANSPORT_ERROR(code, reason));
-        }
+        this.destroy(new ERR_QUIC_TRANSPORT_ERROR(code, reason));
         break;
       case 1: /* Application Error */
-        if (code === 0n) {
-          this.destroy();
-        } else {
-          this.destroy(new ERR_QUIC_APPLICATION_ERROR(code, reason));
-        }
+        this.destroy(new ERR_QUIC_APPLICATION_ERROR(code, reason));
         break;
       case 2: /* Version Negotiation Error */
         this.destroy(new ERR_QUIC_VERSION_NEGOTIATION_ERROR());
         break;
-      case 3: /* Idle close */ {
-        // An idle close is not really an error. We can just destroy.
+      case 3: /* Idle close */
         this.destroy();
         break;
-      }
     }
   }
 
@@ -2069,9 +2081,6 @@ class QuicSession {
     if (this.#onversionnegotiation) {
       this.#onversionnegotiation(version, requestedVersions, supportedVersions);
     }
-    // Version negotiation is always a fatal event - the session must be
-    // destroyed regardless of whether the callback is set.
-    this.destroy(new ERR_QUIC_VERSION_NEGOTIATION_ERROR());
     if (onSessionVersionNegotiationChannel.hasSubscribers) {
       onSessionVersionNegotiationChannel.publish({
         __proto__: null,
@@ -2081,6 +2090,9 @@ class QuicSession {
         session: this,
       });
     }
+    // Version negotiation is always a fatal event - the session must be
+    // destroyed regardless of whether the callback is set.
+    this.destroy(new ERR_QUIC_VERSION_NEGOTIATION_ERROR());
   }
 
   /**
@@ -2515,19 +2527,6 @@ class QuicEndpoint {
     validateObject(options, 'options');
     const {
       sessionTicket,
-      onstream,
-      ondatagram,
-      ondatagramstatus,
-      onpathvalidation,
-      onsessionticket,
-      onversionnegotiation,
-      onhandshake,
-      onnewtoken,
-      onorigin,
-      // Stream-level callbacks.
-      onheaders,
-      ontrailers,
-      onwanttrailers,
       ...rest
     } = options;
 
@@ -2539,24 +2538,7 @@ class QuicEndpoint {
     const session = this.#newSession(handle);
     // Set callbacks before any async work to avoid missing events
     // that fire during or immediately after the handshake.
-    if (onstream) session.onstream = onstream;
-    if (ondatagram) session.ondatagram = ondatagram;
-    if (ondatagramstatus) session.ondatagramstatus = ondatagramstatus;
-    if (onpathvalidation) session.onpathvalidation = onpathvalidation;
-    if (onsessionticket) session.onsessionticket = onsessionticket;
-    if (onversionnegotiation) session.onversionnegotiation = onversionnegotiation;
-    if (onhandshake) session.onhandshake = onhandshake;
-    if (onnewtoken) session.onnewtoken = onnewtoken;
-    if (onorigin) session.onorigin = onorigin;
-    // Store stream-level callbacks for application to client-created streams.
-    if (onheaders || ontrailers || onwanttrailers) {
-      session[kStreamCallbacks] = {
-        __proto__: null,
-        onheaders,
-        ontrailers,
-        onwanttrailers,
-      };
-    }
+    applyCallbacks(session, options);
     return session;
   }
 
@@ -2735,26 +2717,7 @@ class QuicEndpoint {
     // the onsession callback, to avoid missing events that fire
     // during or immediately after the handshake.
     if (this.#sessionCallbacks) {
-      const cbs = this.#sessionCallbacks;
-      if (cbs.onstream) session.onstream = cbs.onstream;
-      if (cbs.ondatagram) session.ondatagram = cbs.ondatagram;
-      if (cbs.ondatagramstatus) session.ondatagramstatus = cbs.ondatagramstatus;
-      if (cbs.onpathvalidation) session.onpathvalidation = cbs.onpathvalidation;
-      if (cbs.onsessionticket) session.onsessionticket = cbs.onsessionticket;
-      if (cbs.onversionnegotiation) session.onversionnegotiation = cbs.onversionnegotiation;
-      if (cbs.onhandshake) session.onhandshake = cbs.onhandshake;
-      if (cbs.onnewtoken) session.onnewtoken = cbs.onnewtoken;
-      if (cbs.onorigin) session.onorigin = cbs.onorigin;
-      // Store stream-level callbacks on the session for application
-      // to each new incoming stream.
-      if (cbs.onheaders || cbs.ontrailers || cbs.onwanttrailers) {
-        session[kStreamCallbacks] = {
-          __proto__: null,
-          onheaders: cbs.onheaders,
-          ontrailers: cbs.ontrailers,
-          onwanttrailers: cbs.onwanttrailers,
-        };
-      }
+      applyCallbacks(session, this.#sessionCallbacks);
     }
     if (onEndpointServerSessionChannel.hasSubscribers) {
       onEndpointServerSessionChannel.publish({
diff --git a/lib/internal/quic/state.js b/lib/internal/quic/state.js
@@ -185,8 +185,8 @@ class QuicEndpointState {
   }
 
   /**
-   * The number of underlying callbacks that are pending. If the session
-   * is closing, these are the number of callbacks that the session is
+   * The number of underlying callbacks that are pending. If the endpoint
+   * is closing, these are the number of callbacks that the endpoint is
    * waiting on before it can be closed.
    * @type {bigint}
    */
diff --git a/lib/internal/quic/stats.js b/lib/internal/quic/stats.js
@@ -302,7 +302,7 @@ class QuicEndpointStats {
    * True if this QuicEndpointStats object is still connected to the underlying
    * Endpoint stats source. If this returns false, then the stats object is
    * no longer being updated and should be considered stale.
-   * @returns {boolean}
+   * @type {boolean}
    */
   get isConnected() {
     return !this.#disconnected;
@@ -324,7 +324,7 @@ class QuicSessionStats {
 
   /**
    * @param {symbol} privateSymbol
-   * @param {BigUint64Array} buffer
+   * @param {ArrayBuffer} buffer
    */
   constructor(privateSymbol, buffer) {
     // We use the kPrivateConstructor symbol to restrict the ability to
@@ -384,7 +384,7 @@ class QuicSessionStats {
   }
 
   /** @type {bigint} */
-  get maxBytesInFlights() {
+  get maxBytesInFlight() {
     return this.#handle[IDX_STATS_SESSION_MAX_BYTES_IN_FLIGHT];
   }
 
@@ -499,7 +499,7 @@ class QuicSessionStats {
       bidiOutStreamCount: `${this.bidiOutStreamCount}`,
       uniInStreamCount: `${this.uniInStreamCount}`,
       uniOutStreamCount: `${this.uniOutStreamCount}`,
-      maxBytesInFlights: `${this.maxBytesInFlights}`,
+      maxBytesInFlight: `${this.maxBytesInFlight}`,
       bytesInFlight: `${this.bytesInFlight}`,
       blockCount: `${this.blockCount}`,
       cwnd: `${this.cwnd}`,
@@ -544,7 +544,7 @@ class QuicSessionStats {
       bidiOutStreamCount: this.bidiOutStreamCount,
       uniInStreamCount: this.uniInStreamCount,
       uniOutStreamCount: this.uniOutStreamCount,
-      maxBytesInFlights: this.maxBytesInFlights,
+      maxBytesInFlight: this.maxBytesInFlight,
       bytesInFlight: this.bytesInFlight,
       blockCount: this.blockCount,
       cwnd: this.cwnd,
@@ -572,7 +572,7 @@ class QuicSessionStats {
    * True if this QuicSessionStats object is still connected to the underlying
    * Session stats source. If this returns false, then the stats object is
    * no longer being updated and should be considered stale.
-   * @returns {boolean}
+   * @type {boolean}
    */
   get isConnected() {
     return !this.#disconnected;
@@ -697,7 +697,7 @@ class QuicStreamStats {
       depth: options.depth == null ? null : options.depth - 1,
     };
 
-    return `StreamStats ${inspect({
+    return `QuicStreamStats ${inspect({
       connected: this.isConnected,
       createdAt: this.createdAt,
       openedAt: this.openedAt,
@@ -717,7 +717,7 @@ class QuicStreamStats {
    * True if this QuicStreamStats object is still connected to the underlying
    * Stream stats source. If this returns false, then the stats object is
    * no longer being updated and should be considered stale.
-   * @returns {boolean}
+   * @type {boolean}
    */
   get isConnected() {
     return !this.#disconnected;
diff --git a/test/parallel/test-quic-internal-endpoint-stats-state.mjs b/test/parallel/test-quic-internal-endpoint-stats-state.mjs
