Skip to content

Commit ba9012d

Browse files
apapirovskijasnell
apapirovski
authored and
jasnell
committed
http2: add tests for push stream error handling
Add tests that cover errors for wrong arguments, as well as tests for error codes from nghttp2. Fix pushStream to emit NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE on session rather than stream. PR-URL: #15281 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 29fd88c commit ba9012d

File tree

5 files changed

+244
-3
lines changed

5 files changed

+244
-3
lines changed

‎lib/internal/http2/core.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1790,7 +1790,7 @@ class ServerHttp2Stream extends Http2Stream {
17901790
break;
17911791
case NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE:
17921792
err = new errors.Error('ERR_HTTP2_OUT_OF_STREAMS');
1793-
process.nextTick(() => this.emit('error', err));
1793+
process.nextTick(() => session.emit('error', err));
17941794
break;
17951795
case NGHTTP2_ERR_STREAM_CLOSED:
17961796
err = new errors.Error('ERR_HTTP2_STREAM_CLOSED');

‎test/parallel/test-http2-server-push-stream-errors-args.js

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Flags: --expose-http2
2+
'use strict';
3+
4+
const common = require('../common');
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
const assert = require('assert');
8+
const http2 = require('http2');
9+
10+
// Check that pushStream handles being passed wrong arguments
11+
// in the expected manner
12+
13+
const server = http2.createServer();
14+
server.on('stream', common.mustCall((stream, headers) => {
15+
const port = server.address().port;
16+
17+
// Must receive a callback (function)
18+
common.expectsError(
19+
() => stream.pushStream({
20+
':scheme': 'http',
21+
':path': '/foobar',
22+
':authority': `localhost:${port}`,
23+
}, {}, 'callback'),
24+
{
25+
code: 'ERR_INVALID_CALLBACK',
26+
message: 'callback must be a function'
27+
}
28+
);
29+
30+
// Must validate headers
31+
common.expectsError(
32+
() => stream.pushStream({ 'connection': 'test' }, {}, () => {}),
33+
{
34+
code: 'ERR_HTTP2_INVALID_CONNECTION_HEADERS',
35+
message: 'HTTP/1 Connection specific headers are forbidden'
36+
}
37+
);
38+
39+
stream.end('test');
40+
}));
41+
42+
server.listen(0, common.mustCall(() => {
43+
const port = server.address().port;
44+
const headers = { ':path': '/' };
45+
const client = http2.connect(`http://localhost:${port}`);
46+
const req = client.request(headers);
47+
req.setEncoding('utf8');
48+
49+
let data = '';
50+
req.on('data', common.mustCall((d) => data += d));
51+
req.on('end', common.mustCall(() => {
52+
assert.strictEqual(data, 'test');
53+
server.close();
54+
client.destroy();
55+
}));
56+
req.end();
57+
}));

‎test/parallel/test-http2-server-push-stream-errors.js

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// Flags: --expose-http2
2+
'use strict';
3+
4+
const common = require('../common');
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
const http2 = require('http2');
8+
const {
9+
constants,
10+
Http2Session,
11+
nghttp2ErrorString
12+
} = process.binding('http2');
13+
14+
// tests error handling within pushStream
15+
// - NGHTTP2_ERR_NOMEM (should emit session error)
16+
// - NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE (should emit session error)
17+
// - NGHTTP2_ERR_STREAM_CLOSED (should emit stream error)
18+
// - every other NGHTTP2 error from binding (should emit stream error)
19+
20+
const specificTestKeys = [
21+
'NGHTTP2_ERR_NOMEM',
22+
'NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE',
23+
'NGHTTP2_ERR_STREAM_CLOSED'
24+
];
25+
26+
const specificTests = [
27+
{
28+
ngError: constants.NGHTTP2_ERR_NOMEM,
29+
error: {
30+
code: 'ERR_OUTOFMEMORY',
31+
type: Error,
32+
message: 'Out of memory'
33+
},
34+
type: 'session'
35+
},
36+
{
37+
ngError: constants.NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE,
38+
error: {
39+
code: 'ERR_HTTP2_OUT_OF_STREAMS',
40+
type: Error,
41+
message: 'No stream ID is available because ' +
42+
'maximum stream ID has been reached'
43+
},
44+
type: 'session'
45+
},
46+
{
47+
ngError: constants.NGHTTP2_ERR_STREAM_CLOSED,
48+
error: {
49+
code: 'ERR_HTTP2_STREAM_CLOSED',
50+
type: Error,
51+
message: 'The stream is already closed'
52+
},
53+
type: 'stream'
54+
},
55+
];
56+
57+
const genericTests = Object.getOwnPropertyNames(constants)
58+
.filter((key) => (
59+
key.indexOf('NGHTTP2_ERR') === 0 && specificTestKeys.indexOf(key) < 0
60+
))
61+
.map((key) => ({
62+
ngError: constants[key],
63+
error: {
64+
code: 'ERR_HTTP2_ERROR',
65+
type: Error,
66+
message: nghttp2ErrorString(constants[key])
67+
},
68+
type: 'stream'
69+
}));
70+
71+
72+
const tests = specificTests.concat(genericTests);
73+
74+
let currentError;
75+
76+
// mock submitPushPromise because we only care about testing error handling
77+
Http2Session.prototype.submitPushPromise = () => currentError.ngError;
78+
79+
const server = http2.createServer();
80+
server.on('stream', common.mustCall((stream, headers) => {
81+
const errorMustCall = common.expectsError(currentError.error);
82+
const errorMustNotCall = common.mustNotCall(
83+
`${currentError.error.code} should emit on ${currentError.type}`
84+
);
85+
console.log(currentError);
86+
87+
if (currentError.type === 'stream') {
88+
stream.session.on('error', errorMustNotCall);
89+
stream.on('error', errorMustCall);
90+
stream.on('error', common.mustCall(() => {
91+
stream.respond();
92+
stream.end();
93+
}));
94+
} else {
95+
stream.session.once('error', errorMustCall);
96+
stream.on('error', errorMustNotCall);
97+
}
98+
99+
stream.pushStream({}, () => {});
100+
}, tests.length));
101+
102+
server.listen(0, common.mustCall(() => runTest(tests.shift())));
103+
104+
function runTest(test) {
105+
const port = server.address().port;
106+
const url = `http://localhost:${port}`;
107+
const headers = {
108+
':path': '/',
109+
':method': 'POST',
110+
':scheme': 'http',
111+
':authority': `localhost:${port}`
112+
};
113+
114+
const client = http2.connect(url);
115+
const req = client.request(headers);
116+
117+
currentError = test;
118+
req.resume();
119+
req.end();
120+
121+
req.on('end', common.mustCall(() => {
122+
client.destroy();
123+
124+
if (!tests.length) {
125+
server.close();
126+
} else {
127+
runTest(tests.shift());
128+
}
129+
}));
130+
}

‎test/parallel/test-http2-server-push-stream-head.js

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Flags: --expose-http2
2+
'use strict';
3+
4+
const common = require('../common');
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
const assert = require('assert');
8+
const http2 = require('http2');
9+
10+
// Check that pushStream handles method HEAD correctly
11+
// - stream should end immediately (no body)
12+
13+
const server = http2.createServer();
14+
server.on('stream', common.mustCall((stream, headers) => {
15+
const port = server.address().port;
16+
if (headers[':path'] === '/') {
17+
stream.pushStream({
18+
':scheme': 'http',
19+
':method': 'HEAD',
20+
':authority': `localhost:${port}`,
21+
}, common.mustCall((push, headers) => {
22+
assert.strictEqual(push._writableState.ended, true);
23+
stream.end('test');
24+
}));
25+
}
26+
stream.respond({
27+
'content-type': 'text/html',
28+
':status': 200
29+
});
30+
}));
31+
32+
server.listen(0, common.mustCall(() => {
33+
const port = server.address().port;
34+
const headers = { ':path': '/' };
35+
const client = http2.connect(`http://localhost:${port}`);
36+
const req = client.request(headers);
37+
req.setEncoding('utf8');
38+
39+
client.on('stream', common.mustCall((stream, headers) => {
40+
assert.strictEqual(headers[':scheme'], 'http');
41+
assert.strictEqual(headers[':path'], '/');
42+
assert.strictEqual(headers[':authority'], `localhost:${port}`);
43+
}));
44+
45+
let data = '';
46+
47+
req.on('data', common.mustCall((d) => data += d));
48+
req.on('end', common.mustCall(() => {
49+
assert.strictEqual(data, 'test');
50+
server.close();
51+
client.destroy();
52+
}));
53+
req.end();
54+
}));

‎test/parallel/test-http2-server-push-stream.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@ server.on('stream', common.mustCall((stream, headers) => {
1515
':scheme': 'http',
1616
':path': '/foobar',
1717
':authority': `localhost:${port}`,
18-
}, (push, headers) => {
18+
}, common.mustCall((push, headers) => {
1919
push.respond({
2020
'content-type': 'text/html',
2121
':status': 200,
2222
'x-push-data': 'pushed by server',
2323
});
2424
push.end('pushed by server data');
2525
stream.end('test');
26-
});
26+
}));
2727
}
2828
stream.respond({
2929
'content-type': 'text/html',

0 commit comments

Comments
 (0)