quic: start adding in the internal quic js api by jasnell · Pull Request #53256 · nodejs/node · GitHub

While the external API for QUIC is expected to be the WebTransport API primarily, this provides the
internal API for QUIC that aligns with the native C++ QUIC components. This is the first of several PRs that will fill out this part of the implementation. The goal of doing this incrementally is to make things easier to review by doing it in smaller chunks

Unless a massive improvement is planned on WebStream, I would recommend not using them in this lower level. They are 1 order of magnitude slower than Node streams. Using them here would limit the applicability of our Quic implementation.

Just using functions and callbacks might even be faster.

Really interesting @jasnell .

Just a reminder we have continued to apply fixes to your original attempt many of which probably still apply today (latest branch: https://github.com/HalleyAssist/node/commits/test_quic16/ - not everything in this branch is relevant). Some fixes could probably done better with more drastic changes, however they all represent observed leaks, infinite loops or crashes observed in our staging and pilot environments.

Including yesterday a fix for blocking streams creating an infinite loop in SendPendingData (which can be tested for with rx packet loss simulation). When you do get a JS api I'll try and contribute some of these directly.

Makes me a bit nervous that these names are a single character apart. High chance of typos in future maintenance.

Not ignoring this comment, just plan to address it in a subsequent iteration

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

While the external API for QUIC is expected to be the WebTransport API primarily, this provides the internal API for QUIC that aligns with the native C++ QUIC components.

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

While the external API for QUIC is expected to be the WebTransport API primarily, this provides the internal API for QUIC that aligns with the native C++ QUIC components. PR-URL: #53256 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

PR-URL: #53256 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Landed in 103b843...858bce5

This landed with failing Coverage CI, and now it fails for all PRs against main

This change is why coverage jobs error

While the external API for QUIC is expected to be the WebTransport API primarily, this provides the internal API for QUIC that aligns with the native C++ QUIC components. PR-URL: #53256 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

PR-URL: #53256 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.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 Jun 2, 2024

jasnell added the quic Issues and PRs related to the QUIC implementation / HTTP/3. label Jun 2, 2024

jasnell requested review from Qard and anonrig June 2, 2024 03:35

jasnell force-pushed the quic-part-13 branch 2 times, most recently from 1babee6 to 9f2f600 Compare June 2, 2024 04:03

jasnell requested a review from mcollina June 2, 2024 04:04

jasnell force-pushed the quic-part-13 branch from 9f2f600 to f716980 Compare June 2, 2024 04:06

jasnell requested a review from tniessen June 2, 2024 14:30

jasnell force-pushed the quic-part-13 branch from f716980 to 5342557 Compare June 2, 2024 14:53

jasnell commented Jun 2, 2024

View reviewed changes

Comment thread lib/internal/quic/quic.js Outdated

targos added dont-land-on-v18.x labels Jun 2, 2024

jasnell commented Jun 2, 2024

View reviewed changes

Comment thread lib/internal/quic/datagrams.js Outdated

jasnell force-pushed the quic-part-13 branch 8 times, most recently from d4ea9c3 to f245d2c Compare June 2, 2024 22:14

VoltrexKeyva reviewed Jun 3, 2024

View reviewed changes

mcollina reviewed Jun 4, 2024

View reviewed changes

Qard approved these changes Jun 14, 2024

View reviewed changes

anonrig approved these changes Jun 14, 2024

View reviewed changes

jasnell force-pushed the quic-part-13 branch from f245d2c to 8f406a1 Compare July 6, 2024 16:31

jasnell force-pushed the quic-part-13 branch from 9dc124d to 952b293 Compare July 8, 2024 03:41

mcollina reviewed Jul 8, 2024

View reviewed changes

Comment thread lib/internal/quic/quic.js Outdated

Comment thread lib/internal/quic/quic.js Outdated

achingbrain mentioned this pull request Jul 18, 2024

Add support for QUIC libp2p/js-libp2p#1459

Open

robertsLando mentioned this pull request Aug 8, 2024

feat: add quic support mqttjs/MQTT.js#1925

Draft

jasnell force-pushed the quic-part-13 branch from 952b293 to 37deb95 Compare September 28, 2024 16:38

anonrig approved these changes Sep 28, 2024

View reviewed changes

jasnell force-pushed the quic-part-13 branch from 37deb95 to 61ffef3 Compare September 28, 2024 16:56

This comment was marked as outdated.

Sign in to view

jasnell force-pushed the quic-part-13 branch from 61ffef3 to f883756 Compare September 28, 2024 17:11

jasnell added 2 commits September 28, 2024 10:38

quic: start adding in the internal quic js api

fd1f2d0

While the external API for QUIC is expected to be the WebTransport API primarily, this provides the internal API for QUIC that aligns with the native C++ QUIC components.

src: make minor tweaks to quic c++ for c++20

82566f8

jasnell force-pushed the quic-part-13 branch from f883756 to 82566f8 Compare September 28, 2024 17:51

jasnell added a commit that referenced this pull request Sep 29, 2024

src: make minor tweaks to quic c++ for c++20

858bce5

PR-URL: #53256 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

jasnell closed this Sep 29, 2024

targos mentioned this pull request Sep 30, 2024

src: apply getCallSite optimization #55174

Closed

targos reviewed Sep 30, 2024

View reviewed changes

This was referenced Sep 30, 2024

build: fix nycrc format #55188

Closed

build: fix not valid json in coverage #55179

Merged

targos pushed a commit that referenced this pull request Oct 5, 2024

src: make minor tweaks to quic c++ for c++20

295c17c

PR-URL: #53256 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

aduh95 mentioned this pull request Oct 9, 2024

V22.10.0 proposal #55343

Merged

                   "tools/**",
                   "benchmark/**",
-                  "deps/**"
+                  "deps/**",

		instance[kStats] = createStreamStats(handle.stats);
		instance[kState] = createStreamState(handle.state);

quic: start adding in the internal quic js api by jasnell · Pull Request #53256 · nodejs/node · GitHub

Uh oh!

Uh oh!

quic: start adding in the internal quic js api - #53256

quic: start adding in the internal quic js api#53256jasnell wants to merge 2 commits intonodejs:mainnodejs/node:mainfrom jasnell:quic-part-13jasnell/node:quic-part-13Copy head branch name to clipboard

Conversation

jasnell commented Jun 2, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

splitice commented Jun 7, 2024

Uh oh!

Qard Jun 14, 2024

Choose a reason for hiding this comment

Uh oh!

jasnell Jul 7, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

nodejs-github-bot commented Sep 28, 2024

Uh oh!

nodejs-github-bot commented Sep 28, 2024 • edited by jasnell Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasnell commented Sep 29, 2024

Uh oh!

targos commented Sep 30, 2024

Uh oh!

targos Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

quic: start adding in the internal quic js api#53256
jasnell wants to merge 2 commits into
nodejs:mainfrom
jasnell:quic-part-13

nodejs-github-bot commented Sep 28, 2024 •
edited by jasnell
Loading