Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http2: add tests for push stream error handling #15281

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1814,7 +1814,7 @@ class ServerHttp2Stream extends Http2Stream {
break;
case NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE:
err = new errors.Error('ERR_HTTP2_OUT_OF_STREAMS');
process.nextTick(() => this.emit('error', err));
process.nextTick(() => session.emit('error', err));
break;
case NGHTTP2_ERR_STREAM_CLOSED:
err = new errors.Error('ERR_HTTP2_STREAM_CLOSED');
Expand Down
57 changes: 57 additions & 0 deletions test/parallel/test-http2-server-push-stream-errors-args.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// Flags: --expose-http2
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const assert = require('assert');
const http2 = require('http2');

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a quick description of the test here?

// Check that pushStream handles being passed wrong arguments
// in the expected manner

const server = http2.createServer();
server.on('stream', common.mustCall((stream, headers) => {
const port = server.address().port;

// Must receive a callback (function)
common.expectsError(
() => stream.pushStream({
':scheme': 'http',
':path': '/foobar',
':authority': `localhost:${port}`,
}, {}, 'callback'),
{
code: 'ERR_INVALID_CALLBACK',
message: 'Callback must be a function'
}
);

// Must validate headers
common.expectsError(
() => stream.pushStream({ 'connection': 'test' }, {}, () => {}),
{
code: 'ERR_HTTP2_INVALID_CONNECTION_HEADERS',
message: 'HTTP/1 Connection specific headers are forbidden'
}
);

stream.end('test');
}));

server.listen(0, common.mustCall(() => {
const port = server.address().port;
const headers = { ':path': '/' };
const client = http2.connect(`http://localhost:${port}`);
const req = client.request(headers);
req.setEncoding('utf8');

let data = '';
req.on('data', common.mustCall((d) => data += d));
req.on('end', common.mustCall(() => {
assert.strictEqual(data, 'test');
server.close();
client.destroy();
}));
req.end();
}));
130 changes: 130 additions & 0 deletions test/parallel/test-http2-server-push-stream-errors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
// Flags: --expose-http2
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const http2 = require('http2');
const {
constants,
Http2Session,
nghttp2ErrorString
} = process.binding('http2');

// tests error handling within pushStream
// - NGHTTP2_ERR_NOMEM (should emit session error)
// - NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE (should emit session error)
// - NGHTTP2_ERR_STREAM_CLOSED (should emit stream error)
// - every other NGHTTP2 error from binding (should emit stream error)

const specificTestKeys = [
'NGHTTP2_ERR_NOMEM',
'NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE',
'NGHTTP2_ERR_STREAM_CLOSED'
];

const specificTests = [
{
ngError: constants.NGHTTP2_ERR_NOMEM,
error: {
code: 'ERR_OUTOFMEMORY',
type: Error,
message: 'Out of memory'
},
type: 'session'
},
{
ngError: constants.NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE,
error: {
code: 'ERR_HTTP2_OUT_OF_STREAMS',
type: Error,
message: 'No stream ID is available because ' +
'maximum stream ID has been reached'
},
type: 'session'
},
{
ngError: constants.NGHTTP2_ERR_STREAM_CLOSED,
error: {
code: 'ERR_HTTP2_STREAM_CLOSED',
type: Error,
message: 'The stream is already closed'
},
type: 'stream'
},
];

const genericTests = Object.getOwnPropertyNames(constants)
.filter((key) => (
key.indexOf('NGHTTP2_ERR') === 0 && specificTestKeys.indexOf(key) < 0
))
.map((key) => ({
ngError: constants[key],
error: {
code: 'ERR_HTTP2_ERROR',
type: Error,
message: nghttp2ErrorString(constants[key])
},
type: 'stream'
}));


const tests = specificTests.concat(genericTests);

let currentError;

// mock submitPushPromise because we only care about testing error handling
Http2Session.prototype.submitPushPromise = () => currentError.ngError;

const server = http2.createServer();
server.on('stream', common.mustCall((stream, headers) => {
const errorMustCall = common.expectsError(currentError.error);
const errorMustNotCall = common.mustNotCall(
`${currentError.error.code} should emit on ${currentError.type}`
);
console.log(currentError);

if (currentError.type === 'stream') {
stream.session.on('error', errorMustNotCall);
stream.on('error', errorMustCall);
stream.on('error', common.mustCall(() => {
stream.respond();
stream.end();
}));
} else {
stream.session.once('error', errorMustCall);
stream.on('error', errorMustNotCall);
}

stream.pushStream({}, () => {});
}, tests.length));

server.listen(0, common.mustCall(() => runTest(tests.shift())));

function runTest(test) {
const port = server.address().port;
const url = `http://localhost:${port}`;
const headers = {
':path': '/',
':method': 'POST',
':scheme': 'http',
':authority': `localhost:${port}`
};

const client = http2.connect(url);
const req = client.request(headers);

currentError = test;
req.resume();
req.end();

req.on('end', common.mustCall(() => {
client.destroy();

if (!tests.length) {
server.close();
} else {
runTest(tests.shift());
}
}));
}
54 changes: 54 additions & 0 deletions test/parallel/test-http2-server-push-stream-head.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Flags: --expose-http2
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const assert = require('assert');
const http2 = require('http2');

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a quick description of this test here?

// Check that pushStream handles method HEAD correctly
// - stream should end immediately (no body)

const server = http2.createServer();
server.on('stream', common.mustCall((stream, headers) => {
const port = server.address().port;
if (headers[':path'] === '/') {
stream.pushStream({
':scheme': 'http',
':method': 'HEAD',
':authority': `localhost:${port}`,
}, common.mustCall((push, headers) => {
assert.strictEqual(push._writableState.ended, true);
stream.end('test');
}));
}
stream.respond({
'content-type': 'text/html',
':status': 200
});
}));

server.listen(0, common.mustCall(() => {
const port = server.address().port;
const headers = { ':path': '/' };
const client = http2.connect(`http://localhost:${port}`);
const req = client.request(headers);
req.setEncoding('utf8');

client.on('stream', common.mustCall((stream, headers) => {
assert.strictEqual(headers[':scheme'], 'http');
assert.strictEqual(headers[':path'], '/');
assert.strictEqual(headers[':authority'], `localhost:${port}`);
}));

let data = '';

req.on('data', common.mustCall((d) => data += d));
req.on('end', common.mustCall(() => {
assert.strictEqual(data, 'test');
server.close();
client.destroy();
}));
req.end();
}));
4 changes: 2 additions & 2 deletions test/parallel/test-http2-server-push-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ server.on('stream', common.mustCall((stream, headers) => {
':scheme': 'http',
':path': '/foobar',
':authority': `localhost:${port}`,
}, (push, headers) => {
}, common.mustCall((push, headers) => {
push.respond({
'content-type': 'text/html',
':status': 200,
'x-push-data': 'pushed by server',
});
push.end('pushed by server data');
stream.end('test');
});
}));
}
stream.respond({
'content-type': 'text/html',
Expand Down