http2: refactor how trailers are done

Rather than an option, introduce a method and an event...

```js
server.on('stream', (stream) => {
  stream.respond(undefined, { waitForTrailers: true });
  stream.on('wantTrailers', () => {
    stream.sendTrailers({ abc: 'xyz'});
  });
  stream.end('hello world');
});
```

This is a breaking change in the API such that the prior
`options.getTrailers` is no longer supported at all.
Ordinarily this would be semver-major and require a
deprecation but the http2 stuff is still experimental.

Backport-PR-URL: https://github.com/nodejs/node/pull/22850
PR-URL: https://github.com/nodejs/node/pull/19959
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

2 files changed, 127 insertions, 78 deletions

diff --git a/doc/api/errors.md b/doc/api/errors.md
index 1c5836e553d..1ab394580e7 100755
--- a/doc/api/errors.md
+++ b/doc/api/errors.md
@@ -861,6 +861,19 @@ When setting the priority for an HTTP/2 stream, the stream may be marked as
 a dependency for a parent stream. This error code is used when an attempt is
 made to mark a stream and dependent of itself.
 
+<a id="ERR_HTTP2_TRAILERS_ALREADY_SENT"></a>
+### ERR_HTTP2_TRAILERS_ALREADY_SENT
+
+Trailing headers have already been sent on the `Http2Stream`.
+
+<a id="ERR_HTTP2_TRAILERS_NOT_READY"></a>
+### ERR_HTTP2_TRAILERS_NOT_READY
+
+The `http2stream.sendTrailers()` method cannot be called until after the
+`'wantTrailers'` event is emitted on an `Http2Stream` object. The
+`'wantTrailers'` event will only be emitted if the `waitForTrailers` option
+is set for the `Http2Stream`.
+
 <a id="ERR_HTTP2_UNSUPPORTED_PROTOCOL"></a>
 ### ERR_HTTP2_UNSUPPORTED_PROTOCOL
 
diff --git a/doc/api/http2.md b/doc/api/http2.md
index ea5449733b0..47b5bb8af81 100644
--- a/doc/api/http2.md
+++ b/doc/api/http2.md
@@ -176,13 +176,13 @@ immediately following the `'frameError'` event.
 added: v8.4.0
 -->
 
-The `'goaway'` event is emitted when a GOAWAY frame is received. When invoked,
+The `'goaway'` event is emitted when a `GOAWAY` frame is received. When invoked,
 the handler function will receive three arguments:
 
-* `errorCode` {number} The HTTP/2 error code specified in the GOAWAY frame.
+* `errorCode` {number} The HTTP/2 error code specified in the `GOAWAY` frame.
 * `lastStreamID` {number} The ID of the last stream the remote peer successfully
   processed (or `0` if no ID is specified).
-* `opaqueData` {Buffer} If additional opaque data was included in the GOAWAY
+* `opaqueData` {Buffer} If additional opaque data was included in the `GOAWAY`
   frame, a `Buffer` instance will be passed containing that data.
 
 *Note*: The `Http2Session` instance will be shut down automatically when the
@@ -193,7 +193,7 @@ the handler function will receive three arguments:
 added: v8.4.0
 -->
 
-The `'localSettings'` event is emitted when an acknowledgment SETTINGS frame
+The `'localSettings'` event is emitted when an acknowledgment `SETTINGS` frame
 has been received. When invoked, the handler function will receive a copy of
 the local settings.
 
