Skip to content

Map HTTP/2 RST_STREAM codes back to RPC codes #321

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

Merged
merged 3 commits into from
Jul 15, 2022
Merged

Map HTTP/2 RST_STREAM codes back to RPC codes #321

merged 3 commits into from
Jul 15, 2022

Conversation

akshayjshah
Copy link
Contributor

HTTP/2 includes its own set of error codes, which are sent in RST_STREAM
frames. When servers send one of these codes to clients, we should map
the HTTP/2 code back to one of our status codes.

HTTP/2 includes its own set of error codes, which are sent in RST_STREAM
frames. When servers send one of these codes to clients, we should map
the HTTP/2 code back to one of our status codes.
@akshayjshah akshayjshah requested a review from pkwarren July 15, 2022 03:30
@akshayjshah
Copy link
Contributor Author

The string munging to extract HTTP/2 error codes is quite unpleasant; filed golang/go#53896 to see if the Go team has any interest in making this a bit cleaner.

error.go Outdated
}
msg = strings.TrimSuffix(msg, fromPeerSuffix)
i := strings.LastIndex(msg, ";")
if i < 0 || i == len(msg) {
Copy link
Contributor

@pkwarren pkwarren Jul 15, 2022

Choose a reason for hiding this comment

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

Suggested change
if i < 0 || i == len(msg) {
if i < 0 {

I don't think LastIndex will return len(msg) - I think it will be in range 0:len(msg)-1 if found.

Copy link
Contributor Author

@akshayjshah akshayjshah Jul 15, 2022

Choose a reason for hiding this comment

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

Off by one error on my part, thank you for catching this! I meant to ensure that i < len(msg)-1, since we're slicing with [i+1] below.

@akshayjshah akshayjshah merged commit 1030d2e into main Jul 15, 2022
@akshayjshah akshayjshah deleted the ajs/rst branch July 15, 2022 17:25
Copy link

@Shudogg Shudogg left a comment

Choose a reason for hiding this comment

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

Msg

akshayjshah added a commit that referenced this pull request Jul 26, 2023
HTTP/2 includes its own set of error codes, which are sent in RST_STREAM
frames. When servers send one of these codes to clients, we should map
the HTTP/2 code back to one of our status codes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants