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

[v12.x] http2: update handling of streams on rst_stream frames #39659

Closed

Conversation

kumarak
Copy link
Contributor

@kumarak kumarak commented Aug 4, 2021

Backport ref PR #39622 and resolve merge conflict with
git cherry-pick

@github-actions github-actions bot added c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. v12.x labels Aug 4, 2021
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

Should I expect the testcase to fail on earlier versions of Node.js 12? When I test locally with 12.22.4 and without the http2 change in this PR the testcase passes.

@kumarak
Copy link
Contributor Author

kumarak commented Aug 5, 2021

@richardlau, Ideally the test should verify the fixes but I was unable to reproduce the issue without grpc-js. The original issue scripts are here(repro-node-crash). I tried reproducing it with http2 streams.

So I simplified the test case and only checks the RST_STREAM frames getting sent with cancel error code, the stream is in the paused-reading state, and there is no issue with handling it(e.g: no endpoint hang, no stream close error). The behavior is same what it was previously and the test case passes with the older version as well.

@richardlau
Copy link
Member

This went out in 12.22.5 in f2f7a45...9cd1f53.

@richardlau richardlau closed this Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants