Skip to content

impl(gax): refactor SendError to use BoxError - #6475

Open
suzmue wants to merge 4 commits into
googleapis:mainfrom
suzmue:bidi-senderror-refactor
Open

impl(gax): refactor SendError to use BoxError#6475
suzmue wants to merge 4 commits into
googleapis:mainfrom
suzmue:bidi-senderror-refactor

Conversation

@suzmue

@suzmue suzmue commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Refactor SendError in google-cloud-gax to capture BoxError directly in the Serialization variant instead of wrapping crate::error::Error.

Remove temporary stream_closed and ser constructors now that construction is handled directly via SendError::StreamClosed and SendError::Serialization. These were intended to make refactoring the error type easier, but I have decided to keep an enum instead of doing an opaque struct with an internal SendErrorKind.

@suzmue
suzmue requested a review from a team as a code owner August 19, 2026 22:12

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors SendError in the gax crate by removing internal helper methods, simplifying the enum variants, and implementing From<SendError> for crate::error::Error. The review feedback recommends preserving the underlying serialization error details in the Display representation of SendError::Serialization to ensure errors remain actionable and easy to debug, in line with the repository's style guide.

Comment thread src/gax/src/streaming.rs Outdated
Comment thread src/gax/src/streaming.rs Outdated
@suzmue
suzmue marked this pull request as draft August 19, 2026 22:17
@suzmue
suzmue force-pushed the bidi-senderror-refactor branch from 375116a to ea4a33b Compare August 19, 2026 22:19
@suzmue
suzmue marked this pull request as ready for review August 19, 2026 22:40
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.38%. Comparing base (fa829ec) to head (8018a86).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6475   +/-   ##
=======================================
  Coverage   96.38%   96.38%           
=======================================
  Files         298      298           
  Lines       84328    84317   -11     
=======================================
- Hits        81280    81273    -7     
+ Misses       3048     3044    -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@suzmue suzmue changed the title impl(gax): refactor SendError to use BoxError and implement From impl(gax): refactor SendError to use BoxError Aug 19, 2026

@coryan coryan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this still needed if we don't change the examples?

Add SendError::into_error to explicitly convert a SendError into a
gax::error::Error without implementing the From trait.

Hmmm.... I am not sure "without implementing the From trait" is a win....

@suzmue

suzmue commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

is still needed if we don't change the examples?

Add SendError::into_error to explicitly convert a SendError into a
gax::error::Error without implementing the From trait.

Hmmm.... I am not sure "without implementing the From trait" is a win....

When I tried to implement From I got all sorts of errors across our crates that it could no longer determine which type something was. It is totally possible this isn't a big deal and I just didn't look into it closely enough and there was a simple fix (I did not look that hard for one). I will remove the conversion since its unneeded and can look into whether we can implement From without breaking anything, because that is what I wanted to do at first.

suzmue added 4 commits August 20, 2026 17:51
Refactor SendError in google-cloud-gax to capture BoxError directly in
the Serialization variant instead of wrapping crate::error::Error.

Remove temporary stream_closed and ser constructors now that construction
is handled directly via SendError::StreamClosed and
SendError::Serialization.
…essage

Format SendError::Serialization with {0} to preserve the underlying
error details in Display output.
Add SendError::into_error to explicitly convert a SendError into a
gax::error::Error without implementing the From trait.
@suzmue
suzmue force-pushed the bidi-senderror-refactor branch from ee76b5e to 8018a86 Compare August 20, 2026 17:53
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.

2 participants