Skip to content

Commit

Permalink
http2: use 'close' event instead of 'streamClosed'
Browse files Browse the repository at this point in the history
PR-URL: #17328
Fixes: #15303
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
  • Loading branch information
jasnell authored and MylesBorins committed Dec 8, 2017
1 parent bd035d7 commit aba3544
Show file tree
Hide file tree
Showing 25 changed files with 45 additions and 47 deletions.
26 changes: 13 additions & 13 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ All [`Http2Stream`][] instances are destroyed either when:
When an `Http2Stream` instance is destroyed, an attempt will be made to send an
`RST_STREAM` frame will be sent to the connected peer.

Once the `Http2Stream` instance is destroyed, the `'streamClosed'` event will
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
Expand All @@ -653,6 +653,18 @@ abnormally aborted in mid-communication.
*Note*: The `'aborted'` event will only be emitted if the `Http2Stream`
writable side has not been ended.

#### Event: 'close'
<!-- YAML
added: v8.4.0
-->

The `'close'` event is emitted when the `Http2Stream` is destroyed. Once
this event is emitted, the `Http2Stream` instance is no longer usable.

The listener callback is passed a single argument specifying the HTTP/2 error
code specified when closing the stream. If the code is any value other than
`NGHTTP2_NO_ERROR` (`0`), an `'error'` event will also be emitted.

#### Event: 'error'
<!-- YAML
added: v8.4.0
Expand All @@ -672,18 +684,6 @@ argument identifying the frame type, and an integer argument identifying the
error code. The `Http2Stream` instance will be destroyed immediately after the
`'frameError'` event is emitted.

#### Event: 'streamClosed'
<!-- YAML
added: v8.4.0
-->

The `'streamClosed'` event is emitted when the `Http2Stream` is destroyed. Once
this event is emitted, the `Http2Stream` instance is no longer usable.

The listener callback is passed a single argument specifying the HTTP/2 error
code specified when closing the stream. If the code is any value other than
`NGHTTP2_NO_ERROR` (`0`), an `'error'` event will also be emitted.

