Skip to content

Some http2 errors are not captured #28895

Closed
Closed
Some http2 errors are not captured#28895
Labels
http2Issues and PRs related to the http2 subsystem.

Description

@szmarczak
szmarczak
opened on Jul 29, 2019
const http2 = require('http2');

try {
    const session = http2.connect('https://example.com', {
        settings: {
            maxFrameSize: 1
        }
    });
    
    session.on('error', error => {
        console.log('Captured an error.');
    });
} catch (error) {
    console.log('Captured an error.');
}

See it on RunKit.

Actual behavior

internal/http2/util.js:521
      throw new ERR_HTTP2_INVALID_SETTING_VALUE.RangeError(
      ^

RangeError [ERR_HTTP2_INVALID_SETTING_VALUE]: Invalid value for setting "maxFrameSize": 1
    at ClientHttp2Session.settings (internal/http2/core.js:1156:5)
    at ClientHttp2Session.setupHandle (internal/http2/core.js:861:8)
    at Object.onceWrapper (events.js:291:20)
    at TLSSocket.emit (events.js:203:13)
    at TLSSocket.onConnectSecure (_tls_wrap.js:1346:10)
    at TLSSocket.emit (events.js:203:13)
    at TLSSocket._finishInit (_tls_wrap.js:792:8)
    at TLSWrap.ssl.onhandshakedone (_tls_wrap.js:606:12) {
  actual: 1,
  min: 16384,
  max: 16777215
}

Expected behavior

Captured an error.

Possible solution

It's thrown from here:

node/lib/internal/http2/core.js

Line 975 in 98d8ed6

socket.once(connectEvent, setupFn);

I think it needs to be like:

      socket.once(connectEvent, () => {
        try {
          setupFn();
        } catch (error) {
          socket.destroy(error);
        }
      });

Activity

  1. himself65 commented on Jul 29, 2019

    @himself65
    himself65
    on Jul 29, 2019
    Member

    https://nodejs.org/dist/latest-v12.x/docs/api/http2.html#http2_settings_object

    maxFrameSize <number> Specifies the size of the largest frame payload. The minimum allowed value is 16,384. The maximum allowed value is 224-1. Default: 16,384 bytes.

  2. szmarczak commented on Jul 29, 2019

    @szmarczak
    szmarczak
    on Jul 29, 2019
    ContributorAuthor

    @himself65 I think you have misunderstood me, the error isn't captured at all. It's an unhandled error.

  3. lpinca
    added
    http2Issues and PRs related to the http2 subsystem.
    on Jul 29, 2019

  4. reklatsmasters commented on Jul 30, 2019

    @reklatsmasters
    reklatsmasters
    on Jul 30, 2019
    Contributor

    TLSWrap.ssl.onhandshakedone called in next tick and lost current context. More related example:

    const tls = require('tls');

try {
    const session = tls.connect(443, 'example.com', () => {
        throw new Error('Not captured');
    });

    session.on('error', error => {
        console.log('Captured an error', error);
    });
} catch (error) {
    console.log('Captured an error', error);
}

  5. lpinca commented on Aug 5, 2019

    @lpinca
    lpinca
    on Aug 5, 2019
    Member

    Fixed by #28908.

  6. lpinca
    closed this as completedon Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    http2Issues and PRs related to the http2 subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions