Skip to content

feat(gax): add getErrorInfoList to ErrorDetails #4150

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

Closed
westarle wants to merge 1 commit into from
Closed

feat(gax): add getErrorInfoList to ErrorDetails #4150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions gax-java/gax/src/main/java/com/google/api/gax/rpc/ErrorDetails.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
import com.google.rpc.RequestInfo;
import com.google.rpc.ResourceInfo;
import com.google.rpc.RetryInfo;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import javax.annotation.Nullable;

Expand All @@ -60,6 +62,14 @@ public ErrorInfo getErrorInfo() {
return unpack(ErrorInfo.class);
}

/**
* This returns all occurrences of ErrorInfo. A single error response may contain multiple
* ErrorInfo messages.
*/
public List<ErrorInfo> getErrorInfoList() {
Copy link
Copy Markdown
Contributor

@blakeli0 blakeli0 Mar 16, 2026

Choose a reason for hiding this comment

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

Per aip-193, Each type of detail payload must be included at most once.

I think this is theoretically possible but a violation of AIP.

Copy link
Copy Markdown
Contributor Author

@westarle westarle Mar 16, 2026

Choose a reason for hiding this comment

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

thank you, that's extremely helpful!

return unpackList(ErrorInfo.class);
}

/**
* Describes when the clients can retry a failed request. Clients could ignore the recommendation
* here or retry when this information is missing from error responses.
Expand Down Expand Up @@ -172,4 +182,26 @@ <T extends Message> T unpack(Class<T> errorTypeClazz) {
}
return null;
}

@VisibleForTesting
<T extends Message> List<T> unpackList(Class<T> errorTypeClazz) {
List<Any> rawErrorMessages = getRawErrorMessages();
if (rawErrorMessages == null) {
return Collections.emptyList();
}
List<T> unpackedMessages = new ArrayList<>();
for (Any detail : rawErrorMessages) {
if (detail.is(errorTypeClazz)) {
try {
unpackedMessages.add(detail.unpack(errorTypeClazz));
} catch (InvalidProtocolBufferException e) {
throw new ProtocolBufferParsingException(
String.format(
"Failed to unpack %s from raw error messages", errorTypeClazz.getSimpleName()),
e);
}
}
}
return Collections.unmodifiableList(unpackedMessages);
}
}
22 changes: 22 additions & 0 deletions gax-java/gax/src/test/java/com/google/api/gax/rpc/ErrorDetailsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,28 @@ void errorInfo_shouldUnpackErrorInfoProtoMessage() {
Truth.assertThat(errorDetails.getErrorInfo()).isEqualTo(ERROR_INFO);
}

@Test
void unpackList_shouldReturnEmptyListIfRawErrorMessagesIsNull() {
errorDetails = ErrorDetails.builder().setRawErrorMessages(null).build();
Truth.assertThat(errorDetails.unpackList(ErrorInfo.class)).isEmpty();
}

@Test
void getErrorInfoList_shouldUnpackAllErrorInfoProtoMessages() {
ErrorInfo errorInfo2 =
ErrorInfo.newBuilder().setDomain("googleapis.com").setReason("ANOTHER_REASON").build();

errorDetails =
ErrorDetails.builder()
.setRawErrorMessages(
ImmutableList.of(Any.pack(ERROR_INFO), Any.pack(DEBUG_INFO), Any.pack(errorInfo2)))
.build();

Truth.assertThat(errorDetails.getErrorInfoList())
.containsExactly(ERROR_INFO, errorInfo2)
.inOrder();
}
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 16, 2026

Choose a reason for hiding this comment

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

medium

The new unpackList method can throw a ProtocolBufferParsingException if unpacking fails, which is consistent with the existing unpack method. To ensure complete test coverage for the new logic, please consider adding a test case for this exception scenario, similar to the existing unpack_shouldThrowExceptionIfUnpackingErrorMassageFailed test.

  }

  @Test
  void unpackList_shouldThrowExceptionIfUnpackingErrorMessageFailed() {
    Any malformedErrorType =
        Any.newBuilder()
            .setTypeUrl("type.googleapis.com/google.rpc.ErrorInfo")
            .setValue(ByteString.copyFromUtf8("This is an invalid message!"))
            .build();
    errorDetails =
        ErrorDetails.builder().setRawErrorMessages(ImmutableList.of(malformedErrorType)).build();
    ProtocolBufferParsingException exception =
        assertThrows(
            ProtocolBufferParsingException.class, () -> errorDetails.unpackList(ErrorInfo.class));
    Truth.assertThat(exception.getMessage())
        .isEqualTo(
            String.format(
                "Failed to unpack %s from raw error messages", ErrorInfo.class.getSimpleName()));
  }
References
  1. The repository style guide states that all new logic should be accompanied by tests. The exception handling path in the new unpackList method is currently not covered by tests. (link)


@Test
void retryInfo_shouldUnpackRetryInfoProtoMessage() {
Truth.assertThat(errorDetails.getRetryInfo()).isEqualTo(RETRY_INFO);
Expand Down
Loading