Skip to content

fix(gax): implement getMessage() in FlowController exceptions - #14077

Open
NiranjanKumar001 wants to merge 9 commits into
googleapis:mainfrom
NiranjanKumar001:issues/12950
Open

fix(gax): implement getMessage() in FlowController exceptions#14077
NiranjanKumar001 wants to merge 9 commits into
googleapis:mainfrom
NiranjanKumar001:issues/12950

Conversation

@NiranjanKumar001

Copy link
Copy Markdown

Fixes #12950

@NiranjanKumar001
NiranjanKumar001 requested review from a team as code owners August 14, 2026 12:33

@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 overrides the getMessage() and toString() methods in MaxOutstandingElementCountReachedException and MaxOutstandingRequestBytesReachedException within FlowController.java, and adds a test case to verify their outputs. The reviewer recommends passing the formatted message to the superclass constructor via super(message) instead of overriding these methods, which aligns with standard Java exception handling practices and ensures proper serialization and logging.

@NiranjanKumar001

Copy link
Copy Markdown
Author

/gemini review

@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 FlowControlException and its subclasses (MaxOutstandingElementCountReachedException and MaxOutstandingRequestBytesReachedException) to pass exception messages to the superclass constructor instead of overriding toString(), and adds corresponding unit tests. The review feedback recommends changing the visibility of the new FlowControlException constructor from protected to private to preserve the API design constraint that prevents external subclassing, since nested subclasses can still access private constructors.

…x/batching/FlowController.java

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@NiranjanKumar001

Copy link
Copy Markdown
Author

/gemini review

@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 FlowController exceptions to pass formatted error messages to the parent FlowControlException constructor instead of overriding toString(), and adds a test to verify these messages. The review feedback correctly identifies a grammatical error in the exception messages, suggesting changing 'have been reached' to 'has been reached' and updating the corresponding test assertions for consistency. All comments are constructive and should be addressed.

NiranjanKumar001 and others added 4 commits August 14, 2026 20:36
…x/batching/FlowController.java

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…x/batching/FlowController.java

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…x/batching/FlowControllerTest.java

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…x/batching/FlowControllerTest.java

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@NiranjanKumar001

Copy link
Copy Markdown
Author

/gemini review

@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 updates the exceptions in FlowController to pass formatted error messages to the parent Exception constructor instead of overriding toString(), and adds a corresponding unit test. The review feedback suggests making the new test method public to ensure it is executed properly under JUnit 4.

…x/batching/FlowControllerTest.java

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@NiranjanKumar001

Copy link
Copy Markdown
Author

/gemini review

@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 the exceptions in FlowController to pass descriptive error messages to the parent FlowControlException constructor instead of overriding toString(), and adds unit tests to verify these messages. The reviewer feedback suggests removing the now-unused no-argument constructor in FlowControlException and replacing String.format with string concatenation in the exception constructors to avoid performance overhead in high-throughput scenarios.

@NiranjanKumar001

Copy link
Copy Markdown
Author

/gemini review

@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 updates FlowController's custom exceptions (MaxOutstandingElementCountReachedException and MaxOutstandingRequestBytesReachedException) to pass descriptive error messages to their superclass constructor instead of overriding toString(). Additionally, a unit test has been added to verify that getMessage() returns the correct error messages. There are no review comments, so no further feedback is provided.

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.

Exceptions created by FlowController don't provide getMessage method implementations

1 participant