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

test: deflake test-http2-empty-frame-without-eof #45212

Closed

Conversation

santigimeno
Copy link
Member

It may happen that the data in emptyframe.http2 reaches the client even before the client has started sending the request, causing an ERR_HTTP2_STREAM_ERROR instead. Fix this by making sure the frame is not sent until some data reaches the server.

Observed on windows when working on #42340

It may happen that the data in `emptyframe.http2` reaches the client
even before the client has started sending the request, causing an
`ERR_HTTP2_STREAM_ERROR` instead. Fix this by making sure the frame is
not sent until some data reaches the server.
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Oct 27, 2022
@santigimeno santigimeno added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Oct 27, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 27, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@lpinca lpinca added the fast-track PRs that do not need to wait for 48 hours to land. label Oct 28, 2022
@github-actions
Copy link
Contributor

Fast-track has been requested by @lpinca. Please 👍 to approve.

@juanarbol
Copy link
Member

The last CI run failed in node-test-commit-linux-containered (unrelated to this PR) job, for whatever reason, GitHub is telling us that this is OK; I will run the CI again just for "security"

@nodejs-github-bot

This comment was marked as outdated.

@juanarbol
Copy link
Member

The last CI run failed in node-test-commit-linux-containered (unrelated to this PR) job, for whatever reason, GitHub is telling us that this is OK; I will run the CI again just for "security"

Nevermind, It was my browser messing around :/

juanarbol pushed a commit that referenced this pull request Oct 28, 2022
It may happen that the data in `emptyframe.http2` reaches the client
even before the client has started sending the request, causing an
`ERR_HTTP2_STREAM_ERROR` instead. Fix this by making sure the frame is
not sent until some data reaches the server.

PR-URL: #45212
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
@juanarbol
Copy link
Member

Landed in ae45d21

@juanarbol juanarbol closed this Oct 28, 2022
@santigimeno santigimeno deleted the santi/unflake_test_http2_empty branch October 28, 2022 18:27
RafaelGSS pushed a commit that referenced this pull request Nov 1, 2022
It may happen that the data in `emptyframe.http2` reaches the client
even before the client has started sending the request, causing an
`ERR_HTTP2_STREAM_ERROR` instead. Fix this by making sure the frame is
not sent until some data reaches the server.

PR-URL: #45212
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Nov 1, 2022
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
It may happen that the data in `emptyframe.http2` reaches the client
even before the client has started sending the request, causing an
`ERR_HTTP2_STREAM_ERROR` instead. Fix this by making sure the frame is
not sent until some data reaches the server.

PR-URL: #45212
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
It may happen that the data in `emptyframe.http2` reaches the client
even before the client has started sending the request, causing an
`ERR_HTTP2_STREAM_ERROR` instead. Fix this by making sure the frame is
not sent until some data reaches the server.

PR-URL: #45212
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
It may happen that the data in `emptyframe.http2` reaches the client
even before the client has started sending the request, causing an
`ERR_HTTP2_STREAM_ERROR` instead. Fix this by making sure the frame is
not sent until some data reaches the server.

PR-URL: #45212
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
It may happen that the data in `emptyframe.http2` reaches the client
even before the client has started sending the request, causing an
`ERR_HTTP2_STREAM_ERROR` instead. Fix this by making sure the frame is
not sent until some data reaches the server.

PR-URL: #45212
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants