Fix HTTP/2 RST_STREAM behaviour, add auto-drain, deprecate 'aborted', fix related compat API issues

doc/api/deprecations.md

@@ -4574,7 +4574,22 @@
 may lead to subtle bugs. Calling `hmac.digest()` on a finalized `Hmac` instance
 will throw an error in a future version.
 
+### DEP0207: `.aborted` property and `'aborted'` event in `http2`
+
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/000000
+    description: Documentation-only deprecation.
+-->
+
+Type: Documentation-only
+
+Use the standard {Stream} API: check `stream.destroyed` and listen for
+`'close'` and `'error'`. Parallels [DEP0156][] for `http`.
+
 [DEP0142]: #dep0142-repl_builtinlibs
+[DEP0156]: #dep0156-aborted-property-and-abort-aborted-event-in-http
 [NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
 [RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3
 [RFC 8247 Section 2.4]: https://www.rfc-editor.org/rfc/rfc8247#section-2.4

doc/api/errors.md

@@ -1718,6 +1718,15 @@ Use of the `101` Informational status code is forbidden in HTTP/2.
 An invalid HTTP status code has been specified. Status codes must be an integer
 between `100` and `599` (inclusive).
 
+<a id="ERR_HTTP2_STREAM_ABORTED"></a>
+
+### `ERR_HTTP2_STREAM_ABORTED`
+
+The peer reset the `Http2Stream` with a clean error code (`NGHTTP2_NO_ERROR`
+or `NGHTTP2_CANCEL`) before sending `END_STREAM`, so the readable side will
+not be fully delivered. Mirrors HTTP/1's `ECONNRESET` for a peer-side
+`socket.destroy()`.
+
 <a id="ERR_HTTP2_STREAM_CANCEL"></a>
 
 ### `ERR_HTTP2_STREAM_CANCEL`

doc/api/http2.md

@@ -1236,22 +1236,23 @@
 
 ##### Destruction
 
-All [`Http2Stream`][] instances are destroyed either when:
+All [`Http2Stream`][] instances are destroyed when one of the following
+happens:
 
-* An `RST_STREAM` frame for the stream is received by the connected peer,
-  and (for client streams only) pending data has been read.
-* The `http2stream.close()` method is called, and (for client streams only)
-  pending data has been read.
-* The `http2stream.destroy()` or `http2session.destroy()` methods are called.
+* Both sides send `END_STREAM` (a clean exchange).
+* The peer sends an `RST_STREAM` frame.
+* `http2stream.close()`, `http2stream.destroy()`, or `http2session.destroy()`
+  is called locally.
 
-When an `Http2Stream` instance is destroyed, an attempt will be made to send an
-`RST_STREAM` frame to the connected peer.
+For clean exchanges and clean cancels, the destroy is deferred until any
+pending `'end'` and `'finish'` events have fired. When destroyed, an
+attempt is made to send an `RST_STREAM` frame to the connected peer if
+one hasn't already been sent.
 
-When the `Http2Stream` instance is destroyed, the `'close'` event will
-be emitted. Because `Http2Stream` is an instance of `stream.Duplex`, the
-`'end'` event will also be emitted if the stream data is currently flowing.
-The `'error'` event may also be emitted if `http2stream.destroy()` was called
-with an `Error` passed as the first argument.
+`'close'` is always emitted on destroy. `'end'` and `'finish'` fire if
+their respective halves completed before destroy. `'error'` fires when
+the destroy carries an error — either via `http2stream.destroy(err)`,
+or when the peer reset the stream before sending `END_STREAM`.
 
 After the `Http2Stream` has been destroyed, the `http2stream.destroyed`
 property will be `true` and the `http2stream.rstCode` property will specify the
@@ -1262,14 +1263,18 @@
 
 <!-- YAML
 added: v8.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/000000
+    description: Documentation-only deprecation.
 -->
 
-The `'aborted'` event is emitted whenever a `Http2Stream` instance is
-abnormally aborted in mid-communication.
-Its listener does not expect any arguments.
+> Stability: 0 - Deprecated. Use `'close'` and `'error'` plus
+> `stream.destroyed`.
 
-The `'aborted'` event will only be emitted if the `Http2Stream` writable side
-has not been ended.
+Emitted when an `Http2Stream` is closed before the writable side has
+been ended (via `.end()` or auto-ended via `respond({ endStream: true })`).
+Listeners receive no arguments.
 
 #### Event: `'close'`
 
@@ -1281,19 +1286,29 @@
 this event is emitted, the `Http2Stream` instance is no longer usable.
 
 The HTTP/2 error code used when closing the stream can be retrieved using
-the `http2stream.rstCode` property. If the code is any value other than
-`NGHTTP2_NO_ERROR` (`0`), an `'error'` event will have also been emitted.
+the `http2stream.rstCode` property.
 
 #### Event: `'error'`
 
 <!-- YAML
 added: v8.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/000000
+    description: >-
+      Also emitted on peer-initiated resets that arrive before
+      `END_STREAM` (`ERR_HTTP2_STREAM_ABORTED` for clean codes,
+      `ERR_HTTP2_STREAM_ERROR` otherwise). Locally-initiated resets
+      without an explicit error remain silent.
 -->
 
 * `error` {Error}
 
-The `'error'` event is emitted when an error occurs during the processing of
-an `Http2Stream`.
+Emitted when an error occurs processing the `Http2Stream`. This includes
+peer-initiated resets that arrive before the readable side has been
+fully delivered: a clean reset code (`NGHTTP2_NO_ERROR` or
+`NGHTTP2_CANCEL`) surfaces as [`ERR_HTTP2_STREAM_ABORTED`][], any other
+code as [`ERR_HTTP2_STREAM_ERROR`][].
 
 #### Event: `'frameError'`
 
@@ -1375,8 +1390,8 @@
 
 * Type: {boolean}
 
-Set to `true` if the `Http2Stream` instance was aborted abnormally. When set,
-the `'aborted'` event will have been emitted.
+`true` if the `Http2Stream` was closed while the writable side was
+still open. When set, the `'aborted'` event was emitted.
 
 #### `http2stream.bufferSize`
 
@@ -1719,6 +1734,13 @@
 
 <!-- YAML
 added: v8.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/000000
+    description: >-
+      If no `'response'` listener is attached when the response headers
+      arrive, the response body is now silently discarded - matching
+      the `lib/http` client behaviour.
 -->
 
 * `headers` {HTTP/2 Headers Object}
@@ -1740,6 +1762,16 @@
 });
 ```
 
+If no `'response'` listener is attached at the moment the response
+arrives, the response body will be entirely discarded (the stream is
+silently resumed). However, if a `'response'` listener is added, the
+data from the response object **must** be consumed — either by calling
+`response.read()` whenever there is a `'readable'` event, by adding a
+`'data'` handler, or by calling the `.resume()` method. Until the data
+is consumed, the `'end'` event will not fire. Also, until the data is
+read, it will consume memory that can eventually lead to a "process
+out of memory" error.
+
 ```cjs
 const http2 = require('node:http2');
 const client = http2.connect('https://localhost');
