GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — http2: Fix error stream write followed by destroy by davedoesdev · Pull Request #35951 · nodejs/node

Conversation

@davedoesdev

@davedoesdev davedoesdev commented Nov 4, 2020

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Without the fix, the test fails with:

node:internal/http2/core:179
  debugStream(stream[kID], stream[kSession][kType], message, ...args);
                                           ^

TypeError: Cannot read property 'Symbol(type)' of undefined
    at debugStreamObj (node:internal/http2/core:179:44)
    at node:internal/http2/core:2041:7
    at processTicksAndRejections (node:internal/process/task_queues:75:11)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot

nodejs-github-bot commented Nov 4, 2020

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot added the http2 Issues and PRs related to the http2 subsystem. label Nov 4, 2020
crypt096
crypt096 approved these changes Nov 4, 2020
View reviewed changes
mcollina
mcollina approved these changes Nov 4, 2020
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
rickyes
rickyes approved these changes Nov 5, 2020
View reviewed changes
Comment thread test/parallel/test-http2-destroy-after-write.js Outdated

@rickyes rickyes Nov 5, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
session.on('stream', function(stream) {
session.on('stream', common.mustCall(function(stream) {

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/parallel/test-http2-destroy-after-write.js Outdated

@rickyes rickyes Nov 5, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
server.on('session', function(session) {
server.on('session', common.mustCall(function(session) {

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 5, 2020
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 5, 2020
@nodejs-github-bot

nodejs-github-bot commented Nov 5, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/34074/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Nov 5, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/34075/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Trott
Trott approved these changes Nov 5, 2020
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented Nov 5, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/34084/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mcollina

mcollina commented Nov 5, 2020

Copy link
Copy Markdown
Member

There seems to be a problem on ARM :(
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@davedoesdev

davedoesdev commented Nov 5, 2020

Copy link
Copy Markdown
Contributor Author

There seems to be a problem on ARM :(

14:22:04 + git checkout -f refs/remotes/jenkins_tmp
14:22:04 fatal: Unable to create '/home/iojs/build/workspace/node-test-binary-arm/.git/index.lock': File exists.
14:22:04 
14:22:04 Another git process seems to be running in this repository,
...
14:22:07 mkdir: cannot create directory ‘out/junit’: No such file or directory
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Nov 6, 2020 •
edited by Trott
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/34112/ ✅
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@richardlau

richardlau commented Nov 6, 2020

Copy link
Copy Markdown
Member

There seems to be a problem on ARM :(

14:22:04 + git checkout -f refs/remotes/jenkins_tmp
14:22:04 fatal: Unable to create '/home/iojs/build/workspace/node-test-binary-arm/.git/index.lock': File exists.
14:22:04 
14:22:04 Another git process seems to be running in this repository,
...
14:22:07 mkdir: cannot create directory ‘out/junit’: No such file or directory

Should have been fixed by nodejs/build#2475, I've resumed the last CI run.
Trott reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@davedoesdev @Trott
http2: fix error stream write followed by destroy
1fd7d8e 
PR-URL: #35951
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@Trott
Trott merged commit 1fd7d8e into nodejs:master Nov 6, 2020
@Trott

Trott commented Nov 6, 2020

Copy link
Copy Markdown
Member

Landed in 1fd7d8e
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
danielleadams pushed a commit that referenced this pull request Nov 9, 2020
@davedoesdev @danielleadams
http2: fix error stream write followed by destroy
1dd744a 
PR-URL: #35951
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@danielleadams danielleadams mentioned this pull request Nov 9, 2020
v15.2.0 proposal #36055
Merged
BethGriggs pushed a commit that referenced this pull request Dec 9, 2020
@davedoesdev @BethGriggs
http2: fix error stream write followed by destroy
b03ea21 
PR-URL: #35951
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this pull request Dec 10, 2020
@davedoesdev @BethGriggs
http2: fix error stream write followed by destroy
1a520e5 
PR-URL: #35951
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Dec 10, 2020
v14.15.2 proposal #36476
Merged
BethGriggs pushed a commit that referenced this pull request Dec 15, 2020
@davedoesdev @BethGriggs
http2: fix error stream write followed by destroy
92bd7b5 
PR-URL: #35951
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes
@Trott Trott Trott approved these changes
+2 more reviewers
@rickyes rickyes rickyes approved these changes
@crypt096 crypt096 crypt096 approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

http2 Issues and PRs related to the http2 subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@davedoesdev @nodejs-github-bot @mcollina @richardlau @Trott @rickyes @crypt096
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.