quic: handle application-specific no-error codes

doc/api/quic.md

@@ -1959,10 +1959,10 @@ added: v23.8.0
 * Type: {Promise}
 
 A promise that is fulfilled when the stream is fully closed. It resolves
-when the stream closes cleanly (including idle timeout). It rejects with
+when the stream closes without a QUIC error, including idle timeout and a
+`RESET_STREAM` whose application code represents no error. It rejects with
 an `ERR_QUIC_APPLICATION_ERROR` or `ERR_QUIC_TRANSPORT_ERROR` when the
-stream is closed due to a QUIC error (e.g., stream reset by the peer,
-CONNECTION\_CLOSE with a non-zero error code).
+stream is closed due to a QUIC error.
 
 ### `stream.destroy([error[, options]])`
 
@@ -2191,7 +2191,8 @@ The callback to invoke when the peer aborts a direction of the stream by
 sending a `RESET_STREAM` frame (the peer abandons their writable side, so
 no further data will arrive on our readable side).
 
-The callback receives a Node.js error whose `errorCode` (`bigint`)
+The callback receives `undefined` when the application error code represents
+no error. Otherwise, it receives a Node.js error whose `errorCode` (`bigint`)
 property carries the application error code from the wire frame.
 
 The stream is **not** automatically destroyed when this callback fires —
@@ -2214,7 +2215,8 @@ The callback to invoke when the peer aborts a direction of the stream by
 sending a `STOP_SENDING` frame (the peer asks us to stop writing on our
 writable side).
 
-The callback receives a Node.js error whose `errorCode` (`bigint`)
+The callback receives `undefined` when the application error code represents
+no error. Otherwise, it receives a Node.js error whose `errorCode` (`bigint`)
 property carries the application error code from the wire frame. Read/write.
 
 ### `stream.headers`
@@ -3491,10 +3493,11 @@ selects how the stream's async iterator reports this:
 
 * `'ignore'` - The truncation itself is ignored: only a stream or connection
   error is reported, and any clean abort/cancellation or similar simply ends
-  the stream. A non-zero peer reset, non-zero local stop-sending or connection
-  error still fails, but a truncation with no error at all (an idle timeout,
-  a graceful close, or a plain `stopSending()`) ends the read cleanly with the
-  data received. This matches `stream.closed`, which rejects only on an error.
+  the stream. A peer reset whose application code represents an error, a
+  non-zero local stop-sending, or a connection error still fails. A truncation
+  with no error at all (an idle timeout, a graceful close, or a plain
+  `stopSending()`) ends the read cleanly with the data received. This matches
+  `stream.closed`, which rejects only on an error.
 
 #### `sessionOptions.verifyPeer` (client only)
 
@@ -4091,7 +4094,7 @@ added: v23.8.0
 -->
 
 * `this` {quic.QuicStream}
-* `error` {any}
+* `error` {Error|undefined}
 
 ### Callback: `OnHeadersCallback`
 
@@ -4785,7 +4788,7 @@ added:
 
 * `stream` {quic.QuicStream}
 * `session` {quic.QuicSession}
-* `error` {any} The QUIC error associated with the reset.
+* `error` {Error|undefined} The QUIC error associated with the reset.
 
 Published when a stream receives a RESET\_STREAM frame from the peer,
 indicating the peer has aborted its sending direction. This is a key signal

lib/internal/quic/quic.js

@@ -699,7 +699,7 @@ const endpointRegistry = new SafeSet();
 /**
  * @callback OnStreamErrorCallback
  * @this {QuicStream}
- * @param {any} error
+ * @param {Error|undefined} error
  * @returns {void}
  */
 
@@ -767,15 +767,14 @@ setCallbacks({
   /**
    * Called when the underlying session C++ handle is closed either normally
    * or with an error.
-   * @param {number} errorType
-   * @param {number} code
-   * @param {string} [reason]
-   * @param {string} [errorName] Decoded TLS alert name when `code` is a
-   *   CRYPTO_ERROR; otherwise undefined.
+   * @param {Array} [error] Raw `[type, code, reason, errorName]` data.
    */
-  onSessionClose(errorType, code, reason, errorName) {
-    debug('session close callback', errorType, code, reason, errorName);
-    this[kOwner][kFinishClose](errorType, code, reason, errorName);
+  onSessionClose(error) {
+    if (error !== undefined) {
+      error = convertQuicError(error);
+    }
+    debug('session close callback', error);
+    this[kOwner][kFinishClose](error);
   },
 
   /**
@@ -967,20 +966,16 @@ setCallbacks({
   },
 
   onStreamClose(error) {
-    // Called when the stream C++ handle has been closed. The error is
-    // either undefined (clean close) or a raw array [type, code, reason]
-    // from QuicError::ToV8Value. Convert to a proper Node.js Error.
+    // Called when the stream C++ handle has been closed. The error is either
+    // undefined (clean close) or a raw [type, code, reason, errorName] array
+    // from the session-aware native error conversion.
     if (!this[kOwner]) return;
     if (error !== undefined) {
       error = convertQuicError(error);
     } else if (!this[kOwner].destroyed) {
-      // The stream is closing cleanly, but it may have been reset by the
-      // peer (ReceiveStreamReset) or locally (resetStream). The C++ side
-      // records the reset code in state.resetCode. If set, surface the
-      // reset as the close error so stream.closed rejects -- the reset
-      // was an abnormal termination even if the session closed cleanly.
-      const resetCode = getQuicStreamState(this[kOwner]).resetCode;
-      if (resetCode !== undefined && resetCode > 0n) {
+      const state = getQuicStreamState(this[kOwner]);
+      if (state.resetCodeIsError) {
+        const resetCode = state.resetCode;
         error = makeQuicError(
           'ERR_QUIC_APPLICATION_ERROR',
           'QUIC application error',
@@ -1732,9 +1727,9 @@ class QuicStream {
                               errorCode: inner.stopSendingCode });
       }
 
-      // Non-zero reset is always an error:
+      // A reset carrying an application error always fails the read.
       const peerResetCode = inner.state.resetCode;
-      if (peerResetCode > 0n) {
+      if (inner.state.resetCodeIsError) {
         throw new QuicError(
           `The QUIC stream was reset by the peer with error code ${peerResetCode}`,
           { __proto__: null,
@@ -3914,21 +3909,18 @@ class QuicSession {
   }
 
   /**
-   * @param {number} errorType
-   * @param {number} code
-   * @param {string} [reason]
+   * @param {Error} [error]
    */
-  [kFinishClose](errorType, code, reason, errorName) {
-    // If code is zero, then we closed without an error. Yay! We can destroy
+  [kFinishClose](error) {
+    // If the error is undefined, then we closed without an error. Yay! We can destroy
     // safely without specifying an error.
-    if (code === 0n) {
+    if (error === undefined) {
       debug('finishing closing the session with no error');
       this.destroy();
       return;
     }
 
-    debug('finishing closing the session with an error',
-          errorType, code, reason, errorName);
+    debug('finishing closing the session with an error', error);
 
     // If the local side initiated this close with an error code (via
     // close({ code })), this is an intentional shutdown; not an error.
@@ -3938,44 +3930,7 @@ class QuicSession {
       return;
     }
 
-    // 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).
-    // The errorType values map to ngtcp2_ccerr_type:
-    //   0 = NGTCP2_CCERR_TYPE_TRANSPORT
-    //   1 = NGTCP2_CCERR_TYPE_APPLICATION
-    //   2 = NGTCP2_CCERR_TYPE_VERSION_NEGOTIATION
-    //   3 = NGTCP2_CCERR_TYPE_IDLE_CLOSE
-    //   4 = NGTCP2_CCERR_TYPE_DROP_CONN
-    //   5 = NGTCP2_CCERR_TYPE_RETRY
-    // The DROP_CONN/RETRY cases are typically intercepted before reaching
-    // here (DROP_CONN tears the connection down without notifying us, RETRY
-    // is server-only). The default branch is a safety net so any
-    // unexpected value still completes the close path - without it the
-    // session would leak with `closed` hanging forever.
-    switch (errorType) {
-      case 0: /* Transport Error */
-        this.destroy(makeQuicError('ERR_QUIC_TRANSPORT_ERROR',
-                                   'QUIC transport error',
-                                   'transport', code, reason, errorName));
-        break;
-      case 1: /* Application Error */
-        this.destroy(makeQuicError('ERR_QUIC_APPLICATION_ERROR',
-                                   'QUIC application error',
-                                   'application', code, reason, errorName));
-        break;
-      case 2: /* Version Negotiation Error */
-        this.destroy(new ERR_QUIC_VERSION_NEGOTIATION_ERROR());
-        break;
-      case 3: /* Idle close */
-        this.destroy();
-        break;
-      default:
-        this.destroy(makeQuicError('ERR_QUIC_TRANSPORT_ERROR',
-                                   'QUIC transport error',
-                                   'transport', code, reason, errorName));
-        break;
-    }
+    this.destroy(error);
   }
 
   [kKeylog](line) {

lib/internal/quic/state.js

@@ -112,6 +112,7 @@ const {
   IDX_STATE_STREAM_WRITE_DESIRED_SIZE,
   IDX_STATE_STREAM_BUDGET,
   IDX_STATE_STREAM_RESET_CODE,
+  IDX_STATE_STREAM_RESET_CODE_IS_ERROR,
   IDX_STATE_STREAM_SIZE,
 } = internalBinding('quic');
 
@@ -153,6 +154,7 @@ assert(IDX_STATE_STREAM_WANTS_RESET !== undefined);
 assert(IDX_STATE_STREAM_WANTS_STOP_SENDING !== undefined);
 assert(IDX_STATE_STREAM_WRITE_DESIRED_SIZE !== undefined);
 assert(IDX_STATE_STREAM_RESET_CODE !== undefined);
+assert(IDX_STATE_STREAM_RESET_CODE_IS_ERROR !== undefined);
 
 const kEmptyObject = { __proto__: null };
 
@@ -873,6 +875,14 @@ class QuicStreamState {
       handle, this.#offset + IDX_STATE_STREAM_RESET_CODE, kIsLittleEndian);
   }
 
+  /** @type {boolean} */
+  get resetCodeIsError() {
+    const handle = this.#handle;
+    if (handle === undefined) return undefined;
+    return DataViewPrototypeGetUint8(
+      handle, this.#offset + IDX_STATE_STREAM_RESET_CODE_IS_ERROR) !== 0;
+  }
+
   /** @type {bigint} */
   get writeDesiredSize() {
     const handle = this.#handle;
@@ -924,6 +934,7 @@ class QuicStreamState {
       wantsStopSending,
       early,
       resetCode,
+      resetCodeIsError,
       writeDesiredSize,
       budget,
     } = this;
@@ -943,6 +954,7 @@ class QuicStreamState {
       wantsStopSending,
       early,
       resetCode: `${resetCode}`,
+      resetCodeIsError,
       writeDesiredSize,
       budget,
     };
@@ -979,6 +991,7 @@ class QuicStreamState {
       wantsStopSending,
       early,
       resetCode,
+      resetCodeIsError,
       writeDesiredSize,
       budget,
     } = this;
@@ -998,6 +1011,7 @@ class QuicStreamState {
       wantsStopSending,
       early,
       resetCode,
+      resetCodeIsError,
       writeDesiredSize,
       budget,
     }, opts)}`;

src/quic/application.h

@@ -60,6 +60,12 @@ class Session::Application : public MemoryRetainer {
   // and is not necessarily 0.
   virtual error_code GetNoErrorCode() const = 0;
 
+  // Returns whether a received application error code means "no error".
+  // Applications may override this when unknown codes have special semantics.
+  virtual bool IsNoErrorCode(error_code code) const {
+    return code == GetNoErrorCode();
+  }
+
   // The "internal error code" is the application-level error code used
   // to signal a non-specific failure when no more specific code has
   // been provided by the caller. For example, `writer.fail(reason)` on

src/quic/data.cc

@@ -439,17 +439,6 @@ const char* QuicError::name() const {
 }
 
 MaybeLocal<Value> QuicError::ToV8Value(Environment* env) const {
-  if ((type() == Type::TRANSPORT && code() == NGTCP2_NO_ERROR) ||
-      (type() == Type::APPLICATION &&
-       (code() == 0 || code() == NGHTTP3_H3_NO_ERROR)) ||
-      type() == Type::IDLE_CLOSE) {
-    // Application code 0 is the default no-error code for raw QUIC
-    // applications (DefaultApplication::GetNoErrorCode() returns 0).
-    // NGHTTP3_H3_NO_ERROR (0x100) is the HTTP/3 no-error code.
-    // Idle close is always clean — the session timed out normally.
-    return Undefined(env->isolate());
-  }
-
   Local<Value> type_str;
   if (!node::ToV8Value(env->context(), TypeName(type())).ToLocal(&type_str)) {
     return {};
@@ -464,7 +453,7 @@ MaybeLocal<Value> QuicError::ToV8Value(Environment* env) const {
 
   // Note that per the QUIC specification, the reason, if present, is
   // expected to be UTF-8 encoded. The spec uses the term "SHOULD" here,
-  // which means that is is entirely possible that some QUIC implementation
+  // which means that is entirely possible that some QUIC implementation
   // could choose a different encoding, in which case the conversion here
   // will produce garbage. That's ok though, we're going to use the default
   // assumption that the impl is following the guidelines.

src/quic/data.h

@@ -284,11 +284,11 @@ class QuicError final : public MemoryRetainer {
 
   std::string ToString() const;
 
-  // Returns an array containing [type, code, reason], where type is a string
-  // representation of the error type, code is a bigint representation of the
-  // error code, and reason is a string representation of the error reason, or
-  // undefined if the reason is the empty string. This is expected to be used
-  // to construct a JS error object from the information in JS.
+  // Returns an array containing [type, code, reason, name], where type is a
+  // string representation of the error type, code is a bigint representation
+  // of the error code, and reason and name are strings or undefined. This is a
+  // lossless representation; whether the code means "no error" depends on the
+  // negotiated application and is determined by Session.
   v8::MaybeLocal<v8::Value> ToV8Value(Environment* env) const;
   inline v8::MaybeLocal<v8::Value> ToV8Value(Realm* realm) const {
     return ToV8Value(realm->env());

src/quic/http3.cc

@@ -37,6 +37,9 @@ constexpr size_t kSessionTicketAppDataHeaderSize = 6;  // type + version + crc
 constexpr size_t kSessionTicketAppDataPayloadSize =
     kSessionTicketAppDataSize - kSessionTicketAppDataHeaderSize;
 
+// RFC 9297 defines H3_DATAGRAM_ERROR, but nghttp3 does not expose it.
+constexpr error_code kH3DatagramError = 0x33;
+
 inline void WriteBE32(uint8_t* buf, uint32_t val) {
   buf[0] = static_cast<uint8_t>((val >> 24) & 0xff);
   buf[1] = static_cast<uint8_t>((val >> 16) & 0xff);
@@ -186,6 +189,22 @@ class Http3ApplicationImpl final : public Session::Application {
 
   error_code GetNoErrorCode() const override { return NGHTTP3_H3_NO_ERROR; }
 
+  bool IsNoErrorCode(error_code code) const override {
+    if (code == NGHTTP3_H3_NO_ERROR) return true;
+    if (code == kH3DatagramError) return false;
+
+    switch (code) {
+#define V(name) case NGHTTP3_##name:
+      HTTP3_APPLICATION_ERRORS(V)
+#undef V
+      return false;
+      default:
+        // RFC 9114 Section 8 requires unknown HTTP/3 error codes to be
+        // treated as equivalent to H3_NO_ERROR.
+        return true;
+    }
+  }
+
   // HTTP/3 defines H3_INTERNAL_ERROR (0x102) for non-specific failures
   // initiated by the implementation; this is the right code to send
   // on RESET_STREAM when a stream is being aborted without an