@@ -214,7 +214,7 @@ session.on('localSettings', (settings) => {
 added: v8.4.0
 -->
 
-The `'remoteSettings'` event is emitted when a new SETTINGS frame is received
+The `'remoteSettings'` event is emitted when a new `SETTINGS` frame is received
 from the connected peer. When invoked, the handler function will receive a copy
 of the remote settings.
 
@@ -385,7 +385,7 @@ added: v8.11.2
 * `code` {number} An HTTP/2 error code
 * `lastStreamID` {number} The numeric ID of the last processed `Http2Stream`
 * `opaqueData` {Buffer|TypedArray|DataView} A `TypedArray` or `DataView`
-  instance containing additional data to be carried within the GOAWAY frame.
+  instance containing additional data to be carried within the `GOAWAY` frame.
 
 Transmits a `GOAWAY` frame to the connected peer *without* shutting down the
 `Http2Session`.
@@ -419,7 +419,7 @@ added: v8.4.0
 * Value: {boolean}
 
 Indicates whether or not the `Http2Session` is currently waiting for an
-acknowledgment for a sent SETTINGS frame. Will be `true` after calling the
+acknowledgment for a sent `SETTINGS` frame. Will be `true` after calling the
 `http2session.settings()` method. Will be `false` once all sent SETTINGS
 frames have been acknowledged.
 
@@ -554,9 +554,9 @@ Once called, the `http2session.pendingSettingsAck` property will be `true`
 while the session is waiting for the remote peer to acknowledge the new
 settings.
 
-*Note*: The new settings will not become effective until the SETTINGS
+*Note*: The new settings will not become effective until the `SETTINGS`
 acknowledgment is received and the `'localSettings'` event is emitted. It
-is possible to send multiple SETTINGS frames while acknowledgment is still
+is possible to send multiple `SETTINGS` frames while acknowledgment is still
 pending.
 
 #### http2session.type
@@ -699,8 +699,8 @@ added: v8.4.0
   * `weight` {number} Specifies the relative dependency of a stream in relation
     to other streams with the same `parent`. The value is a number between `1`
     and `256` (inclusive).
-  * `getTrailers` {Function} Callback function invoked to collect trailer
-    headers.
+  * `waitForTrailers` {boolean} When `true`, the `Http2Stream` will emit the
+    `'wantTrailers'` event after the final `DATA` frame has been sent.
 
 * Returns: {ClientHttp2Stream}
 
@@ -727,15 +727,15 @@ req.on('response', (headers) => {
 });
 ```
 
-When set, the `options.getTrailers()` function is called immediately after
-queuing the last chunk of payload data to be sent. The callback is passed a
-single object (with a `null` prototype) that the listener may use to specify
-the trailing header fields to send to the peer.
+When the `options.waitForTrailers` option is set, the `'wantTrailers'` event
+is emitted immediately after queuing the last chunk of payload data to be sent.
+The `http2stream.sendTrailers()` method can then be called to send trailing
+headers to the peer.
 
-*Note*: The HTTP/1 specification forbids trailers from containing HTTP/2
-pseudo-header fields (e.g. `':method'`, `':path'`, etc). An `'error'` event
-will be emitted if the `getTrailers` callback attempts to set such header
-fields.
+It is important to note that when `options.waitForTrailers` is set, the
+`Http2Stream` will *not* automatically close when the final `DATA` frame is
+transmitted. User code *must* call either `http2stream.sendTrailers()` or
+`http2stream.close()` to close the `Http2Stream`.
 
 The `:method` and `:path` pseudo-headers are not specified within `headers`,
 they respectively default to:
@@ -881,6 +881,16 @@ stream.on('trailers', (headers, flags) => {
 });
 ```
 
+#### Event: 'wantTrailers'
+<!-- YAML
+added: REPLACEME
+-->
+
+The `'wantTrailers'` event is emitted when the `Http2Stream` has queued the
+final `DATA` frame to be sent on a frame and the `Http2Stream` is ready to send
+trailing headers. When initiating a request or response, the `waitForTrailers`
+option must be set for this event to be emitted.
+
 #### http2stream.aborted
 <!-- YAML
 added: v8.4.0
@@ -1046,6 +1056,35 @@ Provides miscellaneous information about the current state of the
 
 A current state of this `Http2Stream`.
 
+#### http2stream.sendTrailers(headers)
+<!-- YAML
+added: REPLACEME
+-->
+
+* `headers` {HTTP/2 Headers Object}
+
+Sends a trailing `HEADERS` frame to the connected HTTP/2 peer. This method
+will cause the `Http2Stream` to be immediately closed and must only be
+called after the `'wantTrailers'`  event has been emitted. When sending a
+request or sending a response, the `options.waitForTrailers` option must be set
+in order to keep the `Http2Stream` open after the final `DATA` frame so that
+trailers can be sent.
+
+```js
+const http2 = require('http2');
+const server = http2.createServer();
+server.on('stream', (stream) => {
+  stream.respond(undefined, { waitForTrailers: true });
+  stream.on('wantTrailers', () => {
+    stream.sendTrailers({ xyz: 'abc' });
+  });
+  stream.end('Hello World');
+});
+```
+
+The HTTP/1 specification forbids trailers from containing HTTP/2 pseudo-header
+fields (e.g. `':method'`, `':path'`, etc).
+
 ### Class: ClientHttp2Stream
 <!-- YAML
 added: v8.4.0
@@ -1213,9 +1252,8 @@ added: v8.4.0
 * `options` {Object}
   * `endStream` {boolean} Set to `true` to indicate that the response will not
     include payload data.
-  * `getTrailers` {Function} Callback function invoked to collect trailer
-    headers.
-* Returns: {undefined}
+  * `waitForTrailers` {boolean} When `true`, the `Http2Stream` will emit the
+    `'wantTrailers'` event after the final `DATA` frame has been sent.
 
 ```js
 const http2 = require('http2');
@@ -1226,29 +1264,28 @@ server.on('stream', (stream) => {
 });
 ```
 
-When set, the `options.getTrailers()` function is called immediately after
-queuing the last chunk of payload data to be sent. The callback is passed a
-single object (with a `null` prototype) that the listener may use to specify
-the trailing header fields to send to the peer.
+When the `options.waitForTrailers` option is set, the `'wantTrailers'` event
+will be emitted immediately after queuing the last chunk of payload data to be
+sent. The `http2stream.sendTrailers()` method can then be used to sent trailing
+header fields to the peer.
+
+It is important to note that when `options.waitForTrailers` is set, the
+`Http2Stream` will *not* automatically close when the final `DATA` frame is
+transmitted. User code *must* call either `http2stream.sendTrailers()` or
+`http2stream.close()` to close the `Http2Stream`.
 
 ```js
 const http2 = require('http2');
 const server = http2.createServer();
 server.on('stream', (stream) => {
-  stream.respond({ ':status': 200 }, {
-    getTrailers(trailers) {
-      trailers.ABC = 'some value to send';
-    }
+  stream.respond({ ':status': 200 }, { waitForTrailers: true });
+  stream.on('wantTrailers', () => {
+    stream.sendTrailers({ ABC: 'some value to send' });
   });
   stream.end('some data');
 });
 ```
 
-*Note*: The HTTP/1 specification forbids trailers from containing HTTP/2
-pseudo-header fields (e.g. `':status'`, `':path'`, etc). An `'error'` event
-will be emitted if the `getTrailers` callback attempts to set such header
-fields.
-
 #### http2stream.respondWithFD(fd[, headers[, options]])
 <!-- YAML
 added: v8.4.0
@@ -1258,8 +1295,8 @@ added: v8.4.0
 * `headers` {HTTP/2 Headers Object}
 * `options` {Object}
   * `statCheck` {Function}
-  * `getTrailers` {Function} Callback function invoked to collect trailer
-    headers.
+  * `waitForTrailers` {boolean} When `true`, the `Http2Stream` will emit the
+    `'wantTrailers'` event after the final `DATA` frame has been sent.
   * `offset` {number} The offset position at which to begin reading.
   * `length` {number} The amount of data from the fd to send.
 
@@ -1306,10 +1343,15 @@ Note that using the same file descriptor concurrently for multiple streams
 is not supported and may result in data loss. Re-using a file descriptor
 after a stream has finished is supported.
 
-When set, the `options.getTrailers()` function is called immediately after
-queuing the last chunk of payload data to be sent. The callback is passed a
-single object (with a `null` prototype) that the listener may use to specify
-the trailing header fields to send to the peer.
+When the `options.waitForTrailers` option is set, the `'wantTrailers'` event
+will be emitted immediately after queuing the last chunk of payload data to be
+sent. The `http2stream.sendTrailers()` method can then be used to sent trailing
+header fields to the peer.
+
+It is important to note that when `options.waitForTrailers` is set, the
+`Http2Stream` will *not* automatically close when the final `DATA` frame is
+transmitted. User code *must* call either `http2stream.sendTrailers()` or
+`http2stream.close()` to close the `Http2Stream`.
 
 ```js
 const http2 = require('http2');
@@ -1325,21 +1367,15 @@ server.on('stream', (stream) => {
     'last-modified': stat.mtime.toUTCString(),
     'content-type': 'text/plain'
   };
-  stream.respondWithFD(fd, headers, {
-    getTrailers(trailers) {
-      trailers.ABC = 'some value to send';
-    }
+  stream.respondWithFD(fd, headers, { waitForTrailers: true });
+  stream.on('wantTrailers', () => {
+    stream.sendTrailers({ ABC: 'some value to send' });
   });
 
   stream.on('close', () => fs.closeSync(fd));
 });
 ```
 
-*Note*: The HTTP/1 specification forbids trailers from containing HTTP/2
-pseudo-header fields (e.g. `':status'`, `':path'`, etc). An `'error'` event
-will be emitted if the `getTrailers` callback attempts to set such header
-fields.
-
 #### http2stream.respondWithFile(path[, headers[, options]])
 <!-- YAML
 added: v8.4.0
@@ -1351,8 +1387,8 @@ added: v8.4.0
   * `statCheck` {Function}
   * `onError` {Function} Callback function invoked in the case of an
     Error before send.
-  * `getTrailers` {Function} Callback function invoked to collect trailer
-    headers.
+  * `waitForTrailers` {boolean} When `true`, the `Http2Stream` will emit the
+    `'wantTrailers'` event after the final `DATA` frame has been sent.
   * `offset` {number} The offset position at which to begin reading.
   * `length` {number} The amount of data from the fd to send.
 
@@ -1426,29 +1462,29 @@ The `options.onError` function may also be used to handle all the errors
 that could happen before the delivery of the file is initiated. The
 default behavior is to destroy the stream.
 
-When set, the `options.getTrailers()` function is called immediately after
-queuing the last chunk of payload data to be sent. The callback is passed a
-single object (with a `null` prototype) that the listener may use to specify
-the trailing header fields to send to the peer.
+When the `options.waitForTrailers` option is set, the `'wantTrailers'` event
+will be emitted immediately after queuing the last chunk of payload data to be
+sent. The `http2stream.sendTrilers()` method can then be used to sent trailing
+header fields to the peer.
+
+It is important to note that when `options.waitForTrailers` is set, the
+`Http2Stream` will *not* automatically close when the final `DATA` frame is
+transmitted. User code *must* call either `http2stream.sendTrailers()` or
+`http2stream.close()` to close the `Http2Stream`.
 
 ```js
 const http2 = require('http2');
 const server = http2.createServer();
 server.on('stream', (stream) => {
-  function getTrailers(trailers) {
-    trailers.ABC = 'some value to send';
-  }
   stream.respondWithFile('/some/file',
                          { 'content-type': 'text/plain' },
-                         { getTrailers });
+                         { waitForTrailers: true });
+  stream.on('wantTrailers', () => {
+    stream.sendTrailers({ ABC: 'some value to send' });
+  });
 });
 ```
 
-*Note*: The HTTP/1 specification forbids trailers from containing HTTP/2
-pseudo-header fields (e.g. `':status'`, `':path'`, etc). An `'error'` event
-will be emitted if the `getTrailers` callback attempts to set such header
-fields.
-
 ### Class: Http2Server
 <!-- YAML
 added: v8.4.0
@@ -1715,7 +1751,7 @@ changes:
     limit to be exceeded, but new `Http2Stream` instances will be rejected
     while this limit is exceeded. The current number of `Http2Stream` sessions,
     the current memory use of the header compression tables, current data
-    queued to be sent, and unacknowledged PING and SETTINGS frames are all
+    queued to be sent, and unacknowledged `PING` and `SETTINGS` frames are all
     counted towards the current limit. **Default:** `10`.
   * `maxHeaderListPairs` {number} Sets the maximum number of header entries.
     The minimum value is `4`. **Default:** `128`.
@@ -1726,7 +1762,7 @@ changes:
     exceed this limit will result in a `'frameError'` event being emitted
     and the stream being closed and destroyed.
   * `paddingStrategy` {number} Identifies the strategy used for determining the
-     amount of padding to use for HEADERS and DATA frames. **Default:**
+     amount of padding to use for `HEADERS` and `DATA` frames. **Default:**
      `http2.constants.PADDING_STRATEGY_NONE`. Value may be one of:
      * `http2.constants.PADDING_STRATEGY_NONE` - Specifies that no padding is
        to be applied.
@@ -1744,7 +1780,7 @@ changes:
        calculated amount needed to ensure alignment, the maximum will be used
        and the total frame length will *not* necessarily be aligned at 8 bytes.
   * `peerMaxConcurrentStreams` {number} Sets the maximum number of concurrent
-    streams for the remote peer as if a SETTINGS frame had been received. Will
+    streams for the remote peer as if a `SETTINGS` frame had been received. Will
     be overridden if the remote peer sets its own value for
     `maxConcurrentStreams`. **Default:** `100`.
   * `selectPadding` {Function} When `options.paddingStrategy` is equal to
@@ -1825,7 +1861,7 @@ changes:
     limit to be exceeded, but new `Http2Stream` instances will be rejected
     while this limit is exceeded. The current number of `Http2Stream` sessions,
     the current memory use of the header compression tables, current data
-    queued to be sent, and unacknowledged PING and SETTINGS frames are all
+    queued to be sent, and unacknowledged `PING` and `SETTINGS` frames are all
     counted towards the current limit. **Default:** `10`.
   * `maxHeaderListPairs` {number} Sets the maximum number of header entries.
     The minimum value is `4`. **Default:** `128`.
@@ -1836,7 +1872,7 @@ changes:
     exceed this limit will result in a `'frameError'` event being emitted
     and the stream being closed and destroyed.
   * `paddingStrategy` {number} Identifies the strategy used for determining the
-     amount of padding to use for HEADERS and DATA frames. **Default:**
+     amount of padding to use for `HEADERS` and `DATA` frames. **Default:**
      `http2.constants.PADDING_STRATEGY_NONE`. Value may be one of:
      * `http2.constants.PADDING_STRATEGY_NONE` - Specifies that no padding is
        to be applied.
@@ -1854,7 +1890,7 @@ changes:
        calculated amount needed to ensure alignment, the maximum will be used
        and the total frame length will *not* necessarily be aligned at 8 bytes.
   * `peerMaxConcurrentStreams` {number} Sets the maximum number of concurrent
-    streams for the remote peer as if a SETTINGS frame had been received. Will
+    streams for the remote peer as if a `SETTINGS` frame had been received. Will
     be overridden if the remote peer sets its own value for
     `maxConcurrentStreams`. **Default:** `100`.
   * `selectPadding` {Function} When `options.paddingStrategy` is equal to
@@ -1917,7 +1953,7 @@ changes:
     limit to be exceeded, but new `Http2Stream` instances will be rejected
     while this limit is exceeded. The current number of `Http2Stream` sessions,
     the current memory use of the header compression tables, current data
-    queued to be sent, and unacknowledged PING and SETTINGS frames are all
+    queued to be sent, and unacknowledged `PING` and `SETTINGS` frames are all
     counted towards the current limit. **Default:** `10`.
   * `maxHeaderListPairs` {number} Sets the maximum number of header entries.
     The minimum value is `1`. **Default:** `128`.
@@ -1932,7 +1968,7 @@ changes:
     exceed this limit will result in a `'frameError'` event being emitted
     and the stream being closed and destroyed.
   * `paddingStrategy` {number} Identifies the strategy used for determining the
-     amount of padding to use for HEADERS and DATA frames. **Default:**
+     amount of padding to use for `HEADERS` and `DATA` frames. **Default:**
      `http2.constants.PADDING_STRATEGY_NONE`. Value may be one of:
      * `http2.constants.PADDING_STRATEGY_NONE` - Specifies that no padding is
        to be applied.
@@ -1950,7 +1986,7 @@ changes:
        calculated amount needed to ensure alignment, the maximum will be used
        and the total frame length will *not* necessarily be aligned at 8 bytes.
   * `peerMaxConcurrentStreams` {number} Sets the maximum number of concurrent
-    streams for the remote peer as if a SETTINGS frame had been received. Will
+    streams for the remote peer as if a `SETTINGS` frame had been received. Will
     be overridden if the remote peer sets its own value for
     `maxConcurrentStreams`. **Default:** `100`.
   * `selectPadding` {Function} When `options.paddingStrategy` is equal to
@@ -2123,7 +2159,7 @@ All additional properties on the settings object are ignored.
 When `options.paddingStrategy` is equal to
 `http2.constants.PADDING_STRATEGY_CALLBACK`, the HTTP/2 implementation will
 consult the `options.selectPadding` callback function, if provided, to determine
-the specific amount of padding to use per HEADERS and DATA frame.
+the specific amount of padding to use per `HEADERS` and `DATA` frame.
 
 The `options.selectPadding` function receives two numeric arguments,
 `frameLen` and `maxFrameLen` and must return a number `N` such that
@@ -2140,7 +2176,7 @@ const server = http2.createServer({
 ```
 
 *Note*: The `options.selectPadding` function is invoked once for *every*
-HEADERS and DATA frame. This has a definite noticeable impact on
+`HEADERS` and `DATA` frame. This has a definite noticeable impact on
 performance.
 
 ### Error Handling
@@ -3101,9 +3137,9 @@ The `name` property of the `PerformanceEntry` will be equal to either
 If `name` is equal to `Http2Stream`, the `PerformanceEntry` will contain the
 following additional properties:
 
-* `bytesRead` {number} The number of DATA frame bytes received for this
+* `bytesRead` {number} The number of `DATA` frame bytes received for this
   `Http2Stream`.
-* `bytesWritten` {number} The number of DATA frame bytes sent for this
+* `bytesWritten` {number} The number of `DATA` frame bytes sent for this
   `Http2Stream`.
 * `id` {number} The identifier of the associated `Http2Stream`
 * `timeToFirstByte` {number} The number of milliseconds elapsed between the