#### Event: 'timeout'
<!-- YAML
added: v8.4.0
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ class Http2ServerRequest extends Readable {
stream.on('close', onStreamClosedRequest);
stream.on('aborted', onStreamAbortedRequest);
const onfinish = this[kFinish].bind(this);
stream.on('streamClosed', onfinish);
stream.on('close', onfinish);
stream.on('finish', onfinish);
this.on('pause', onRequestPause);
this.on('resume', onRequestResume);
Expand Down Expand Up @@ -383,7 +383,7 @@ class Http2ServerResponse extends Stream {
stream.on('close', onStreamClosedResponse);
stream.on('aborted', onStreamAbortedResponse);
const onfinish = this[kFinish].bind(this);
stream.on('streamClosed', onfinish);
stream.on('close', onfinish);
stream.on('finish', onfinish);
}

Expand Down
8 changes: 3 additions & 5 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,8 @@ function onStreamTrailers() {
return headersList;
}

// Called when the stream is closed. The streamClosed event is emitted on the
// Http2Stream instance. Note that this event is distinctly different than the
// require('stream') interface 'close' event which deals with the state of the
// Readable and Writable sides of the Duplex.
// Called when the stream is closed. The close event is emitted on the
// Http2Stream instance
function onStreamClose(code) {
const stream = this[kOwner];
stream[kUpdateTimer]();
Expand Down Expand Up @@ -1473,7 +1471,7 @@ function continueStreamDestroy(err, callback) {
abort(this);
this.push(null); // Close the readable side
this.end(); // Close the writable side
process.nextTick(emit, this, 'streamClosed', code);
process.nextTick(emit, this, 'close', code);
}

function finishStreamDestroy() {
Expand Down
2 changes: 1 addition & 1 deletion test/known_issues/test-http2-client-http1-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);

const req = client.request();
req.on('streamClosed', common.mustCall());
req.on('close', common.mustCall());

client.on('error', common.expectsError({
code: 'ERR_HTTP2_ERROR',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ server.on('listening', common.mustCall(() => {
// second call doesn't do anything
assert.doesNotThrow(() => req.rstStream(8));

req.on('streamClosed', common.mustCall((code) => {
req.on('close', common.mustCall((code) => {
assert.strictEqual(req.destroyed, true);
assert.strictEqual(code, 0);
server.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ server.on('listening', common.mustCall(() => {
})(err);
}));

req.on('streamClosed', common.mustCall((code) => {
req.on('close', common.mustCall((code) => {
assert.strictEqual(req.rstCode, NGHTTP2_INTERNAL_ERROR);
assert.strictEqual(code, NGHTTP2_INTERNAL_ERROR);
server.close();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-client-unescaped-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ server.listen(0, common.mustCall(() => {
type: Error,
message: 'Stream closed with error code 1'
}));
req.on('streamClosed', common.mustCall(maybeClose));
req.on('close', common.mustCall(maybeClose));
}

for (let i = 0; i <= count; i += 1)
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http2-compat-serverresponse-end.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,10 @@ const {


{
// Should be able to call .end with cb from stream 'streamClosed'
// Should be able to call .end with cb from stream 'close'
const server = createServer(mustCall((request, response) => {
response.writeHead(HTTP_STATUS_OK, { foo: 'bar' });
response.stream.on('streamClosed', mustCall(() => {
response.stream.on('close', mustCall(() => {
response.end(mustCall());
}));
}));
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http2-compat-socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ server.on('request', common.mustCall(function(request, response) {
assert.strictEqual(request.socket.connecting, false);

// socket events are bound and emitted on Http2Stream
request.socket.on('streamClosed', common.mustCall());
request.socket.once('streamClosed', common.mustCall());
request.socket.on('close', common.mustCall());
request.socket.once('close', common.mustCall());
request.socket.on('testEvent', common.mustCall());
request.socket.emit('testEvent');
}));
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-multiheaders-raw.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ server.on('stream', common.mustCall((stream, headers, flags, rawHeaders) => {
server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);
const req = client.request(src);
req.on('streamClosed', common.mustCall(() => {
req.on('close', common.mustCall(() => {
server.close();
client.destroy();
}));
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-multiheaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);
const req = client.request(src);
req.on('response', common.mustCall(checkHeaders));
req.on('streamClosed', common.mustCall(() => {
req.on('close', common.mustCall(() => {
server.close();
client.destroy();
}));
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-http2-options-max-reserved-streams.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ server.on('stream', common.mustCall((stream) => {
pushedStream.respond({ ':status': 200 });
pushedStream.on('aborted', common.mustCall());
pushedStream.on('error', common.mustNotCall());
pushedStream.on('streamClosed',
pushedStream.on('close',
common.mustCall((code) => assert.strictEqual(code, 8)));
}));

Expand Down Expand Up @@ -67,12 +67,12 @@ server.on('listening', common.mustCall(() => {
client.on('stream', common.mustCall((stream) => {
stream.resume();
stream.on('end', common.mustCall());
stream.on('streamClosed', common.mustCall(maybeClose));
stream.on('close', common.mustCall(maybeClose));
}));

req.on('response', common.mustCall());

req.resume();
req.on('end', common.mustCall());
req.on('streamClosed', common.mustCall(maybeClose));
req.on('close', common.mustCall(maybeClose));
}));
2 changes: 1 addition & 1 deletion test/parallel/test-http2-respond-file-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);
const req = client.request();

req.on('streamClosed', common.mustCall(() => {
req.on('close', common.mustCall(() => {
client.destroy();
server.close();
}));
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-respond-file-fd-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);
const req = client.request();

req.on('streamClosed', common.mustCall(() => {
req.on('close', common.mustCall(() => {
client.destroy();
server.close();
}));
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http2-respond-file-fd-range.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ server.listen(0, () => {
req.on('end', common.mustCall(() => {
assert.strictEqual(check, data.toString('utf8', 8, 11));
}));
req.on('streamClosed', common.mustCall(maybeClose));
req.on('close', common.mustCall(maybeClose));
req.end();
}

Expand All @@ -88,7 +88,7 @@ server.listen(0, () => {
req.on('end', common.mustCall(() => {
assert.strictEqual(check, data.toString('utf8', 8, 28));
}));
req.on('streamClosed', common.mustCall(maybeClose));
req.on('close', common.mustCall(maybeClose));
req.end();
}

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-respond-no-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const server = http2.createServer();
const status = [204, 205, 304];

server.on('stream', common.mustCall((stream) => {
stream.on('streamClosed', common.mustCall(() => {
stream.on('close', common.mustCall(() => {
assert.strictEqual(stream.destroyed, true);
}));
stream.respond({ ':status': status.shift() });
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-response-splitting.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ server.listen(0, common.mustCall(() => {
}));
req.resume();
req.on('end', common.mustCall());
req.on('streamClosed', common.mustCall(maybeClose));
req.on('close', common.mustCall(maybeClose));
}

doTest(str, 'location', str);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-server-rst-before-respond.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ server.on('listening', common.mustCall(() => {

req.on('headers', common.mustNotCall());

req.on('streamClosed', common.mustCall((code) => {
req.on('close', common.mustCall((code) => {
assert.strictEqual(h2.constants.NGHTTP2_NO_ERROR, code);
server.close();
client.destroy();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-server-rst-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ server.listen(0, common.mustCall(() => {
':method': 'POST',
rstmethod: test[0]
});
req.on('streamClosed', common.mustCall((code) => {
req.on('close', common.mustCall((code) => {
assert.strictEqual(code, test[1]);
countdown.dec();
}));
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-server-socketerror.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ server.listen(0, common.mustCall(() => {
const req = client.request();
req.resume();
req.on('end', common.mustCall());
req.on('streamClosed', common.mustCall(() => {
req.on('close', common.mustCall(() => {
client.destroy();
server.close();
}));
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-stream-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);
const req = client.request();
req.resume();
req.on('streamClosed', common.mustCall(() => {
req.on('close', common.mustCall(() => {
client.destroy();
server.close();
}));
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-stream-destroy-event-order.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ let req;
const server = http2.createServer();
server.on('stream', common.mustCall((stream) => {
stream.on('error', common.mustCall(() => {
stream.on('streamClosed', common.mustCall((code) => {
stream.on('close', common.mustCall((code) => {
assert.strictEqual(code, 2);
client.destroy();
server.close();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-too-large-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ server.listen(0, common.mustCall(() => {
type: Error,
message: 'Stream closed with error code 11'
}));
req.on('streamClosed', common.mustCall((code) => {
req.on('close', common.mustCall((code) => {
assert.strictEqual(code, NGHTTP2_ENHANCE_YOUR_CALM);
server.close();
client.destroy();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-too-many-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ server.listen(0, common.mustCall(() => {
type: Error,
message: 'Stream closed with error code 11'
}));
req.on('streamClosed', common.mustCall((code) => {
req.on('close', common.mustCall((code) => {
assert.strictEqual(code, NGHTTP2_ENHANCE_YOUR_CALM);
server.close();
client.destroy();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-write-callbacks.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ server.listen(0, common.mustCall(() => {
req.setEncoding('utf8');
req.on('data', (chunk) => actual += chunk);
req.on('end', common.mustCall(() => assert.strictEqual(actual, 'abcxyz')));
req.on('streamClosed', common.mustCall(() => {
req.on('close', common.mustCall(() => {
client.destroy();
server.close();
}));
Expand Down

0 comments on commit aba3544

Please sign in to comment.