8000 http2: add tests for push stream error handling · nodejs/node@ba9012d · GitHub
[go: up one dir, main page]
Skip to content
8000

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
  • test-http2-server-push-stream-errors.js
  • test-http2-server-push-stream-head.js
  • test-http2-server-push-stream.js

    • 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)
    0