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.

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

Amend RPC <-> HTTP code mappings in accordance with RFC 003 #1039

Merged
merged 3 commits into from
Mar 18, 2024
Merged

Amend RPC <-> HTTP code mappings in accordance with RFC 003 #1039

merged 3 commits into from
Mar 18, 2024

Conversation

srikrsna-buf
Copy link
Member

Amend RPC <-> HTTP code mappings in accordance with RFC 003.

From the RFC:

This fixes some transport-level bugs and improves interoperability with gRPC, but does change how Connect clients interpret malformed responses.
... most Connect users won't notice these changes, because clients read RPC error codes from the response body. However, this will change clients' interpretation of malformed responses (usually coming from a misbehaving proxy).

@srikrsna-buf srikrsna-buf requested a review from timostamm March 13, 2024 13:56
timostamm
timostamm reviewed Mar 13, 2024
View reviewed changes
Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

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

I think we're good to retire unsupported-content-encoding.spec.ts, provided that the conformance tests cover the status mapping.

packages/connect/src/protocol-connect/http-status.ts
@@ -23,27 +23,15 @@ import { Code } from "../code.js";
export function codeFromHttpStatus(httpStatus: number): Code {
switch (httpStatus) {
case 400: // Bad Request
return Code.InvalidArgument;
return Code.Internal;
Copy link
Member

Choose a reason for hiding this comment

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

Can you update the link in the JSDoc comment for this function to point to the "HTTP to Error Code" section?

@srikrsna-buf srikrsna-buf mentioned this pull request Mar 13, 2024
Closed
@srikrsna-buf srikrsna-buf requested a review from timostamm March 18, 2024 09:25
@srikrsna-buf srikrsna-buf merged commit 06234f1 into main Mar 18, 2024
5 checks passed
@srikrsna-buf srikrsna-buf deleted the sk/rfc-003 branch March 18, 2024 15:15
@paul-sachs paul-sachs mentioned this pull request Mar 26, 2024
Closed
@timostamm timostamm mentioned this pull request Sep 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timostamm timostamm timostamm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@srikrsna-buf @timostamm