feat(gax-httpjson): populate ErrorDetails in HttpJsonApiExceptionFactory by westarle · Pull Request #4145 · googleapis/sdk-platform-java

westarle · 2026-03-16T04:32:17Z

No description provided.

gemini-code-assist · 2026-03-16T04:32:31Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly improves the error reporting capabilities within the gax-httpjson library by enabling the extraction and propagation of rich error details from HTTP JSON responses. By parsing gRPC-style status objects embedded in the HTTP response content, the system can now provide more precise error codes, messages, and structured error information, leading to more robust and informative exception handling for users.

Highlights

Enhanced Error Detail Extraction: The HttpJsonApiExceptionFactory now parses detailed error information, including gRPC status codes, messages, and ErrorDetails, from HTTP JSON error responses. This allows for more granular error handling and diagnostics.
Status Code and Message Overriding: The factory can now override the HTTP status code and message with values provided in the JSON error payload, ensuring that the ApiException reflects the more specific gRPC status when available.
Comprehensive Test Coverage: New unit tests have been added to validate the correct extraction of ErrorDetails, the overriding behavior of status codes and messages, and scenarios where no detailed error information is present.

Changelog

gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonApiExceptionFactory.java
- Imported ErrorDetails and Status classes to support detailed error parsing.
- Added logic to parse the Status object from the HttpResponseException content.
- Implemented conditional logic to override the StatusCode with the gRPC code if present in the parsed Status.
- Updated the message to use the gRPC message if available and non-empty.
- Introduced a new ApiExceptionFactory.createException call to include ErrorDetails when Status details are found.
gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonApiExceptionFactoryTest.java
- Added a new test file for HttpJsonApiExceptionFactory.
- Included testCreate_withErrorDetails to verify the correct parsing and population of ErrorDetails.
- Added testCreate_withOkStatusNoMessageNoDetails to check behavior when only a zero code is present.
- Created testCreate_withCodeOverridesHttpStatusCode to confirm gRPC code overriding HTTP status code.
- Implemented testCreate_withMessageOverridesHttpStatusMessage to ensure gRPC message overrides HTTP status message.
- Added testCreate_withoutErrorDetails to test scenarios where no detailed error payload is available.

Activity

No human activity has been recorded on this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request enhances the HttpJsonApiExceptionFactory to parse and populate ErrorDetails from HTTP JSON error responses, providing more granular error information. The changes correctly extract the status code, message, and details from the response body, overriding the transport-level error information where appropriate. The addition of a comprehensive test suite in HttpJsonApiExceptionFactoryTest is excellent and covers various scenarios, ensuring the new logic is robust. My feedback focuses on a small refactoring to improve code clarity by avoiding parameter reassignment and removing a redundant check, which will enhance maintainability.

gemini-code-assist · 2026-03-16T04:34:11Z

...java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonApiExceptionFactory.java

+      if (status.getMessage() != null && !status.getMessage().isEmpty()) {
+        message = status.getMessage();
+        throwable =
+            new HttpResponseException.Builder(
+                    e.getStatusCode(), e.getStatusMessage(), e.getHeaders())
+                .setContent(e.getContent())
+                .setMessage(message)
+                .build();
+      }
+
+      if (status.getDetailsCount() > 0) {
+        ErrorDetails errorDetails =
+            ErrorDetails.builder().setRawErrorMessages(status.getDetailsList()).build();
+        return ApiExceptionFactory.createException(throwable, statusCode, canRetry, errorDetails);
+      }
      return createApiException(throwable, statusCode, message, canRetry);


For improved clarity and to avoid reassigning a method parameter, I suggest introducing a new cause variable to hold the exception that will be wrapped by the ApiException. This avoids modifying the throwable parameter, which can be confusing.

Additionally, status.getMessage() on a protobuf-generated class will return an empty string if not set, not null. The null check is redundant and can be removed.

Suggested change

if (status.getMessage() != null && !status.getMessage().isEmpty()) {

message = status.getMessage();

throwable =

new HttpResponseException.Builder(

e.getStatusCode(), e.getStatusMessage(), e.getHeaders())

.setContent(e.getContent())

.setMessage(message)

.build();

}

if (status.getDetailsCount() > 0) {

ErrorDetails errorDetails =

ErrorDetails.builder().setRawErrorMessages(status.getDetailsList()).build();

return ApiExceptionFactory.createException(throwable, statusCode, canRetry, errorDetails);

}

return createApiException(throwable, statusCode, message, canRetry);

Throwable cause = throwable;

if (!status.getMessage().isEmpty()) {

message = status.getMessage();

cause =

new HttpResponseException.Builder(

e.getStatusCode(), e.getStatusMessage(), e.getHeaders())

.setContent(e.getContent())

.setMessage(message)

.build();

}

