From aefee4f0ea38d4b1b391a5469d10bc9a0dc5f259 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Fri, 8 May 2026 15:04:28 +0200 Subject: [PATCH] quic: add proper error codes & messages for QUIC failures Signed-off-by: Tim Perry --- lib/internal/errors.js | 4 +- lib/internal/quic/quic.js | 64 +++++++++++++---- src/quic/data.cc | 69 ++++++++++++++++++- src/quic/data.h | 3 + src/quic/session.cc | 11 +++ .../test-quic-session-close-error-code.mjs | 16 ++++- .../test-quic-stream-destroy-emits-reset.mjs | 10 ++- .../test-quic-stream-destroy-options-code.mjs | 5 +- ...est-quic-stream-writer-fail-error-code.mjs | 19 +---- 9 files changed, 157 insertions(+), 44 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 1d31e2b43dc2bd..aad4288562a40d 100644 --- a/lib/internal/errors.js +++ b/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 ' + diff --git a/lib/internal/quic/quic.js b/lib/internal/quic/quic.js index 6ca59469faf2de..415852567535eb 100644 --- a/lib/internal/quic/quic.js +++ b/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; } } diff --git a/src/quic/data.cc b/src/quic/data.cc index be2bf458d28352..a92bee96ca1829 100644 --- a/src/quic/data.cc +++ b/src/quic/data.cc @@ -1,13 +1,14 @@ #if HAVE_OPENSSL && HAVE_QUIC #include "guard.h" #ifndef OPENSSL_NO_QUIC -#include "data.h" #include #include #include #include +#include #include #include +#include "data.h" #include "defs.h" #include "util.h" @@ -346,6 +347,62 @@ std::optional 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 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 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 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(); } diff --git a/src/quic/data.h b/src/quic/data.h index 2b6d777caf7b81..5960783cea1cd1 100644 --- a/src/quic/data.h +++ b/src/quic/data.h @@ -231,6 +231,9 @@ class QuicError final : public MemoryRetainer { bool is_crypto_error() const; std::optional 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 diff --git a/src/quic/session.cc b/src/quic/session.cc index b53bc291c20163..8d4aba778247f4 100644 --- a/src/quic/session.cc +++ b/src/quic/session.cc @@ -3152,12 +3152,23 @@ void Session::EmitClose(const QuicError& error) { Integer::New(env()->isolate(), static_cast(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); diff --git a/test/parallel/test-quic-session-close-error-code.mjs b/test/parallel/test-quic-session-close-error-code.mjs index e907cd672c19d4..5d0392bf102585 100644 --- a/test/parallel/test-quic-session-close-error-code.mjs +++ b/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', diff --git a/test/parallel/test-quic-stream-destroy-emits-reset.mjs b/test/parallel/test-quic-stream-destroy-emits-reset.mjs index 280e25bc293a2d..38a3bbec59961c 100644 --- a/test/parallel/test-quic-stream-destroy-emits-reset.mjs +++ b/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(); }); }); diff --git a/test/parallel/test-quic-stream-destroy-options-code.mjs b/test/parallel/test-quic-stream-destroy-options-code.mjs index bdb64329b477bb..54f1b5e78c2b53 100644 --- a/test/parallel/test-quic-stream-destroy-options-code.mjs +++ b/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(); }); }); diff --git a/test/parallel/test-quic-stream-writer-fail-error-code.mjs b/test/parallel/test-quic-stream-writer-fail-error-code.mjs index 75a680ec4fa8bf..661e715fe63040 100644 --- a/test/parallel/test-quic-stream-writer-fail-error-code.mjs +++ b/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. n []" -// 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