@@ -4031,11 +4063,8 @@
 added: v8.4.0
 -->
 
-The `'aborted'` event is emitted whenever a `Http2ServerRequest` instance is
-abnormally aborted in mid-communication.
-
-The `'aborted'` event will only be emitted if the `Http2ServerRequest` writable
-side has not been ended.
+The `'aborted'` event is emitted whenever a `Http2ServerRequest` instance
+is closed while the underlying writable side is still open.
 
 #### Event: `'close'`
 
@@ -5022,6 +5051,8 @@
 [`'unknownProtocol'`]: #event-unknownprotocol
 [`ClientHttp2Stream`]: #class-clienthttp2stream
 [`Duplex`]: stream.md#class-streamduplex
+[`ERR_HTTP2_STREAM_ABORTED`]: errors.md#err_http2_stream_aborted
+[`ERR_HTTP2_STREAM_ERROR`]: errors.md#err_http2_stream_error
 [`Http2ServerRequest`]: #class-http2http2serverrequest
 [`Http2ServerResponse`]: #class-http2http2serverresponse
 [`Http2Session` and Sockets]: #http2session-and-sockets

lib/internal/errors.js

@@ -1321,6 +1321,8 @@ E('ERR_HTTP2_SOCKET_UNBOUND',
 E('ERR_HTTP2_STATUS_101',
   'HTTP status code 101 (Switching Protocols) is forbidden in HTTP/2', Error);
 E('ERR_HTTP2_STATUS_INVALID', 'Invalid status code: %s', RangeError);
+E('ERR_HTTP2_STREAM_ABORTED',
+  'The stream was reset before the readable side was fully consumed', Error);
 E('ERR_HTTP2_STREAM_CANCEL', function(error) {
   let msg = 'The pending stream has been canceled';
   if (error) {

lib/internal/http2/compat.js

@@ -158,11 +158,11 @@ function onStreamEnd() {
 }
 
 function onStreamError(error) {
-  // This is purposefully left blank
-  //
-  // errors in compatibility mode are
-  // not forwarded to the request
-  // and response objects.
+  // Mirror HTTP/1's IncomingMessage._destroy: forward 'error' to req
+  // only when a listener is attached.
+  const request = this[kRequest];
+  if (request !== undefined && request.listenerCount('error') > 0)
+    request.emit('error', error);
 }
 
 function onRequestPause() {
@@ -460,7 +460,9 @@ function onStreamCloseResponse() {
   this.removeListener('wantTrailers', onStreamTrailersReady);
   this[kResponse] = undefined;
 
-  res.emit('finish');
+  // Only emit 'finish' when the underlying writable actually finished
+  if (this.writableFinished)
+    res.emit('finish');
   res.emit('close');
 }