Skip to content

Commit

Permalink
http2: emit timeout on compat request and response
Browse files Browse the repository at this point in the history
v8.x Backport Note: The timeout has been increased to 10ms.

Fixes: nodejs#20079

PR-URL: nodejs#22252
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
  • Loading branch information
jasnell authored and kjin committed Oct 16, 2018
1 parent f1e310e commit 3dc84ca
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 2 deletions.
9 changes: 9 additions & 0 deletions lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,13 @@ function onStreamCloseRequest() {
req.emit('close');
}

function onStreamTimeout(kind) {
return function onStreamTimeout() {
const obj = this[kind];
obj.emit('timeout');
};
}

class Http2ServerRequest extends Readable {
constructor(stream, headers, options, rawHeaders) {
super(options);
Expand All @@ -251,6 +258,7 @@ class Http2ServerRequest extends Readable {
stream.on('error', onStreamError);
stream.on('aborted', onStreamAbortedRequest);
stream.on('close', onStreamCloseRequest);
stream.on('timeout', onStreamTimeout(kRequest));
this.on('pause', onRequestPause);
this.on('resume', onRequestResume);
}
Expand Down Expand Up @@ -403,6 +411,7 @@ class Http2ServerResponse extends Stream {
stream.on('aborted', onStreamAbortedResponse);
stream.on('close', onStreamCloseResponse);
stream.on('wantTrailers', onStreamTrailersReady);
stream.on('timeout', onStreamTimeout(kResponse));
}

// User land modules such as finalhandler just check truthiness of this
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-http2-compat-serverrequest-settimeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ if (!common.hasCrypto)
const assert = require('assert');
const http2 = require('http2');

const msecs = common.platformTimeout(1);
// Set the timeout to 10ms since ending the response stream resets the timer.
const msecs = common.platformTimeout(10);
const server = http2.createServer();

server.on('request', (req, res) => {
req.setTimeout(msecs, common.mustCall(() => {
res.end();
}));
res.on('timeout', common.mustCall());
res.on('finish', common.mustCall(() => {
req.setTimeout(msecs, common.mustNotCall());
process.nextTick(() => {
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-http2-compat-serverresponse-settimeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ if (!common.hasCrypto)
const assert = require('assert');
const http2 = require('http2');

const msecs = common.platformTimeout(1);
// Set the timeout to 10ms since ending the response stream resets the timer.
const msecs = common.platformTimeout(10);
const server = http2.createServer();

server.on('request', (req, res) => {
res.setTimeout(msecs, common.mustCall(() => {
res.end();
}));
res.on('timeout', common.mustCall());
res.on('finish', common.mustCall(() => {
res.setTimeout(msecs, common.mustNotCall());
process.nextTick(() => {
Expand Down

0 comments on commit 3dc84ca

Please sign in to comment.