quic: add proper error codes & messages for QUIC failures by pimterry · Pull Request #63198 · nodejs/node

pimterry · 2026-05-08T13:31:52Z

This wraps QUIC errors, providing useful error codes and messages for the defined error code cases from both OpenSSL and ngtcp2.

Before this, QUIC errors looked like:

// Application error with an explicit reason string:Error[ERR_QUIC_APPLICATION_ERROR]: AQUICapplicationerroroccurred.42n[clientshutdown]at[kFinishClose](node:internal/quic/quic:3505:22)atSession.onSessionClose(node:internal/quic/quic:679:31){code: 'ERR_QUIC_APPLICATION_ERROR'}// All our internal errors:Error[ERR_QUIC_TRANSPORT_ERROR]: AQUICtransporterroroccurred.296n[undefined]at[kFinishClose](node:internal/quic/quic:3502:22)atSession.onSessionClose(node:internal/quic/quic:679:31){code: 'ERR_QUIC_TRANSPORT_ERROR'}

Now they look like:

Error[ERR_QUIC_APPLICATION_ERROR]: QUICapplicationerror42: clientshutdownatmakeQuicError(node:internal/quic/quic:999:15)at[kFinishClose](node:internal/quic/quic:3552:22)atSession.onSessionClose(node:internal/quic/quic:681:31){code: 'ERR_QUIC_APPLICATION_ERROR',errorCode: 42n,type: 'application',reason: 'client shutdown'}Error[ERR_QUIC_TRANSPORT_ERROR]: QUICtransporterrorhandshakefailure(296)atmakeQuicError(node:internal/quic/quic:988:15)at[kFinishClose](node:internal/quic/quic:3534:22)atSession.onSessionClose(node:internal/quic/quic:681:31){code: 'ERR_QUIC_TRANSPORT_ERROR',errorCode: 296n,type: 'transport',errorName: 'handshake failure'}

nodejs-github-bot · 2026-05-08T13:31:57Z

Review requested:

@nodejs/quic

codecov · 2026-05-08T15:08:23Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.29%. Comparing base (8257091) to head (c5bd305).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #63198 +/- ##
==========================================
- Coverage 90.32% 90.29% -0.03% 
==========================================
Files 730 730 Lines 234671 234825 +154 Branches 43946 43956 +10 ==========================================
+ Hits 211965 212043 +78 - Misses 14423 14492 +69 - Partials 8283 8290 +7

Files with missing lines	Coverage Δ
lib/internal/errors.js	`97.65% <ø> (-0.01%)`	⬇️
lib/internal/quic/quic.js	`100.00% <100.00%> (ø)`

... and 30 files with indirect coverage changes

🚀 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.

Ethan-Arrowood

this is a great change.

Ethan-Arrowood

this is a great change.

pimterry · 2026-05-18T16:44:23Z

Updated to use bindingdata, rebased and force pushed because we need to update the test from #63193 (just merged) as well since that asserted on the error message.

metcoder95

lgtm

Qard

Is there some reason you're undid the use of the existing core errors system? Shouldn't we be keeping that?

Qard · 2026-05-21T05:43:39Z

+function makeQuicError(code, prefix, type, errorCode, reason, errorName) {
+ const err = new QuicError(
+ quicErrorMessage(prefix, errorCode, reason, errorName),
+ { errorCode, code, type });
+ if (reason) err.reason = reason;
+ if (errorName) err.errorName = errorName;
+ return err;
+}


Should we not keep the same error construction mechanism as the others? We lose a bunch of things by not using that. Not sure we should be taking these out of the global errors set.

This was a suggestion here, changed here.
We still use standard error codes, but we use a single QuicError class instead of the standard construction of per-code error classes: https://github.com/nodejs/node/blob/main/doc/api/quic.md#class-quicerror. This error class is used by users as well to control QUIC error behaviour, and this approach gives us consistent errors for every kind of failure at the protocol level. I don't feel strongly about it myself, but it seemed like a reasonable suggestion and having a single QUIC error class is tidy.
What do we lose by not using the standard error mechanism vs this setup?

We lose the hideStackFrames logic, which means we'll see a bunch more internals things in stack traces here which we might not want to surface, like every validator gains a stack line without this. I'm not against this change, but just want to be sure that special-casing this is acceptable.

In this case, I think the previous code didn't do this either - our validators do hide internal stack traces, but AFAICT the normal error code definitions don't do this unless you use HideStackFramesError explicitly.
It is easy to do though, and I agree it'd be a nice improvement - I've just pushed a commit that adds ErrorCaptureStackTrace(err, makeQuicError) here, which will omit these internal bits from the stack in the same way. This mirrors the equivalent setup for custom errors elsewhere like
node/lib/internal/fs/utils.js
Lines 350 to 363 in 92f48f4
functionhandleErrorFromBinding(ctx){
if(ctx.errno!==undefined){// libuv error numbers
consterr=newUVException(ctx);
ErrorCaptureStackTrace(err,handleErrorFromBinding);
throwerr;
}
if(ctx.error!==undefined){// Errors created in C++ land.
// TODO(joyeecheung): currently, ctx.error are encoding errors
// usually caused by memory problems. We need to figure out proper error
// code(s) for this.
ErrorCaptureStackTrace(ctx.error,handleErrorFromBinding);
throwctx.error;
}
}
for FS bindings.
Is that what you were looking for?

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

Qard

Linter seems to be complaining, but otherwise LGTM.

Move error paths added on main onto the new QuicError flow here, and fix the error code in CheckStreamIdleTimeout, which was incorrect and so failed validation.

pimterry · 2026-05-28T10:32:23Z

Linter seems to be complaining, but otherwise LGTM.

