Skip to content

test: add ALPNProtocols option to clientOptions#35482

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.

Sign up for GitHub

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

Jump to bottom
Merged
lpinca merged 0 commits into from
Oct 9, 2020
Merged

test: add ALPNProtocols option to clientOptions #35482

lpinca merged 0 commits into from
Oct 9, 2020

Conversation

@lpinca
Copy link
Member

@lpincalpinca commented Oct 3, 2020

Without this, the session is destroyed with the following error

Error [ERR_HTTP2_ERROR]: Protocol error at Http2Session.onSessionInternalError (internal/http2/core.js:756:26) Emitted 'error' event on ClientHttp2Session instance at: at emitClose (internal/http2/core.js:1010:10) at internal/http2/core.js:1048:7 at finish (internal/streams/writable.js:731:5) at processTicksAndRejections (internal/process/task_queues.js:80:21){code: 'ERR_HTTP2_ERROR', errno: -505 }

The test then calls session.close() which tries to write to a
destroyed socket. As a result, an unhandled ECONNRESET error is
emitted in the v12 release line.

Refs: #34859

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Oct 3, 2020
@lpinca
Copy link
MemberAuthor

lpinca commented Oct 3, 2020

This should eventually be backported to v12 before #34859.

@addaleax
Copy link
Member

addaleax commented Oct 3, 2020

@lpinca I think you can feel free to just push the commit from here to #34859 once this lands, at least I don’t consider test changes to be subject to the 2-week waiting policy for LTS

@nodejsnodejs deleted a comment Oct 3, 2020
@addaleaxaddaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 5, 2020
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 5, 2020
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Oct 9, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/33484/

@lpincalpinca closed this Oct 9, 2020
@lpincalpincaforce-pushed the add/alpn-protocols-option branch from e863a38 to b474901CompareOctober 9, 2020 14:29
@lpinca
Copy link
MemberAuthor

lpinca commented Oct 9, 2020

Landed in b474901.

@lpincalpinca merged commit b474901 into nodejs:masterOct 9, 2020
@lpincalpinca deleted the add/alpn-protocols-option branch October 9, 2020 14:29
lpinca added a commit to addaleax/node that referenced this pull request Oct 9, 2020
@lpinca
test: add ALPNProtocols option to clientOptions
e221cbe
Without this, the session is destroyed with the following error ``` Error [ERR_HTTP2_ERROR]: Protocol error at Http2Session.onSessionInternalError (internal/http2/core.js:756:26) Emitted 'error' event on ClientHttp2Session instance at: at emitClose (internal/http2/core.js:1010:10) at internal/http2/core.js:1048:7 at finish (internal/streams/writable.js:731:5) at processTicksAndRejections (internal/process/task_queues.js:80:21){code: 'ERR_HTTP2_ERROR', errno: -505 } ``` The test then calls `session.close()` which tries to write to a destroyed socket. As a result, an unhandled `ECONNRESET` error is emitted in the v12 release line. PR-URL: nodejs#35482 Refs: nodejs#34859 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
@lpinca
Copy link
MemberAuthor

lpinca commented Oct 9, 2020

I've cherry-picked b474901 into #34859.

BethGriggs pushed a commit that referenced this pull request Oct 14, 2020
@lpinca@BethGriggs
test: add ALPNProtocols option to clientOptions
7685297
Without this, the session is destroyed with the following error ``` Error [ERR_HTTP2_ERROR]: Protocol error at Http2Session.onSessionInternalError (internal/http2/core.js:756:26) Emitted 'error' event on ClientHttp2Session instance at: at emitClose (internal/http2/core.js:1010:10) at internal/http2/core.js:1048:7 at finish (internal/streams/writable.js:731:5) at processTicksAndRejections (internal/process/task_queues.js:80:21){code: 'ERR_HTTP2_ERROR', errno: -505 } ``` The test then calls `session.close()` which tries to write to a destroyed socket. As a result, an unhandled `ECONNRESET` error is emitted in the v12 release line. PR-URL: #35482 Refs: #34859 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Oct 14, 2020
Merged
MylesBorins pushed a commit that referenced this pull request Nov 3, 2020
@lpinca@MylesBorins
test: add ALPNProtocols option to clientOptions
9ab53b4
Without this, the session is destroyed with the following error ``` Error [ERR_HTTP2_ERROR]: Protocol error at Http2Session.onSessionInternalError (internal/http2/core.js:756:26) Emitted 'error' event on ClientHttp2Session instance at: at emitClose (internal/http2/core.js:1010:10) at internal/http2/core.js:1048:7 at finish (internal/streams/writable.js:731:5) at processTicksAndRejections (internal/process/task_queues.js:80:21){code: 'ERR_HTTP2_ERROR', errno: -505 } ``` The test then calls `session.close()` which tries to write to a destroyed socket. As a result, an unhandled `ECONNRESET` error is emitted in the v12 release line. Backport-PR-URL: #34859 PR-URL: #35482 Refs: #34859 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Nov 3, 2020
Merged
MylesBorins pushed a commit that referenced this pull request Nov 16, 2020
@lpinca@MylesBorins
test: add ALPNProtocols option to clientOptions
02787ce
Without this, the session is destroyed with the following error ``` Error [ERR_HTTP2_ERROR]: Protocol error at Http2Session.onSessionInternalError (internal/http2/core.js:756:26) Emitted 'error' event on ClientHttp2Session instance at: at emitClose (internal/http2/core.js:1010:10) at internal/http2/core.js:1048:7 at finish (internal/streams/writable.js:731:5) at processTicksAndRejections (internal/process/task_queues.js:80:21){code: 'ERR_HTTP2_ERROR', errno: -505 } ``` The test then calls `session.close()` which tries to write to a destroyed socket. As a result, an unhandled `ECONNRESET` error is emitted in the v12 release line. Backport-PR-URL: #34859 PR-URL: #35482 Refs: #34859 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@lpinca@joesepi
test: add ALPNProtocols option to clientOptions
196a072
Without this, the session is destroyed with the following error ``` Error [ERR_HTTP2_ERROR]: Protocol error at Http2Session.onSessionInternalError (internal/http2/core.js:756:26) Emitted 'error' event on ClientHttp2Session instance at: at emitClose (internal/http2/core.js:1010:10) at internal/http2/core.js:1048:7 at finish (internal/streams/writable.js:731:5) at processTicksAndRejections (internal/process/task_queues.js:80:21){code: 'ERR_HTTP2_ERROR', errno: -505 } ``` The test then calls `session.close()` which tries to write to a destroyed socket. As a result, an unhandled `ECONNRESET` error is emitted in the v12 release line. PR-URL: nodejs#35482 Refs: nodejs#34859 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes

@addaleaxaddaleaxaddaleax approved these changes

Assignees

No one assigned

Labels

testIssues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lpinca@addaleax@nodejs-github-bot@jasnell