quic: add proper error codes & messages for QUIC failures

Signed-off-by: Tim Perry <pimterry@gmail.com>

Author: pimterry

lib/internal/errors.js

@@ -1687,14 +1687,14 @@ E('ERR_PERFORMANCE_INVALID_TIMESTAMP',
 E('ERR_PERFORMANCE_MEASURE_INVALID_OPTIONS', '%s', TypeError);
 E('ERR_PROXY_INVALID_CONFIG', '%s', Error);
 E('ERR_PROXY_TUNNEL', '%s', Error);
-E('ERR_QUIC_APPLICATION_ERROR', 'A QUIC application error occurred. %d [%s]', Error);
+E('ERR_QUIC_APPLICATION_ERROR', '%s', Error);
 E('ERR_QUIC_CONNECTION_FAILED', 'QUIC connection failed', Error);
 E('ERR_QUIC_ENDPOINT_CLOSED', 'QUIC endpoint closed: %s (%d)', Error);
 E('ERR_QUIC_OPEN_STREAM_FAILED', 'Failed to open QUIC stream', Error);
 E('ERR_QUIC_STREAM_ABORTED', '%s', Error);
 E('ERR_QUIC_STREAM_RESET',
   'The QUIC stream was reset by the peer with error code %d', Error);
-E('ERR_QUIC_TRANSPORT_ERROR', 'A QUIC transport error occurred. %d [%s]', Error);
+E('ERR_QUIC_TRANSPORT_ERROR', '%s', Error);
 E('ERR_QUIC_VERSION_NEGOTIATION_ERROR', 'The QUIC session requires version negotiation', Error);
 E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parentFilename) {
   let message = 'require() cannot be used on an ESM ' +

lib/internal/quic/quic.js

@@ -673,10 +673,12 @@ setCallbacks({
    * @param {number} errorType
    * @param {number} code
    * @param {string} [reason]
+   * @param {string} [errorName] Decoded TLS alert name when `code` is a
+   *   CRYPTO_ERROR; otherwise undefined.
    */
-  onSessionClose(errorType, code, reason) {
-    debug('session close callback', errorType, code, reason);
-    this[kOwner][kFinishClose](errorType, code, reason);
+  onSessionClose(errorType, code, reason, errorName) {
+    debug('session close callback', errorType, code, reason, errorName);
+    this[kOwner][kFinishClose](errorType, code, reason, errorName);
   },
 
   /**
@@ -968,21 +970,50 @@ class QuicError extends Error {
   }
 }
 
-// Converts a raw QuicError array [type, code, reason] from C++ into a
-// proper Node.js Error object.
+// Build the human-readable message for an ERR_QUIC_TRANSPORT_ERROR or
+// ERR_QUIC_APPLICATION_ERROR. `errorName` is the symbolic name for
+// the wire code when known: either the OpenSSL-decoded TLS alert
+// (CRYPTO_ERROR; 0x100..0x1ff) or one of the named transport codes
+// from RFC 9000 (e.g. PROTOCOL_VIOLATION). Otherwise undefined.
+// `reason` is the peer-supplied UTF-8 reason string from the
+// CONNECTION_CLOSE / RESET_STREAM frame, often empty.
+function quicErrorMessage(prefix, errorCode, reason, errorName) {
+  let msg = `${prefix} `;
+  msg += errorName ? `${errorName} (${errorCode})` : `${errorCode}`;
+  if (reason) msg += `: ${reason}`;
+  return msg;
+}
+
+function makeQuicError(ErrorClass, prefix, type, errorCode, reason, errorName) {
+  const err = new ErrorClass(
+    quicErrorMessage(prefix, errorCode, reason, errorName));
+  err.errorCode = errorCode;
+  err.type = type;
+  if (reason) err.reason = reason;
+  if (errorName) err.errorName = errorName;
+  return err;
+}
+
 function convertQuicError(error) {
   const type = error[0];
   const code = error[1];
   const reason = error[2];
+  const errorName = error[3];
   switch (type) {
     case 'transport':
-      return new ERR_QUIC_TRANSPORT_ERROR(code, reason);
+      return makeQuicError(ERR_QUIC_TRANSPORT_ERROR,
+                           'QUIC transport error',
+                           'transport', code, reason, errorName);
     case 'application':
-      return new ERR_QUIC_APPLICATION_ERROR(code, reason);
+      return makeQuicError(ERR_QUIC_APPLICATION_ERROR,
+                           'QUIC application error',
+                           'application', code, reason, errorName);
     case 'version_negotiation':
       return new ERR_QUIC_VERSION_NEGOTIATION_ERROR();
     default:
-      return new ERR_QUIC_TRANSPORT_ERROR(code, reason);
+      return makeQuicError(ERR_QUIC_TRANSPORT_ERROR,
+                           'QUIC transport error',
+                           'transport', code, reason, errorName);
   }
 }
 
@@ -3463,7 +3494,7 @@ class QuicSession {
    * @param {number} code
    * @param {string} [reason]
    */
-  [kFinishClose](errorType, code, reason) {
+  [kFinishClose](errorType, code, reason, errorName) {
     // If code is zero, then we closed without an error. Yay! We can destroy
     // safely without specifying an error.
     if (code === 0n) {
@@ -3472,7 +3503,8 @@ class QuicSession {
       return;
     }
 
-    debug('finishing closing the session with an error', errorType, code, reason);
+    debug('finishing closing the session with an error',
+          errorType, code, reason, errorName);
 
     // If the local side initiated this close with an error code (via
     // close({ code })), this is an intentional shutdown; not an error.
@@ -3499,10 +3531,14 @@ class QuicSession {
     // session would leak with `closed` hanging forever.
     switch (errorType) {
       case 0: /* Transport Error */
-        this.destroy(new ERR_QUIC_TRANSPORT_ERROR(code, reason));
+        this.destroy(makeQuicError(ERR_QUIC_TRANSPORT_ERROR,
+                                   'QUIC transport error',
+                                   'transport', code, reason, errorName));
         break;
       case 1: /* Application Error */
-        this.destroy(new ERR_QUIC_APPLICATION_ERROR(code, reason));
+        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());
@@ -3511,7 +3547,9 @@ class QuicSession {
         this.destroy();
         break;
       default:
-        this.destroy(new ERR_QUIC_TRANSPORT_ERROR(code, reason));
+        this.destroy(makeQuicError(ERR_QUIC_TRANSPORT_ERROR,
+                                   'QUIC transport error',
+                                   'transport', code, reason, errorName));
         break;
     }
   }

src/quic/data.cc

@@ -1,13 +1,14 @@
 #if HAVE_OPENSSL && HAVE_QUIC
 #include "guard.h"
 #ifndef OPENSSL_NO_QUIC
-#include "data.h"
 #include <env-inl.h>
 #include <memory_tracker-inl.h>
 #include <ngtcp2/ngtcp2.h>
 #include <node_sockaddr-inl.h>
+#include <openssl/ssl.h>
 #include <string_bytes.h>
 #include <v8.h>
+#include "data.h"
 #include "defs.h"
 #include "util.h"
 
@@ -346,6 +347,62 @@ std::optional<int> QuicError::get_crypto_error() const {
   return code() & ~NGTCP2_CRYPTO_ERROR;
 }
 
+const char* QuicError::name() const {
+  // CRYPTO_ERROR carries a TLS alert in its low byte (RFC 9001 sec. 4.8).
+  // OpenSSL's SSL_alert_desc_string_long owns a stable string for every
+  // alert it knows about; we filter out the "unknown" placeholder so the
+  // JS side can present `errorName` as undefined for unrecognised alerts.
+  if (auto alert = get_crypto_error()) {
+    const char* n = SSL_alert_desc_string_long(*alert);
+    if (n != nullptr && std::string_view(n) != "unknown") return n;
+    return nullptr;
+  }
+  // Named transport-layer error codes from RFC 9000 sec. 20.1 (and the
+  // RFC 9368 version-negotiation extension). Application error codes are
+  // opaque to QUIC, so we only decode for transport.
+  if (type() != Type::TRANSPORT) return nullptr;
+  switch (code()) {
+    case NGTCP2_NO_ERROR:
+      return "NO_ERROR";
+    case NGTCP2_INTERNAL_ERROR:
+      return "INTERNAL_ERROR";
+    case NGTCP2_CONNECTION_REFUSED:
+      return "CONNECTION_REFUSED";
+    case NGTCP2_FLOW_CONTROL_ERROR:
+      return "FLOW_CONTROL_ERROR";
+    case NGTCP2_STREAM_LIMIT_ERROR:
+      return "STREAM_LIMIT_ERROR";
+    case NGTCP2_STREAM_STATE_ERROR:
+      return "STREAM_STATE_ERROR";
+    case NGTCP2_FINAL_SIZE_ERROR:
+      return "FINAL_SIZE_ERROR";
+    case NGTCP2_FRAME_ENCODING_ERROR:
+      return "FRAME_ENCODING_ERROR";
+    case NGTCP2_TRANSPORT_PARAMETER_ERROR:
+      return "TRANSPORT_PARAMETER_ERROR";
+    case NGTCP2_CONNECTION_ID_LIMIT_ERROR:
+      return "CONNECTION_ID_LIMIT_ERROR";
+    case NGTCP2_PROTOCOL_VIOLATION:
+      return "PROTOCOL_VIOLATION";
+    case NGTCP2_INVALID_TOKEN:
+      return "INVALID_TOKEN";
+    case NGTCP2_APPLICATION_ERROR:
+      return "APPLICATION_ERROR";
+    case NGTCP2_CRYPTO_BUFFER_EXCEEDED:
+      return "CRYPTO_BUFFER_EXCEEDED";
+    case NGTCP2_KEY_UPDATE_ERROR:
+      return "KEY_UPDATE_ERROR";
+    case NGTCP2_AEAD_LIMIT_REACHED:
+      return "AEAD_LIMIT_REACHED";
+    case NGTCP2_NO_VIABLE_PATH:
+      return "NO_VIABLE_PATH";
+    case NGTCP2_VERSION_NEGOTIATION_ERROR:
+      return "VERSION_NEGOTIATION_ERROR";
+    default:
+      return nullptr;
+  }
+}
+
 MaybeLocal<Value> QuicError::ToV8Value(Environment* env) const {
   if ((type() == Type::TRANSPORT && code() == NGTCP2_NO_ERROR) ||
       (type() == Type::APPLICATION && code() == NGHTTP3_H3_NO_ERROR) ||
@@ -367,6 +424,7 @@ MaybeLocal<Value> QuicError::ToV8Value(Environment* env) const {
       type_str,
       BigInt::NewFromUnsigned(env->isolate(), code()),
       Undefined(env->isolate()),
+      Undefined(env->isolate()),
   };
 
   // Note that per the QUIC specification, the reason, if present, is
@@ -380,6 +438,15 @@ MaybeLocal<Value> QuicError::ToV8Value(Environment* env) const {
     return {};
   }
 
+  // Attach a human-readable name for known wire codes (RFC 9000 sec. 20.1
+  // names and OpenSSL TLS alert descriptions for CRYPTO_ERROR). Unknown
+  // codes leave the slot as undefined.
+  if (const char* n = name()) {
+    if (!node::ToV8Value(env->context(), n).ToLocal(&argv[3])) {
+      return {};
+    }
+  }
+
   return Array::New(env->isolate(), argv, arraysize(argv)).As<Value>();
 }
 

src/quic/data.h

@@ -231,6 +231,9 @@ class QuicError final : public MemoryRetainer {
   bool is_crypto_error() const;
   std::optional<int> get_crypto_error() const;
 
+  // Returns a human-readable name for this error if known, or nullptr
+  const char* name() const;
+
   // Note that since application errors are application-specific and we
   // don't know which application is being used here, it is possible that
   // the comparing two different QuicError instances from different applications

src/quic/session.cc

@@ -3152,12 +3152,23 @@ void Session::EmitClose(const QuicError& error) {
       Integer::New(env()->isolate(), static_cast<int>(error.type())),
       BigInt::NewFromUnsigned(env()->isolate(), error.code()),
       Undefined(env()->isolate()),
+      Undefined(env()->isolate()),
   };
   if (error.reason().length() > 0 &&
       !ToV8Value(env()->context(), error.reason()).ToLocal(&argv[2])) {
     return;
   }
 
+  // Attach a human-readable name for known wire codes (RFC 9000 sec. 20.1
+  // names and OpenSSL TLS alert descriptions for CRYPTO_ERROR). Unknown
+  // codes leave the slot as undefined. See QuicError::name() for the
+  // matching path on stream-level errors.
+  if (const char* n = error.name()) {
+    if (!ToV8Value(env()->context(), n).ToLocal(&argv[3])) {
+      return;
+    }
+  }
+
   MakeCallback(
       BindingData::Get(env()).session_close_callback(), arraysize(argv), argv);
 

test/parallel/test-quic-session-close-error-code.mjs

@@ -29,13 +29,18 @@ const { listen, connect } = await import('../common/quic.mjs');
   const serverEndpoint = await listen(mustCall(async (serverSession) => {
     serverSession.onerror = mustCall((err) => {
       strictEqual(err.code, 'ERR_QUIC_APPLICATION_ERROR');
-      strictEqual(err.message.includes('42n'), true,
+      strictEqual(err.message.includes('42'), true,
                   'error message should contain the code');
       strictEqual(err.message.includes('client shutdown'), true,
                   'error message should contain the reason');
+      strictEqual(err.errorCode, 42n);
+      strictEqual(err.type, 'application');
+      strictEqual(err.reason, 'client shutdown');
     });
     await rejects(serverSession.closed, {
       code: 'ERR_QUIC_APPLICATION_ERROR',
+      errorCode: 42n,
+      reason: 'client shutdown',
     });
     serverGot.resolve();
   }));
@@ -71,8 +76,10 @@ const { listen, connect } = await import('../common/quic.mjs');
   const serverEndpoint = await listen(mustCall(async (serverSession) => {
     serverSession.onerror = mustCall((err) => {
       strictEqual(err.code, 'ERR_QUIC_TRANSPORT_ERROR');
-      strictEqual(err.message.includes('1n'), true,
+      strictEqual(err.message.includes('1'), true,
                   'error message should contain the code');
+      strictEqual(err.errorCode, 1n);
+      strictEqual(err.type, 'transport');
     });
     await rejects(serverSession.closed, {
       code: 'ERR_QUIC_TRANSPORT_ERROR',
@@ -102,7 +109,10 @@ const { listen, connect } = await import('../common/quic.mjs');
   const serverEndpoint = await listen(mustCall(async (serverSession) => {
     serverSession.onerror = mustCall((err) => {
       strictEqual(err.code, 'ERR_QUIC_APPLICATION_ERROR');
-      strictEqual(err.message.includes('99n'), true);
+      strictEqual(err.message.includes('99'), true);
+      strictEqual(err.errorCode, 99n);
+      strictEqual(err.type, 'application');
+      strictEqual(err.reason, 'destroy with code');
     });
     await rejects(serverSession.closed, {
       code: 'ERR_QUIC_APPLICATION_ERROR',

test/parallel/test-quic-stream-destroy-emits-reset.mjs

@@ -12,12 +12,12 @@
 // code is the negotiated application's "internal error" code: for
 // the test fixture's non-h3 ALPN (`quic-test`) the C++
 // DefaultApplication reports `1n`, which propagates to the server
-// as `ERR_QUIC_APPLICATION_ERROR` carrying `1n` in its message.
+// as `ERR_QUIC_APPLICATION_ERROR` exposing `errorCode === 1n`.
 
 import { hasQuic, skip, mustCall } from '../common/index.mjs';
 import assert from 'node:assert';
 
-const { strictEqual, ok, rejects } = assert;
+const { strictEqual, rejects } = assert;
 
 if (!hasQuic) {
   skip('QUIC is not enabled');
@@ -35,10 +35,8 @@ const serverEndpoint = await listen(mustCall((serverSession) => {
     // fired with the expected code.
     stream.onreset = mustCall((err) => {
       strictEqual(err.code, 'ERR_QUIC_APPLICATION_ERROR');
-      // The DefaultApplication's internal error code is 0x1n, which
-      // util.format renders as `1n` (BigInt) in the message text.
-      ok(err.message.includes('1n'),
-         `expected '1n' in message, got: ${err.message}`);
+      // The DefaultApplication's internal error code is 0x1n.
+      strictEqual(err.errorCode, 1n);
       serverResetSeen.resolve();
     });
   });

test/parallel/test-quic-stream-destroy-options-code.mjs

@@ -13,7 +13,7 @@
 import { hasQuic, skip, mustCall } from '../common/index.mjs';
 import assert from 'node:assert';
 
-const { strictEqual, ok, rejects } = assert;
+const { strictEqual, rejects } = assert;
 
 if (!hasQuic) {
   skip('QUIC is not enabled');
@@ -27,8 +27,7 @@ const serverEndpoint = await listen(mustCall((serverSession) => {
   serverSession.onstream = mustCall((stream) => {
     stream.onreset = mustCall((err) => {
       strictEqual(err.code, 'ERR_QUIC_APPLICATION_ERROR');
-      ok(err.message.includes('66n'),
-         `expected '66n' in message, got: ${err.message}`);
+      strictEqual(err.errorCode, 66n);
       serverResetSeen.resolve();
     });
   });

test/parallel/test-quic-stream-writer-fail-error-code.mjs

@@ -17,11 +17,8 @@
 //      the QuicError fast path.
 //
 // The peer-side observation goes through `stream.onreset(err)` where
-// `err` is `ERR_QUIC_APPLICATION_ERROR` carrying the wire code in its
-// message string. We extract the code via regex; once
-// `ERR_QUIC_APPLICATION_ERROR` exposes the numeric code as a property
-// (a planned follow-up), this test can switch to direct property
-// access.
+// `err` is `ERR_QUIC_APPLICATION_ERROR` exposing the wire code on
+// `err.errorCode` (a BigInt).
 
 import { hasQuic, skip, mustCall } from '../common/index.mjs';
 import assert from 'node:assert';
@@ -35,19 +32,9 @@ if (!hasQuic) {
 const { listen, connect } = await import('../common/quic.mjs');
 const { QuicError } = await import('node:quic');
 
-// Extract the numeric wire code from an ERR_QUIC_APPLICATION_ERROR
-// message of the form
-//   "A QUIC application error occurred. <code>n [<reason>]"
-// where the trailing `n` on the code is the BigInt formatting from
-// `util.format('%d', bigint)`. RESET_STREAM frames do not carry a
-// reason string, so the bracketed value is typically `undefined`.
 function wireCodeOf(err) {
   strictEqual(err.code, 'ERR_QUIC_APPLICATION_ERROR');
-  const match = err.message.match(/A QUIC application error occurred\. (\d+)n /);
-  if (!match) {
-    throw new Error(`Could not extract code from message: ${err.message}`);
-  }
-  return BigInt(match[1]);
+  return err.errorCode;
 }
 
 // Server: capture the next two streams. Each stream receives an