Yeah, linting passed in isolation but failed on the merge commit due to new errors added elsewhere.

I've now rebased, fixed the conflicting changes so everything'll hopefully pass, and force pushed. If you have a second to re-review @Qard I'd appreciate it! 🙏 Thanks

nodejs-github-bot · 2026-05-28T17:02:41Z

CI: https://ci.nodejs.org/job/node-test-pull-request/73815/

nodejs-github-bot · 2026-05-28T20:02:08Z

Landed in e1ae3a5

Signed-off-by: Tim Perry <pimterry@gmail.com> PR-URL: #63198 Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

Signed-off-by: Tim Perry <pimterry@gmail.com> PR-URL: nodejs#63198 Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

Signed-off-by: Tim Perry <pimterry@gmail.com> PR-URL: #63198 Backport-PR-URL: #64675 Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 8, 2026

pimterry force-pushed the fix-quic-error-codes branch from 31b4709 to aefee4fCompare May 8, 2026 13:41

jasnell reviewed May 8, 2026
View reviewed changes

Comment threadlib/internal/quic/quic.js Outdated

jasnell reviewed May 8, 2026
View reviewed changes

Comment threadsrc/quic/data.cc

trivikr added the quic Issues and PRs related to the QUIC implementation / HTTP/3. label May 11, 2026

pimterry added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label May 11, 2026

pimterry requested a review from jasnell May 11, 2026 13:45

bjohansebas reviewed May 11, 2026
View reviewed changes

Comment threadlib/internal/errors.js

jasnell reviewed May 14, 2026
View reviewed changes

Comment threadsrc/quic/data.cc Outdated

jasnell reviewed May 14, 2026
View reviewed changes

Comment threadsrc/quic/session.cc Outdated

Ethan-Arrowood reviewed May 15, 2026
View reviewed changes

Ethan-Arrowood approved these changes May 15, 2026
View reviewed changes

pimterry force-pushed the fix-quic-error-codes branch from 40eebfd to 60b6e3bCompare May 18, 2026 16:16

metcoder95 approved these changes May 19, 2026
View reviewed changes

Qard reviewed May 21, 2026
View reviewed changes

pimterry added 6 commits May 27, 2026 15:52

quic: add proper error codes & messages for QUIC failures
d641993
Signed-off-by: Tim Perry <pimterry@gmail.com>

From review: internalize QUIC error strings
5804046
Signed-off-by: Tim Perry <pimterry@gmail.com>

From review: use QuicError class for thrown QUIC transport/app errors
e72d6ce
Signed-off-by: Tim Perry <pimterry@gmail.com>

From review: move QUIC error names into bindingdata
4537f5c

Update just-merged ALPN test too
841af8d

From review: hide internal frames from QUIC errors
c7333ad

Qard approved these changes May 28, 2026
View reviewed changes

Fix issues after rebase
c5bd305
Move error paths added on main onto the new QuicError flow here, and fix the error code in CheckStreamIdleTimeout, which was incorrect and so failed validation.

pimterry force-pushed the fix-quic-error-codes branch from 6adb825 to c5bd305Compare May 28, 2026 09:57

Ethan-Arrowood approved these changes May 28, 2026
View reviewed changes

pimterry added the request-ci Add this label to start a Jenkins CI on a PR. label May 28, 2026

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 28, 2026

Qard approved these changes May 28, 2026
View reviewed changes

pimterry added the commit-queue Add this label to land a pull request using GitHub Actions. label May 28, 2026

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 28, 2026

nodejs-github-bot merged commit e1ae3a5 into nodejs:mainMay 28, 2026
73 checks passed

github-actionsBot mentioned this pull request May 30, 2026
2026-06-01, Version 26.3.0 (Current) #63664
Merged

aduh95 mentioned this pull request Jul 22, 2026
[v24.x backport] node:stream/iter and QUIC backports #64675
Closed

	functionhandleErrorFromBinding(ctx){
	if(ctx.errno!==undefined){// libuv error numbers
	consterr=newUVException(ctx);
	ErrorCaptureStackTrace(err,handleErrorFromBinding);
	throwerr;
	}
	if(ctx.error!==undefined){// Errors created in C++ land.
	// TODO(joyeecheung): currently, ctx.error are encoding errors
	// usually caused by memory problems. We need to figure out proper error
	// code(s) for this.
	ErrorCaptureStackTrace(ctx.error,handleErrorFromBinding);
	throwctx.error;
	}
	}

Uh oh!

Uh oh!

Conversation

pimterry commented May 8, 2026

Uh oh!

nodejs-github-bot commented May 8, 2026

Uh oh!

Uh oh!

Uh oh!

codecovBot commented May 8, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ethan-Arrowood left a comment

Choose a reason for hiding this comment

Uh oh!

Ethan-Arrowood left a comment

Choose a reason for hiding this comment

Uh oh!

pimterry commented May 18, 2026

Uh oh!

metcoder95 left a comment

Choose a reason for hiding this comment

Uh oh!

Qard left a comment

Choose a reason for hiding this comment

Uh oh!

QardMay 21, 2026

Choose a reason for hiding this comment

Uh oh!

pimterryMay 21, 2026

Choose a reason for hiding this comment

Uh oh!

QardMay 22, 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!

pimterryMay 26, 2026

Choose a reason for hiding this comment

Uh oh!

Qard left a comment

Choose a reason for hiding this comment

Uh oh!

pimterry commented May 28, 2026

Uh oh!

nodejs-github-bot commented May 28, 2026

Uh oh!

Uh oh!

nodejs-github-bot commented May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

codecovBot commented May 8, 2026•
edited
Loading

QardMay 22, 2026•
edited
Loading