if (status.getDetailsCount() > 0) {

ErrorDetails errorDetails =

ErrorDetails.builder().setRawErrorMessages(status.getDetailsList()).build();

return ApiExceptionFactory.createException(cause, statusCode, canRetry, errorDetails);

}

return createApiException(cause, statusCode, message, canRetry);

blakeli0 · 2026-03-16T23:17:17Z

...java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonApiExceptionFactory.java

+            new HttpResponseException.Builder(
+                    e.getStatusCode(), e.getStatusMessage(), e.getHeaders())
+                .setContent(e.getContent())
+                .setMessage(message)


This seems a bad practice to re-write an exception with a different message. Because when this exception shows up in the stacktrace, it looks as if the message is from the upstream library (http-client), but it is actually being overridden in downstream library (gax).

Is status.getMessage() required? What are the differences between status.getMessage() and the existing message? If it is indeed required, we can create ApiExceptionFactory.createException(message, cause, statusCode, canRetry, errorDetails) that wraps the message with a new exception.

I created ApiExceptionFactory.createException(message, cause, statusCode, canRetry, errorDetails)

I think the existing message is built by HTTP transport so it looks like: "403 Forbidden (dump of exception info)"; I think the AIP 193 message is the right one for production logs and should exclude redundant and very detailed debug info.

blakeli0 · 2026-03-16T23:21:43Z

...java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonApiExceptionFactory.java

+                .build();
+      }
+
+      if (status.getDetailsCount() > 0) {


ErrorDetails can handle empty details list properly, I don't think we need this check. However, we would lose message if we always call ApiExceptionFactory.createException(cause, statusCode, canRetry, errorDetails). This is probably one more reason to create ApiExceptionFactory.createException(message, cause, statusCode, canRetry, errorDetails).

blakeli0 · 2026-03-16T23:33:08Z

...java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonApiExceptionFactory.java

+      if (status.getCode() > 0) {
+        com.google.rpc.Code rpcCode = com.google.rpc.Code.forNumber(status.getCode());
+        if (rpcCode != null) {
+          statusCode = HttpJsonStatusCode.of(rpcCode);


We already have the mapping from http status code to a Gax StatusCode, which is modeled after grpc Status.

Can we keep using the existing statusCode? Or do you think it is not accurate?

left statusCode here, thanks!

blakeli0 · 2026-03-16T23:38:59Z

gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonErrorParser.java

+      try {
+        String statusStr = errorElement.getAsJsonObject().get("status").getAsString();
+        com.google.rpc.Code rpcCode = com.google.rpc.Code.valueOf(statusStr);
+        statusBuilder.setCode(rpcCode.getNumber());


It seems a little hacky to override the response directly. I would rather use the current statusCode as I mentioned in another comment.

If we really need this info, I would suggest returning this info separately. For example, an object that wraps both Status and grpcStatusCode.

I left the current statusCode here, thank you!

product-auto-label bot added the size: m Pull request size is medium. label Mar 16, 2026

gemini-code-assist bot reviewed Mar 16, 2026

View reviewed changes

westarle force-pushed the feat/actionable-errors-http-factory branch from e522952 to cc9a253 Compare March 16, 2026 04:37

westarle marked this pull request as ready for review March 16, 2026 04:47

westarle force-pushed the feat/actionable-errors-http-factory branch 2 times, most recently from 466bba4 to 90fc7af Compare March 16, 2026 15:01

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Mar 16, 2026

westarle force-pushed the feat/actionable-errors-http-factory branch from 90fc7af to d03cdc3 Compare March 16, 2026 17:01

westarle requested review from blakeli0 and diegomarquezp March 16, 2026 17:46

blakeli0 reviewed Mar 16, 2026

View reviewed changes

feat(gax-httpjson): populate ErrorDetails in HttpJsonApiExceptionFactory

573da98

westarle force-pushed the feat/actionable-errors-http-factory branch from d03cdc3 to 573da98 Compare March 17, 2026 00:02

Merge branch 'main' into feat/actionable-errors-http-factory

4838cf7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gax-httpjson): populate ErrorDetails in HttpJsonApiExceptionFactory#4145

feat(gax-httpjson): populate ErrorDetails in HttpJsonApiExceptionFactory#4145
westarle wants to merge 2 commits intogoogleapis:mainfrom
westarle:feat/actionable-errors-http-factory

westarle commented Mar 16, 2026

Uh oh!

gemini-code-assist bot commented Mar 16, 2026

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Mar 16, 2026

Uh oh!

blakeli0 Mar 16, 2026

Uh oh!

westarle Mar 17, 2026

Uh oh!

blakeli0 Mar 16, 2026

Uh oh!

westarle Mar 17, 2026

Uh oh!

blakeli0 Mar 16, 2026

Uh oh!

westarle Mar 17, 2026

Uh oh!

blakeli0 Mar 16, 2026

Uh oh!

westarle Mar 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

westarle commented Mar 16, 2026

Uh oh!

gemini-code-assist bot commented Mar 16, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants