From 2ecdb6d54f8b43199831e7056323694507e7c764 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 11 Apr 2018 16:11:35 -0700 Subject: [PATCH] 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. PR-URL: https://github.com/nodejs/node/pull/19959 Reviewed-By: Yuta Hiroto Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina --- doc/api/errors.md | 13 ++ doc/api/http2.md | 191 +++++++++++------- lib/internal/errors.js | 5 + lib/internal/http2/compat.js | 7 +- lib/internal/http2/core.js | 85 ++++---- src/node_http2.cc | 100 ++++----- src/node_http2.h | 26 +-- ...est-http2-client-request-options-errors.js | 1 - ...est-http2-compat-serverrequest-trailers.js | 19 +- ...ompat-serverresponse-createpushresponse.js | 19 +- .../test-http2-misused-pseudoheaders.js | 23 +-- .../test-http2-no-wanttrailers-listener.js | 35 ++++ test/parallel/test-http2-respond-errors.js | 37 +--- .../test-http2-respond-file-errors.js | 3 +- .../test-http2-respond-file-fd-errors.js | 3 +- test/parallel/test-http2-sent-headers.js | 7 +- test/parallel/test-http2-trailers.js | 40 +++- 17 files changed, 329 insertions(+), 285 deletions(-) create mode 100644 test/parallel/test-http2-no-wanttrailers-listener.js diff --git a/doc/api/errors.md b/doc/api/errors.md index 00d30193df7d33..0ba9b992fd8e2e 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1017,6 +1017,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. + +### ERR_HTTP2_TRAILERS_ALREADY_SENT + +Trailing headers have already been sent on the `Http2Stream`. + + +### 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`. + ### ERR_HTTP2_UNSUPPORTED_PROTOCOL diff --git a/doc/api/http2.md b/doc/api/http2.md index dae97c0a855893..9a0b5595346307 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. The `Http2Session` instance will be shut down automatically when the `'goaway'` @@ -193,7 +193,7 @@ event is emitted. 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. @@ -213,7 +213,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. @@ -383,7 +383,7 @@ added: v9.4.0 * `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`. @@ -417,7 +417,7 @@ added: v8.4.0 * {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. @@ -551,9 +551,9 @@ Once called, the `http2session.pendingSettingsAck` property will be `true` while the session is waiting for the remote peer to acknowledge the new settings. -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 pending. +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 pending. #### http2session.type + +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 + +* `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