Requiring `respondWithFile()` to only work with regular files
is an artificial restriction on Node’s side and has become unnecessary.
Offsets or lengths cannot be specified for those files,
but that is an inherent property of other file types.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This resolves the issue of using synchronous I/O for
`respondWithFile()` and `respondWithFD()`, and enables
scenarios in which the underlying file does not need
to be a regular file.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
There should be no default error handling when using Http2Stream.
All errors will end up in `'streamError'` on the server anyway,
but they are emitted on `'stream'` as well, otherwise some error
conditions are impossible to debug.
See: https://github.com/nodejs/node/pull/14991
PR-URL: https://github.com/nodejs/node/pull/19232
Reviewed-By: James M Snell <jasnell@gmail.com>
This improves error handling for streams in a few ways.
1. It ensures that no user defined methods (_read, _write, ...) are run
after .destroy has been called.
2. It introduces an explicit error to tell the user if they are write to
write, etc to the stream after it has been destroyed.
3. It makes streams always emit close as the last thing after they have
been destroyed
4. Changes the default _destroy to not gracefully end streams.
It also updates net, http2, zlib and fs to the new error handling.
PR-URL: https://github.com/nodejs/node/pull/18438
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Display the constant name instead of a stream error code
in the error message, because the numerical codes give absolutely
no clue about what happened when an error is emitted.
PR-URL: https://github.com/nodejs/node/pull/18966
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Do not close the request if callback is not a function, and
throw ERR_INVALID_CALLBACK TypeError
PR-URL: https://github.com/nodejs/node/pull/19061
Fixes: https://github.com/nodejs/node/issues/18855
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Previously, if `session.destroy()` was called with an error object,
the information contained in it would be discarded and a generic
`ERR_HTTP2_STREAM_CANCEL` would be used for all pending streams.
Instead, make the information from the original error object
available.
PR-URL: https://github.com/nodejs/node/pull/18988
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Send a human-readable HTTP/1 response in case of an unexpected
ALPN protocol. This helps with debugging this condition,
since previously the only result of it would be a closed socket.
PR-URL: https://github.com/nodejs/node/pull/18986
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Instead of exposing internals of async_hooks & async_wrap throughout
the code base, create necessary helper methods within the internal
async_hooks that allows easy usage by Node.js internals. This stops
every single internal user of async_hooks from importing a ton of
functions, constants and internal Aliased Buffers from C++ async_wrap.
Adds functions initHooksExist, afterHooksExist, and destroyHooksExist
to determine whether the related emit methods need to be triggered.
Adds clearDefaultTriggerAsyncId and clearAsyncIdStack on the JS side
as an alternative to always calling C++.
Moves async_id_symbol and trigger_async_id_symbol to internal
async_hooks as they are never used in C++.
Renames newUid to newAsyncId for added clarity of its purpose.
Adjusts usage throughout the codebase, as well as in a couple of tests.
PR-URL: https://github.com/nodejs/node/pull/18720
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Encapsulate stream requests more:
- `WriteWrap` and `ShutdownWrap` classes are now tailored to the
streams on which they are used. In particular, for most streams
these are now plain `AsyncWrap`s and do not carry the overhead
of unused libuv request data.
- Provide generic `Write()` and `Shutdown()` methods that wrap
around the actual implementations, and make *usage* of streams
easier, rather than implementing; for example, wrap objects
don’t need to be provided by callers anymore.
- Use `EmitAfterWrite()` and `EmitAfterShutdown()` handlers to
call the corresponding JS handlers, rather than always trying
to call them. This makes usage of streams by other C++ code
easier and leaner.
Also fix up some tests that were previously not actually testing
asynchronicity when the comments indicated that they would.
PR-URL: https://github.com/nodejs/node/pull/18676
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add optional Http2ServerRequest and Http2ServerResponse options
to createServer and createSecureServer. Allows custom req & res
classes that extend the default ones to be used without
overriding the prototype.
PR-URL: https://github.com/nodejs/node/pull/15560
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit moves error creation helpers scattered around
under lib/ into lib/internal/errors.js in the hope of being clearer
about the differences of errors that we throw into the user land.
- Move util._errnoException and util._exceptionWithHostPort
into internal/errors.js and simplify their logic so it's
clearer what the properties these helpers create.
- Move the errnoException helper in dns.js to internal/errors.js
into internal/errors.js and rename it to dnsException. Simplify
it's logic so it no longer calls errnoException and skips
the unnecessary argument checks.
PR-URL: https://github.com/nodejs/node/pull/18546
Reviewed-By: James M Snell <jasnell@gmail.com>
This adds the Http1IncomingMessage and Http1ServerReponse options
to http2.createServer().
PR-URL: https://github.com/nodejs/node/pull/15752
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Non-ASCII characters in /lib get compiled into the node binary,
and may bloat the binary size unnecessarily. A linter rule may
help prevent this.
PR-URL: https://github.com/nodejs/node/pull/18043
Fixes: https://github.com/nodejs/node/issues/11209
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Hidden via a symbol because I'm unsure exactly what the API should look
like in the end.
Removes the need to use _unrefActive for efficiently refreshing
timeouts.
It still uses it under the hood but that could be replaced with
insert() directly if it were in the same file.
PR-URL: https://github.com/nodejs/node/pull/18065
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Refactor the read mechanism to completely avoid copying.
Instead of copying individual `DATA` frame contents into buffers,
create `ArrayBuffer` instances for all socket reads and emit
slices of those `ArrayBuffer`s to JS.
PR-URL: https://github.com/nodejs/node/pull/18030
Reviewed-By: James M Snell <jasnell@gmail.com>
Fixes an oversight from
93eb68e6d2
Wasn't caught by a test.
PR-URL: https://github.com/nodejs/node/pull/18062
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/18018
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Add new properties to `Http2Session` to identify alpnProtocol,
and indicator about whether the session is TLS or not, and
initial support for origin set (preparinng for `ORIGIN` frame
support and the client-side `Pool` implementation.
The `originSet` is the set of origins for which an `Http2Session`
may be considered authoritative. Per the `ORIGIN` frame spec,
the originSet is only valid on TLS connections, so this is only
exposed when using a `TLSSocket`.
PR-URL: https://github.com/nodejs/node/pull/17935
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/17939
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
There two similar error codes in lib: "ERR_VALUE_OUT_OF_RANGE"
and "ERR_OUT_OF_RANGE". This change is to reduce them into
"ERR_VALUE_OUT_OF_RANGE"
Fixes: https://github.com/nodejs/node/issues/17603
PR-URL: https://github.com/nodejs/node/pull/17648
Fixes: https://github.com/nodejs/node/issues/17603
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This makes `Http2Stream`s and `Http2Session`s use actual Timeout
objects in a [kTimeout] symbol property, rather than making the
stream/session itself a timer and appending properties to it directly.
PR-URL: https://github.com/nodejs/node/pull/17704
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
`OnCallbackPadding` on the native side already clamps
the return value into the right range, so there’s not need
to also do that on the JS side.
Also, use `>>> 0` instead of `| 0` to get an uint32, since
the communication with C++ land happens through an Uint32Array.
PR-URL: https://github.com/nodejs/node/pull/17717
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/17406
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
This is a significant cleanup and refactoring of the
cleanup/close/destroy logic for Http2Stream and Http2Session.
There are significant changes here in the timing and ordering
of cleanup logic, JS apis. and various related necessary edits.
* fixup js debug messages
* simplify and improve rstStream
* improve and simplify _read
* simplify and improve priority
* simplify on ready a bit
* simplify and improve respond/push
* reduce duplication with _unrefActive
* simplify stream close handling
PR-URL: https://github.com/nodejs/node/pull/17209
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
This update does several significant things:
1. It eliminates the base Nghttp2* classes and folds those
in to node::http2::Http2Session and node::http2::Http2Stream
2. It makes node::http2::Http2Stream a StreamBase instance and
sends that out to JS-land to act as the [kHandle] for the
JavaScript Http2Stream class.
3. It shifts some of the callbacks from C++ off of the JavaScript
Http2Session class to the Http2Stream class.
4. It refactors the data provider structure for FD and Stream
based sending to help encapsulate those functions easier
5. It streamlines some of the functions at the C++ layer to
eliminate now unnecessary redirections
6. It cleans up node_http2.cc for better readability and
maintainability
7. It refactors some of the debug output
8. Because Http2Stream instances are now StreamBases, they are
now also trackable using async-hooks
9. The Stream::OnRead algorithm has been simplified with a
couple bugs fixed.
10. I've eliminated node_http2_core.h and node_http2_core-inl.h
11. Detect invalid handshake a report protocol error to session
12. Refactor out of memory error, improve other errors
13. Add Http2Session.prototype.ping
PR-URL: https://github.com/nodejs/node/pull/17105
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
The initials of expected in TypeError[ERR_INVALID_ARG_TYPE]
are inconsistent. This change is to unify them.
PR-URL: https://github.com/nodejs/node/pull/16401
Fixes: https://github.com/nodejs/node/issues/16383
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Do not call destroy each time rstStream is called since the
first call (or receipt of rst frame) will always trigger
destroy. Expand existing test for this behaviour.
PR-URL: https://github.com/nodejs/node/pull/16753
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
_read should always resume the underlying code that is attempting
to push data to a readable stream. Adjust http2 core code to
resume its reading appropriately.
Some other general cleanup around reading, resuming & draining.
PR-URL: https://github.com/nodejs/node/pull/16580
Fixes: https://github.com/nodejs/node/issues/16578
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* correctly reset write timers: currently reset timers on
both session & stream when write starts and when it ends.
* prevent large writes from timing out: when writing a large
chunk of data in http2, once the data is handed off to C++,
the JS session & stream lose all track of the write and will
timeout if the write doesn't complete within the timeout window
Fix this issue by tracking whether a write request is ongoing and
also tracking how many chunks have been sent since the most recent
write started. (Since each write call resets the timer.)
PR-URL: https://github.com/nodejs/node/pull/16525
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
* move CHECK statements into DEBUG checks
* improve performance by removing branches
* Several if checks were left in while the code was being developed.
Now that the core API has stablized more, the checks are largely
unnecessary and can be removed, yielding a significant boost in
performance.
* refactor flow control for proper backpressure
* use std::queue for inbound headers
* use std::queue for outbound data
* remove now unnecessary FreeHeaders function
* expand comments and miscellaneous edits
* add a couple of misbehaving flow control tests
PR-URL: https://github.com/nodejs/node/pull/16239
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Due to how WHATWG-URL parser works, port numbers are omitted if
they are the default port for a scheme. This meant that
http2.connect could not accept connections on port 80 with http
scheme. Fix this bug by detecting http: scheme and setting port
to 80.
PR-URL: https://github.com/nodejs/node/pull/16337
Fixes: https://github.com/nodejs/node/issues/14304
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Support generic `Duplex` streams through using `StreamWrap`
on the server and client sides, and adding a `createConnection`
method option similar to what the HTTP/1 API provides.
Since HTTP2 is, as a protocol, independent of its underlying transport
layer, Node.js should not enforce any restrictions on what streams
its internals may use.
Ref: https://github.com/nodejs/node/issues/16256
PR-URL: https://github.com/nodejs/node/pull/16269
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
A few small changes:
- fix debug statement location
- simplify conditional with returns
- consistent use of template strings
PR-URL: https://github.com/nodejs/node/pull/16327
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
options.getTrailers & options.selectPadding will always be a
function by the time the code reaches the assert checks. And
even if not (which seems currently impossible), the assert
failure will be almost the same as calling something that is
not a function. The downside of leaving these is that typeof
checks are decently expensive.
PR-URL: https://github.com/nodejs/node/pull/16327
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
