| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Review requested:
|
Sorry, something went wrong.
|
This is very exciting! Nice work @jasnell, it's great to see the progress here. Some thoughts: It feels like this API is sort-of upside-down, compared to others we have. Elsewhere (dgram and net + derivatives) you create a client/session, and you then specify the local binding (e,g. socket.connect({ ... localPort ... }) or dgramSocket.bind(...)) via various parameters in setup. Here you create a representation of the local endpoint first, and then you build the client/server from that. I understand it makes sense from an internal implementation POV and for advanced cases, but it doesn't feel consistent with other APIs or the mental model that (imo) most users will have. There's also no representation of a 'server' in this API. There's only the endpoint itself (not quite the same). This means there's no good place to hang APIs to manage the state tied to the listening process - e.g. there seems to be no 'stop listening' API right now other than closing the endpoint completely (which also kills client connections), and there's no way to independently listen to any server events other than the initially registered callback. I suspect as this API develops, not having a server representation in the model will become inconvenient. I think initially users will reasonably expect something with roughly the shape & semantics of the simple example above: import { connect } from 'node:quic';
const client = connect('123.123.123.123:8888'); // <-- Implicit create an endpoint
client.openUnidirectionalStream({ body: Buffer.from('hello there'); });
await client.close();And I also think something roughly equivalent applies on the server-side too: import { createServer } from 'node:quic';
const keys = getKeysSomehow();
const certs = getCertsSomehow();
const server = createServer({ keys, certs }, (session) => {
session.onstream = (stream) => {
stream.setOutbound(Buffer.from('hello world'));
stream.closed.then(() => {
session.close();
});
readStreamSomehow(stream);
};
});
await server.listen('123.123.123.123:8888'); // <-- Implicit create an endpoint
// ...
await server.close();Supporting advanced use cases too is good but I really think a lot of people are going to come to this expecting that kind of API to be available, and making that possible will be hugely helpful. That's not to say that the endpoint model doesn't add something, it's just that AFAICT for the most common use cases it'll be slightly confusing extra complexity initially. Do we have any good examples of when & how direct endpoint control will be useful for users? How common is the use case where sharing one endpoint between multiple client connections and/or a server is important? For cases that do need endpoint control, I'd suggest exposing that optionally but flipping the API, so that an endpoint becomes a parameter of clients/servers rather than being the source of them (similar to the last example in the description). That means for advanced use cases, this would look (very roughly) like: import { QuicEndpoint, connect, createServer } from 'node:quic';
const endpoint = new QuicEndpoint();
const client = connect('123.123.123.123:8888', { endpoint });
client.openUnidirectionalStream({ body: Buffer.from('hello there'); });
await client.close();
const server = createServer();
server.listen('123.123.123.123:8888', { endpoint });The internal endpoint model remains the same I think, this is intended mostly as a shift in how that's exposed and where the methods go, with implicit endpoint creation whenever it's not specified. Regarding closing endpoints, I think in the simple case users definitely won't expect to close this themselves (because the endpoint is invisible) so tidy auto-closing is unavoidable. Even in the advanced case, I can see reasons to want both auto or manual endpoint closing. How about:
|
Sorry, something went wrong.
|
The API has been updated based on discussions with multiple folks and a review of what deno is doing with their quic api... For a server it is now.... import { listen } from 'node:quic';
async function consume(stream) {
for await (const chunk of stream.readable) {
///...
}
await stream.session.close();
}
listen((server) => {
server.onstream = async (stream) => {
consume(stream);
};
});For a client it is essentially... import { connect } from 'node:quic';
const client = await connect('123.123.123.123:8888');
client.createUnidirectionalStream({ body: Buffer.from('hello world') });
await client.close();There's a lot more to it, of course, but this is the start. |
Sorry, something went wrong.
Sorry, something went wrong.
Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #56328 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
|
Sorry this comment is a little late (please let me know if I should open a new issue), but can the QuicStream class please have a .writable property like the streams from the WebTransport API? That is, at the moment I have to pass a .body option of ArrayBuffer/ArrayBufferView/Blob to createBidirectionalStream/createUnidirectionalStream which means I have to assemble all data in-memory ahead of time instead of streaming it as it arrives. My (hopefully uncontentious) proposal would be for bidi streams to have a writable property: import { connect } from 'node:quic';
const client = await connect('123.123.123.123:8888');
const stream = await client.createBidirectionalStream();
const writer = stream.writable.getWriter();
writer.write(Buffer.from('hello world'));
await writer.close();and for uni streams to be a writable (again, lifted straight from the WebTransport API): import { connect } from 'node:quic';
const client = await connect('123.123.123.123:8888');
const stream = await client.createUnidirectionalStream();
const writer = stream.getWriter();
writer.write(Buffer.from('hello world'));
await writer.close();Thanks so much for landing this, it's really exciting to see progress on QUIC! (I'm happy to have a go at a PR implementing the above, if it's helpful) |
Sorry, something went wrong.
Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #56328 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
| "internal/quic/quic", "internal/quic/symbols", "internal/quic/stats", | ||
| "internal/quic/state", | ||
| #endif // !NODE_OPENSSL_HAS_QUIC | ||
| "quic", // Experimental. |
There was a problem hiding this comment.
Shouldn't we expose it only when Node.js is compiled with QUIC support, and the runtime flag --experimental-quic is passed? It looks like it's being listed in node -p 'require("node:module").builtinModules', which is confusing
Sorry, something went wrong.
Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #56328 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Notable changes: crypto: * (SEMVER-MINOR) support --use-system-ca on Windows (Joyee Cheung) #56833 * (SEMVER-MINOR) added support for reading certificates from macOS system store (Tim Jacomb) #56599 deps: * update timezone to 2025a (Node.js GitHub Bot) #56876 * (SEMVER-MINOR) update ada to v3.0.1 (Yagiz Nizipli) #56452 deps,tools: * (SEMVER-MINOR) add zstd 1.5.6 (Jan Krems) #52100 sqlite: * (SEMVER-MINOR) allow returning `ArrayBufferView`s from user-defined functions (René) #56790 src: * set signal inspector io thread name (RafaelGSS) #56416 * set thread name for main thread and v8 worker (RafaelGSS) #56416 * set worker thread name using worker.name (RafaelGSS) #56416 * use a default thread name for inspector (RafaelGSS) #56416 src, quic: * (SEMVER-MINOR) refine more of the quic implementation (James M Snell) #56328 test: * (SEMVER-MINOR) add WPT for URLPattern (Yagiz Nizipli) #56452 url: * (SEMVER-MINOR) add URLPattern implementation (Yagiz Nizipli) #56452 zlib: * (SEMVER-MINOR) add zstd support (Jan Krems) #52100 PR-URL: #57005
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [node](https://nodejs.org) ([source](https://github.com/nodejs/node)) | minor | `23.7.0` -> `23.8.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>nodejs/node (node)</summary> ### [`v23.8.0`](https://github.com/nodejs/node/releases/tag/v23.8.0): 2025-02-13, Version 23.8.0 (Current), @​targos [Compare Source](nodejs/node@v23.7.0...v23.8.0) ##### Notable Changes ##### Support for using system CA certificates store on macOS and Windows This version adds the `--use-system-ca` command-line flag, which instructs Node.js to use the trusted CA certificates present in the system store along with the `--use-bundled-ca`, `--use-openssl-ca` options. This option is available on macOS and Windows for now. Contributed by Tim Jacomb in [#​56599](nodejs/node#56599) and Joyee Cheung in [#​56833](nodejs/node#56833). ##### Introduction of the URL Pattern API An implementation of the [URL Pattern API](https://developer.mozilla.org/en-US/docs/Web/API/URL_Pattern_API) is now available. The `URLPattern` constructor is exported from the `node:url` module and will be available as a global in Node.js 24. Contributed by Yagiz Nizipli and Daniel Lemire in [#​56452](nodejs/node#56452). ##### Support for the zstd compression algorithm Node.js now includes support for the Zstandard (zstd) compression algorithm. Various APIs have been added to the `node:zlib` module for both compression and decompression of zstd streams. Contributed by Jan Krems in [#​52100](nodejs/node#52100). ##### Node.js thread names Threads created by the Node.js process are now named to improve the debugging experience. Worker threads will use the `name` option that can be passed to the `Worker` constructor. Contributed by Rafael Gonzaga in [#​56416](nodejs/node#56416). ##### Timezone data has been updated to 2025a Included changes: - Paraguay adopts permanent -03 starting spring 2024. - Improve pre-1991 data for the Philippines. ##### Other Notable Changes - \[[`39997867cf`](nodejs/node@39997867cf)] - **(SEMVER-MINOR)** **sqlite**: allow returning `ArrayBufferView`s from user-defined functions (René) [#​56790](nodejs/node#56790) ##### Commits - \[[`0ee9c34d63`](nodejs/node@0ee9c34d63)] - **benchmark**: add simple parse and test benchmarks for URLPattern (James M Snell) [#​56882](nodejs/node#56882) - \[[`b3f2045d14`](nodejs/node@b3f2045d14)] - **build**: gyp exclude libm linking on macOS (deepak1556) [#​56901](nodejs/node#56901) - \[[`e0dd9aefd6`](nodejs/node@e0dd9aefd6)] - **build**: remove explicit linker call to libm on macOS (deepak1556) [#​56901](nodejs/node#56901) - \[[`52399da780`](nodejs/node@52399da780)] - **build**: link with Security.framework in GN build (Cheng) [#​56895](nodejs/node#56895) - \[[`582b9221c9`](nodejs/node@582b9221c9)] - **build**: do not put commands in sources variables (Cheng) [#​56885](nodejs/node#56885) - \[[`ea61b956e9`](nodejs/node@ea61b956e9)] - **build**: add double quotes around <(python) (Luigi Pinca) [#​56826](nodejs/node#56826) - \[[`14236ef778`](nodejs/node@14236ef778)] - **build**: add build option suppress_all_error_on_warn (Michael Dawson) [#​56647](nodejs/node#56647) - \[[`dfd3f430f3`](nodejs/node@dfd3f430f3)] - **build,win**: enable ccache (Stefan Stojanovic) [#​56847](nodejs/node#56847) - \[[`3e207bd9ec`](nodejs/node@3e207bd9ec)] - **(SEMVER-MINOR)** **crypto**: support --use-system-ca on Windows (Joyee Cheung) [#​56833](nodejs/node#56833) - \[[`fe2694a992`](nodejs/node@fe2694a992)] - **crypto**: fix X509\* leak in --use-system-ca (Joyee Cheung) [#​56832](nodejs/node#56832) - \[[`60039a2c36`](nodejs/node@60039a2c36)] - **crypto**: add api to get openssl security level (Michael Dawson) [#​56601](nodejs/node#56601) - \[[`39a474f7c0`](nodejs/node@39a474f7c0)] - **(SEMVER-MINOR)** **crypto**: added support for reading certificates from macOS system store (Tim Jacomb) [#​56599](nodejs/node#56599) - \[[`144bee8067`](nodejs/node@144bee8067)] - **deps**: update zlib to 1.3.0.1-motley-788cb3c (Node.js GitHub Bot) [#​56655](nodejs/node#56655) - \[[`7fd39e3a79`](nodejs/node@7fd39e3a79)] - **deps**: update sqlite to 3.49.0 (Node.js GitHub Bot) [#​56654](nodejs/node#56654) - \[[`d698cb5434`](nodejs/node@d698cb5434)] - **deps**: update amaro to 0.3.2 (marco-ippolito) [#​56916](nodejs/node#56916) - \[[`dbd09067c0`](nodejs/node@dbd09067c0)] - **deps**: V8: cherry-pick [`9ab4059`](nodejs/node@9ab40592f697) (Levi Zim) [#​56781](nodejs/node#56781) - \[[`ee33ef3aa6`](nodejs/node@ee33ef3aa6)] - **deps**: update cjs-module-lexer to 2.0.0 (Michael Dawson) [#​56855](nodejs/node#56855) - \[[`c0542557d0`](nodejs/node@c0542557d0)] - **deps**: update timezone to 2025a (Node.js GitHub Bot) [#​56876](nodejs/node#56876) - \[[`d67cb1f9bb`](nodejs/node@d67cb1f9bb)] - **deps**: update simdjson to 3.12.0 (Node.js GitHub Bot) [#​56874](nodejs/node#56874) - \[[`70b04b4314`](nodejs/node@70b04b4314)] - **deps**: update googletest to [`e235eb3`](nodejs/node@e235eb3) (Node.js GitHub Bot) [#​56873](nodejs/node#56873) - \[[`e11cda003f`](nodejs/node@e11cda003f)] - **(SEMVER-MINOR)** **deps**: update ada to v3.0.1 (Yagiz Nizipli) [#​56452](nodejs/node#56452) - \[[`8743ef525d`](nodejs/node@8743ef525d)] - **deps**: update simdjson to 3.11.6 (Node.js GitHub Bot) [#​56250](nodejs/node#56250) - \[[`0f553e5575`](nodejs/node@0f553e5575)] - **deps**: update amaro to 0.3.1 (Node.js GitHub Bot) [#​56785](nodejs/node#56785) - \[[`380a8d8d2f`](nodejs/node@380a8d8d2f)] - **(SEMVER-MINOR)** **deps,tools**: add zstd 1.5.6 (Jan Krems) [#​52100](nodejs/node#52100) - \[[`66898a7c3b`](nodejs/node@66898a7c3b)] - **doc**: update history of stream.Readable.toWeb() (Jimmy Leung) [#​56928](nodejs/node#56928) - \[[`9e29416e12`](nodejs/node@9e29416e12)] - **doc**: make MDN links to global classes more consistent (Antoine du Hamel) [#​56924](nodejs/node#56924) - \[[`6bc270728a`](nodejs/node@6bc270728a)] - **doc**: make MDN links to global classes more consistent in `assert.md` (Antoine du Hamel) [#​56920](nodejs/node#56920) - \[[`00da003171`](nodejs/node@00da003171)] - **doc**: make MDN links to global classes more consistent (Antoine du Hamel) [#​56923](nodejs/node#56923) - \[[`d90198793a`](nodejs/node@d90198793a)] - **doc**: make MDN links to global classes more consistent in `util.md` (Antoine du Hamel) [#​56922](nodejs/node#56922) - \[[`5f4377a759`](nodejs/node@5f4377a759)] - **doc**: make MDN links to global classes more consistent in `buffer.md` (Antoine du Hamel) [#​56921](nodejs/node#56921) - \[[`7353266b50`](nodejs/node@7353266b50)] - **doc**: improve type stripping documentation (Marco Ippolito) [#​56916](nodejs/node#56916) - \[[`888d2acc3a`](nodejs/node@888d2acc3a)] - **doc**: specificy support for erasable ts syntax (Marco Ippolito) [#​56916](nodejs/node#56916) - \[[`3c082d43bc`](nodejs/node@3c082d43bc)] - **doc**: update post sec release process (Rafael Gonzaga) [#​56907](nodejs/node#56907) - \[[`f0bf35d3c5`](nodejs/node@f0bf35d3c5)] - **doc**: update websocket link to avoid linking to self (Chengzhong Wu) [#​56897](nodejs/node#56897) - \[[`373dbb0e6c`](nodejs/node@373dbb0e6c)] - **doc**: mark `--env-file-if-exists` flag as experimental (Juan José) [#​56893](nodejs/node#56893) - \[[`d436888cc8`](nodejs/node@d436888cc8)] - **doc**: fix typo in cjs example of `util.styleText` (Deokjin Kim) [#​56769](nodejs/node#56769) - \[[`91638eeb4a`](nodejs/node@91638eeb4a)] - **doc**: clarify sqlite user-defined function behaviour (René) [#​56786](nodejs/node#56786) - \[[`bab9c4d331`](nodejs/node@bab9c4d331)] - **events**: getMaxListeners detects 0 listeners (Matthew Aitken) [#​56807](nodejs/node#56807) - \[[`ccaf7fe737`](nodejs/node@ccaf7fe737)] - **fs**: make `FileHandle.readableWebStream` always create byte streams (Ian Kerins) [#​55461](nodejs/node#55461) - \[[`974cec7a0a`](nodejs/node@974cec7a0a)] - **http**: be more generational GC friendly (ywave620) [#​56767](nodejs/node#56767) - \[[`be00058712`](nodejs/node@be00058712)] - **inspector**: add Network.Initiator in inspector protocol (Chengzhong Wu) [#​56805](nodejs/node#56805) - \[[`31293a4b09`](nodejs/node@31293a4b09)] - **inspector**: fix GN build (Cheng) [#​56798](nodejs/node#56798) - \[[`91a302356b`](nodejs/node@91a302356b)] - **inspector**: fix StringUtil::CharacterCount for unicodes (Chengzhong Wu) [#​56788](nodejs/node#56788) - \[[`3b305f25f2`](nodejs/node@3b305f25f2)] - **lib**: filter node:quic from builtinModules when flag not used (James M Snell) [#​56870](nodejs/node#56870) - \[[`f06ee4c54a`](nodejs/node@f06ee4c54a)] - **meta**: bump `actions/upload-artifact` from 4.4.3 to 4.6.0 (dependabot\[bot]) [#​56861](nodejs/node#56861) - \[[`d230bc3b3c`](nodejs/node@d230bc3b3c)] - **meta**: bump `actions/setup-node` from 4.1.0 to 4.2.0 (dependabot\[bot]) [#​56868](nodejs/node#56868) - \[[`d4ecfa745e`](nodejs/node@d4ecfa745e)] - **meta**: move one or more collaborators to emeritus (Node.js GitHub Bot) [#​56889](nodejs/node#56889) - \[[`698c56bb94`](nodejs/node@698c56bb94)] - **meta**: add [@​nodejs/url](https://github.com/nodejs/url) as codeowner (Chengzhong Wu) [#​56783](nodejs/node#56783) - \[[`a274b28857`](nodejs/node@a274b28857)] - **module**: fix require.resolve() crash on non-string paths (Aditi) [#​56942](nodejs/node#56942) - \[[`4e3052aeee`](nodejs/node@4e3052aeee)] - **quic**: fixup errant LocalVector usage (James M Snell) [#​56564](nodejs/node#56564) - \[[`dfc61f7bb7`](nodejs/node@dfc61f7bb7)] - **readline**: fix unresolved promise on abortion (Daniel Venable) [#​54030](nodejs/node#54030) - \[[`9e60501f5e`](nodejs/node@9e60501f5e)] - **sqlite**: fix coverity warnings related to backup() (Colin Ihrig) [#​56961](nodejs/node#56961) - \[[`1913a4aabc`](nodejs/node@1913a4aabc)] - **sqlite**: restore changes from [#​55373](nodejs/node#55373) (Colin Ihrig) [#​56908](nodejs/node#56908) - \[[`8410c955b7`](nodejs/node@8410c955b7)] - **sqlite**: fix use-after-free in StatementSync due to premature GC (Divy Srivastava) [#​56840](nodejs/node#56840) - \[[`01d732d629`](nodejs/node@01d732d629)] - **sqlite**: handle conflicting SQLite and JS errors (Colin Ihrig) [#​56787](nodejs/node#56787) - \[[`39997867cf`](nodejs/node@39997867cf)] - **(SEMVER-MINOR)** **sqlite**: allow returning `ArrayBufferView`s from user-defined functions (René) [#​56790](nodejs/node#56790) - \[[`8dc637681a`](nodejs/node@8dc637681a)] - **sqlite, test**: expose sqlite online backup api (Edy Silva) [#​56253](nodejs/node#56253) - \[[`cfea53eccc`](nodejs/node@cfea53eccc)] - **src**: use `args.This()` in zlib (Michaël Zasso) [#​56988](nodejs/node#56988) - \[[`6b398d6d0b`](nodejs/node@6b398d6d0b)] - **src**: replace `SplitString` with built-in (Yagiz Nizipli) [#​54990](nodejs/node#54990) - \[[`fbb32e0a08`](nodejs/node@fbb32e0a08)] - **src**: add nullptr handling for `NativeKeyObject` (Burkov Egor) [#​56900](nodejs/node#56900) - \[[`83ff7be9fd`](nodejs/node@83ff7be9fd)] - **src**: disallow copy/move fns/constructors (Yagiz Nizipli) [#​56811](nodejs/node#56811) - \[[`63611d0331`](nodejs/node@63611d0331)] - **src**: add a hard dependency v8\_inspector_headers (Chengzhong Wu) [#​56805](nodejs/node#56805) - \[[`3d957d135c`](nodejs/node@3d957d135c)] - **src**: improve error handling in encoding_binding.cc (James M Snell) [#​56915](nodejs/node#56915) - \[[`9e9ac3ccd8`](nodejs/node@9e9ac3ccd8)] - **src**: avoid copy by using std::views::keys (Yagiz Nizipli) [#​56080](nodejs/node#56080) - \[[`086cdc297a`](nodejs/node@086cdc297a)] - **src**: remove obsolete NoArrayBufferZeroFillScope (James M Snell) [#​56913](nodejs/node#56913) - \[[`915d7aeb37`](nodejs/node@915d7aeb37)] - **src**: set signal inspector io thread name (RafaelGSS) [#​56416](nodejs/node#56416) - \[[`f4b086d29d`](nodejs/node@f4b086d29d)] - **src**: set thread name for main thread and v8 worker (RafaelGSS) [#​56416](nodejs/node#56416) - \[[`3579143630`](nodejs/node@3579143630)] - **src**: set worker thread name using worker.name (RafaelGSS) [#​56416](nodejs/node#56416) - \[[`736ff5de6d`](nodejs/node@736ff5de6d)] - **src**: use a default thread name for inspector (RafaelGSS) [#​56416](nodejs/node#56416) - \[[`be8e2b4d8f`](nodejs/node@be8e2b4d8f)] - **src**: improve error handling in permission.cc (James M Snell) [#​56904](nodejs/node#56904) - \[[`d6cf0911ee`](nodejs/node@d6cf0911ee)] - **src**: improve error handling in node_sqlite (James M Snell) [#​56891](nodejs/node#56891) - \[[`521fed1bac`](nodejs/node@521fed1bac)] - **src**: improve error handling in node_os by removing ToLocalChecked (James M Snell) [#​56888](nodejs/node#56888) - \[[`c9a99df8e7`](nodejs/node@c9a99df8e7)] - **src**: improve error handling in node_url (James M Snell) [#​56886](nodejs/node#56886) - \[[`5c82ef3ace`](nodejs/node@5c82ef3ace)] - **src**: add memory retainer traits for external types (Chengzhong Wu) [#​56881](nodejs/node#56881) - \[[`edb194b2d5`](nodejs/node@edb194b2d5)] - **src**: prevent URLPattern property accessors from crashing on invalid this (James M Snell) [#​56877](nodejs/node#56877) - \[[`9624049414`](nodejs/node@9624049414)] - **src**: pull in more electron boringssl adjustments (James M Snell) [#​56858](nodejs/node#56858) - \[[`f8910e384d`](nodejs/node@f8910e384d)] - **src**: make multiple improvements to node_url_pattern (James M Snell) [#​56871](nodejs/node#56871) - \[[`94a0237b18`](nodejs/node@94a0237b18)] - **src**: clean up some obsolete crypto methods (James M Snell) [#​56792](nodejs/node#56792) - \[[`b240ca67b9`](nodejs/node@b240ca67b9)] - **src**: add check for Bignum in GroupOrderSize (Burkov Egor) [#​56702](nodejs/node#56702) - \[[`45692e9c7c`](nodejs/node@45692e9c7c)] - **src, deps**: port electron's boringssl workarounds (James M Snell) [#​56812](nodejs/node#56812) - \[[`a9d80d43cb`](nodejs/node@a9d80d43cb)] - **(SEMVER-MINOR)** **src, quic**: refine more of the quic implementation (James M Snell) [#​56328](nodejs/node#56328) - \[[`93d0beb6c8`](nodejs/node@93d0beb6c8)] - **src,test**: expand test coverage for urlpattern and fix error (James M Snell) [#​56878](nodejs/node#56878) - \[[`5a9732e1d0`](nodejs/node@5a9732e1d0)] - **test**: improve timeout duration for debugger events (Yagiz Nizipli) [#​56970](nodejs/node#56970) - \[[`60c8fc07ff`](nodejs/node@60c8fc07ff)] - **test**: remove unnecessary syscall to cpuinfo (Yagiz Nizipli) [#​56968](nodejs/node#56968) - \[[`40cdf756e6`](nodejs/node@40cdf756e6)] - **test**: update webstorage wpt (Yagiz Nizipli) [#​56963](nodejs/node#56963) - \[[`de77371a9e`](nodejs/node@de77371a9e)] - **test**: execute shell directly for refresh() (Yagiz Nizipli) [#​55051](nodejs/node#55051) - \[[`f4254b8e70`](nodejs/node@f4254b8e70)] - **test**: automatically sync wpt urlpattern tests (Jonas) [#​56949](nodejs/node#56949) - \[[`a473d3f57a`](nodejs/node@a473d3f57a)] - **test**: update snapshots for amaro v0.3.2 (Marco Ippolito) [#​56916](nodejs/node#56916) - \[[`abca97f7e2`](nodejs/node@abca97f7e2)] - **test**: change jenkins reporter (Carlos Espa) [#​56808](nodejs/node#56808) - \[[`7c9fa11127`](nodejs/node@7c9fa11127)] - **test**: fix race condition in test-child-process-bad-stdio (Colin Ihrig) [#​56845](nodejs/node#56845) - \[[`b8b6e68836`](nodejs/node@b8b6e68836)] - **(SEMVER-MINOR)** **test**: add WPT for URLPattern (Yagiz Nizipli) [#​56452](nodejs/node#56452) - \[[`b6d3d52e20`](nodejs/node@b6d3d52e20)] - **test**: adjust check to use OpenSSL sec level (Michael Dawson) [#​56819](nodejs/node#56819) - \[[`3beac87f92`](nodejs/node@3beac87f92)] - **test**: test-crypto-scrypt.js doesn't need internals (Meghan Denny) [#​56673](nodejs/node#56673) - \[[`3af23a10f3`](nodejs/node@3af23a10f3)] - **test**: set `test-fs-cp` as flaky (Stefan Stojanovic) [#​56799](nodejs/node#56799) - \[[`1146f48f67`](nodejs/node@1146f48f67)] - **test**: search cctest files (Chengzhong Wu) [#​56791](nodejs/node#56791) - \[[`86c199b25a`](nodejs/node@86c199b25a)] - **test**: convert test_encoding_binding.cc to a JS test (Chengzhong Wu) [#​56791](nodejs/node#56791) - \[[`bd5484717c`](nodejs/node@bd5484717c)] - **test**: test-crypto-prime.js doesn't need internals (Meghan Denny) [#​56675](nodejs/node#56675) - \[[`f5f54414e4`](nodejs/node@f5f54414e4)] - **test**: temporary remove resource check from fs read-write (Rafael Gonzaga) [#​56789](nodejs/node#56789) - \[[`c8bd2ba0ad`](nodejs/node@c8bd2ba0ad)] - **test**: mark test-without-async-context-frame flaky on windows (James M Snell) [#​56753](nodejs/node#56753) - \[[`2c2e4a4ae0`](nodejs/node@2c2e4a4ae0)] - **test**: remove unnecessary code (Luigi Pinca) [#​56784](nodejs/node#56784) - \[[`4606a5f79b`](nodejs/node@4606a5f79b)] - **test**: mark `test-esm-loader-hooks-inspect-wait` flaky (Richard Lau) [#​56803](nodejs/node#56803) - \[[`38c77e3462`](nodejs/node@38c77e3462)] - **test**: update WPT for url to [`a23788b`](nodejs/node@a23788b77a) (Node.js GitHub Bot) [#​56779](nodejs/node#56779) - \[[`50ebd5fd31`](nodejs/node@50ebd5fd31)] - **test**: remove duplicate error reporter from ci (Carlos Espa) [#​56739](nodejs/node#56739) - \[[`0c3ae25aec`](nodejs/node@0c3ae25aec)] - **test_runner**: print formatted errors on summary (Pietro Marchini) [#​56911](nodejs/node#56911) - \[[`b5a8a812fb`](nodejs/node@b5a8a812fb)] - **tools**: bump eslint version (dependabot\[bot]) [#​56869](nodejs/node#56869) - \[[`e1f86c1b9d`](nodejs/node@e1f86c1b9d)] - **tools**: remove test-asan/ubsan workflows (Michaël Zasso) [#​56823](nodejs/node#56823) - \[[`405a6678b7`](nodejs/node@405a6678b7)] - **tools**: run macOS test workflow with Xcode 16.1 (Michaël Zasso) [#​56831](nodejs/node#56831) - \[[`16529c130f`](nodejs/node@16529c130f)] - **tools**: update sccache and sccache-action (Michaël Zasso) [#​56815](nodejs/node#56815) - \[[`fe004111ea`](nodejs/node@fe004111ea)] - **tools**: fix license-builder for inspector_protocol (Michaël Zasso) [#​56814](nodejs/node#56814) - \[[`bc97a90176`](nodejs/node@bc97a90176)] - **(SEMVER-MINOR)** **url**: add URLPattern implementation (Yagiz Nizipli) [#​56452](nodejs/node#56452) - \[[`77294d8918`](nodejs/node@77294d8918)] - **util**: enforce shouldColorize in styleText array arg (Marco Ippolito) [#​56722](nodejs/node#56722) - \[[`8e6c191601`](nodejs/node@8e6c191601)] - **zlib**: use modern class syntax for zstd classes (Yagiz Nizipli) [#​56965](nodejs/node#56965) - \[[`a3ca7f37a2`](nodejs/node@a3ca7f37a2)] - **zlib**: make all zstd functions experimental (Yagiz Nizipli) [#​56964](nodejs/node#56964) - \[[`4cc7907738`](nodejs/node@4cc7907738)] - **(SEMVER-MINOR)** **zlib**: add zstd support (Jan Krems) [#​52100](nodejs/node#52100) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xNjkuMyIsInVwZGF0ZWRJblZlciI6IjM5LjE2OS4zIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: nodejs#56328 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
|
This commit didn't land cleanly on v22.x-staging. It requires a manual backport, so I'm adding a backport-requested label. |
Sorry, something went wrong.
|
Future work on QUIC will be specific to Node.js 24+ since it requires OpenSSL 3.5 This backport can be skipped. EDIT: I've removed backport-requested label, and added donr-land labels for 18, 20, 22. |
Sorry, something went wrong.
|
I actually think a backport PR that removes all of the quic stuff from older versions would probably be ideal. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
There's a lot more to do to get things fully working but this makes more incremental progress.
Several key bits:
Key question for reviewers: Looking at the API documented in quic.md and demonstrated in test-quic-handshake, what API do you want to see here? This is focusing ONLY on the QUIC API at this point and does NOT touch on the http3 API.
For the server-side, the API is essentially:
For the client side it is essentially:
The API still feels rather awkward. For review, contrast with what Deno is doing here: https://github.com/denoland/deno/pull/21942/files#diff-1645ba9a2a2aac4440671da62b81ab9a723f50faffff0d0e8d016a1f991961a3
At this point there are bugs in the implementation still, yes, but let's focus on the API design at the moment. Please focus review comments on that.
A couple of bits to consider for the reivew...
In this API design, the QuicEndpoint represents the local binding the local UDP port. It supports being both a server and client at the same time. Use can be a bit awkward though so we need to carefully review. Specifically, a single QuicEndpoint can support any number of QuicSessions (both client and server at the same time). This gives maximum flexibility but means a more complicated API. For example, let's take a closer look at the client side example:
The key question here is: does it make sense to expose QuicEndpoint like this? If we look at the Deno API, as an alternative, we see something like:
In Deno's API, they've effectively hidden QuicEndpoint under their notion of QuicConn. While it might be the case that the same underlying UDP port is used when ever connectQuic(...) is called, if it is that ends up being an internal implementation detail and not something that is exposed to users. Which approach is better? Exposing QuicEndpoint the way that I am means we have more flexibility but also more complexity that may not be worth it?
Alternatively, we could go with a simplified API where every call to connect(...) uses a separate QuicEndpoint, so, for instance:
We could still allow for direct reuse of QuicEndpoint in this model by allowing a QuicEndpoint instance to be passed as an option to connect(...), like:
But the question here, whenever client connections are sharing a single endpoint is (a) when should the endpoint be closed, (b) should it ever be closed automatically?, (c) should it be unref'd so that if there are no active sessions but the client-side endpoint has not yet been closed, should it keep the node.js process from exiting?
Basically, looking for feedback on the API design so if you have other ideas for what color to paint the bikeshed, now is the time to discuss it.
@Qard @mcollina @anonrig @nodejs/quic