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

feat: use StreamErrorHandler to send back invalid argument error in bidirectional streaming #4864

Merged
merged 5 commits into from
Oct 24, 2024

Conversation

ianbbqzy
Copy link
Contributor

@ianbbqzy ianbbqzy commented Oct 23, 2024

References to other Issues or PRs

Fixes #4795

follow up to #4819

Have you read the Contributing Guidelines?

yes

Brief description of what is fixed or changed

Use StreamErrorHandler instead of ErrorHandler for sending back invalid request errors.

Other comments

taking inspiration from handleForwardResponseStreamError

can add some unit tests if looks good overall

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

Fair enough, this makes sense to me!

runtime/errors.go Outdated Show resolved Hide resolved
@ianbbqzy ianbbqzy force-pushed the stream-error-handler branch from ea65295 to 2b48c69 Compare October 24, 2024 06:24
@ianbbqzy ianbbqzy force-pushed the stream-error-handler branch 2 times, most recently from 5bb2105 to 71053cc Compare October 24, 2024 06:40
@ianbbqzy ianbbqzy force-pushed the stream-error-handler branch from 71053cc to 7bd4ec4 Compare October 24, 2024 06:59
Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

Thanks, I love this test!

examples/internal/server/a_bit_of_everything.go Outdated Show resolved Hide resolved
runtime/errors.go Outdated Show resolved Hide resolved
@ianbbqzy ianbbqzy force-pushed the stream-error-handler branch 2 times, most recently from 6599679 to c05b3bc Compare October 24, 2024 18:40
@ianbbqzy ianbbqzy force-pushed the stream-error-handler branch from c05b3bc to 1bf22c6 Compare October 24, 2024 18:46
@johanbrandhorst johanbrandhorst merged commit 739d2ee into grpc-ecosystem:main Oct 24, 2024
17 checks passed
@johanbrandhorst
Copy link
Collaborator

Thanks for the contribution!

@ianbbqzy
Copy link
Contributor Author

Hey Johan, curious if there will be a release soon, or is it something that I have to initiate following the instructions in CONTRIBUTION.md?

@johanbrandhorst
Copy link
Collaborator

I'm planning a release soon, yeah, but there's another change in progress that I want to get in first: #4866

@johanbrandhorst
Copy link
Collaborator

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.

Best practices for handling decode failures in a websocket connection
2 participants