Skip to content

fix(gax): propagate structured LRO error details to ApiException #14022

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

Open
nnicolee wants to merge 13 commits into
base: main
Choose a base branch
from
Open

fix(gax): propagate structured LRO error details to ApiException #14022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
97672a9
feat(gax): propagate structured LRO error details to ApiException
nnicolee Aug 10, 2026
5329849
fix: updated http/json snapshot to adhere to monorepo styling
nnicolee Aug 10, 2026
f6913df
fix: updated following gemini feedback
nnicolee Aug 10, 2026
5607dae
added else block to prevent stale errorDetails
nnicolee Aug 10, 2026
0fbf1ba
updated to return nonnull empty ErrorDetails
nnicolee Aug 10, 2026
7e2c12b
added missing import
nnicolee Aug 10, 2026
afec21c
address PR comments and add http/json lro error integration test
nnicolee Aug 11, 2026
ca4a998
Merge branch 'main' into feat/lro-generic-error-propagation
nnicolee Aug 11, 2026
5e7b995
reverted httpjson integration test
nnicolee Aug 11, 2026
9be3932
changed poetryerror to errorinfo and removed unnecessary checks
nnicolee Aug 11, 2026
5bd14bf
ran linting
nnicolee Aug 11, 2026
55de287
Merge branch 'main' into feat/lro-generic-error-propagation
nnicolee Aug 11, 2026
8438f98
Merge branch 'main' into feat/lro-generic-error-propagation
nnicolee Aug 11, 2026
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
35 changes: 35 additions & 0 deletions ...e/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,19 @@

import com.google.api.gax.longrunning.OperationFuture;
import com.google.api.gax.retrying.RetrySettings;
import com.google.api.gax.rpc.ApiException;
import com.google.protobuf.Any;
import com.google.protobuf.Timestamp;
import com.google.rpc.Code;
import com.google.rpc.ErrorInfo;
import com.google.rpc.Status;
import com.google.showcase.v1beta1.EchoClient;
import com.google.showcase.v1beta1.WaitMetadata;
import com.google.showcase.v1beta1.WaitRequest;
import com.google.showcase.v1beta1.WaitResponse;
import com.google.showcase.v1beta1.it.util.TestClientInitializer;
import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.Test;
import org.threeten.bp.Duration;
Expand Down Expand Up @@ -193,4 +199,33 @@ void testHttpJson_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineExce
TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
}
}

@Test
void testGRPC_LROErrorResponse_propagatesErrorDetails() throws Exception {

@lqiu96 lqiu96 Aug 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know parsing HttpJson is a bit more involved/ difficult since we need to manually unpack the Any proto. Would it be possible to also add a HttpJson variant as well?

@nnicolee nnicolee Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added testHttpJson_LROErrorResponse_propagatesErrorDetails!

to support this, I made ProtoRestSerializer.create(TypeRegistry) public in gax-httpjson so that stubs can serialize custom Any fields in requests, and registered PoetryError in HttpJsonEchoStub's static type registry so the serialization of the wait request body succeeds!

@nnicolee nnicolee Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's possible to add a HttpJson variant, but Any requires registering the type in HttpJsonEchoStub's static type registry and passing it to ProtoRestSerializer.create(). Since HttpJsonEchoStub.java is an auto-generated file, we would need to modify the code generator.

To avoid modifying the code generator, I've opted to verify Http/JSON LRO error Details via unit tests!

@lqiu96 lqiu96 Aug 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

qq, do you know if the Wait RPC also requires the PoetryError? IIRC it was only the FailEchoWithDetails RPC.

For the Wait RPC, would we be able to to just set the detail as an ErrorInfo value?

@nnicolee nnicolee Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Wait RPC doesn't require PoetryError, We can pack any error detail inside the Any field, but we still cannot use Error info for the HTTP/JSON integration test because the REST JSON parser must be configured with a type registry containing the descriptor of the message type inside the Any field so it knows how to parse it.

Since HttpJsonEchoStub only has WaitReponse and WaitMetadata in its registry, it cannot parse ErrorInfo and silently ignores it in the JSON response, leading to null error details.

@lqiu96 lqiu96 Aug 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

REST JSON parser must be configured with a type registry containing the descriptor of the message type inside the Any field so it knows how to parse it.

Hmm, I was a bit confused since I believe the HttpJsonErrorParser should have the types to decode this. I took a look at this and IIUC the issue is that we cannot build the WaitRequest with ErrorInfo as it doesn't exist in the type registry (not the response as I was assuming).

I don't think it makes sense add a public setter in ProtoRestSerializer or modify the stub just for testing. Can we add a a comment above this test that explains why we don't have a HttpJson variant for this?

EchoClient grpcClient = TestClientInitializer.createGrpcEchoClient();
try {
ErrorInfo errorInfo =
ErrorInfo.newBuilder().setReason("TEST_REASON").setDomain("googleapis.com").build();
Status status =
Status.newBuilder()
.setCode(Code.ALREADY_EXISTS_VALUE)
.setMessage("The resource already exists")
.addDetails(Any.pack(errorInfo))
.build();
Comment thread
lqiu96 marked this conversation as resolved.
WaitRequest waitRequest = WaitRequest.newBuilder().setError(status).build();
OperationFuture<WaitResponse, WaitMetadata> operationFuture =
grpcClient.waitOperationCallable().futureCall(waitRequest);
ExecutionException exception = assertThrows(ExecutionException.class, operationFuture::get);
Comment thread
nnicolee marked this conversation as resolved.
assertThat(exception.getCause()).isInstanceOf(ApiException.class);
ApiException apiException = (ApiException) exception.getCause();

// Verify that error details are successfully propagated
assertThat(apiException.getErrorDetails()).isNotNull();
assertThat(apiException.getErrorDetails().getErrorInfo()).isEqualTo(errorInfo);
} finally {
grpcClient.close();
grpcClient.awaitTermination(
TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
}
}
}
9 changes: 9 additions & 0 deletions ...m-java/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcOperationSnapshot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
package com.google.api.gax.grpc;

import com.google.api.gax.longrunning.OperationSnapshot;
import com.google.api.gax.rpc.ErrorDetails;
import com.google.api.gax.rpc.StatusCode;
import com.google.longrunning.Operation;
import io.grpc.Status;
Expand Down Expand Up @@ -79,6 +80,14 @@ public String getErrorMessage() {
return operation.getError().getMessage();
}

/** {@inheritDoc} */
@Override
Comment thread
nnicolee marked this conversation as resolved.
public ErrorDetails getErrorDetails() {
return ErrorDetails.builder()
.setRawErrorMessages(operation.getError().getDetailsList())
.build();
}

public static GrpcOperationSnapshot create(Operation operation) {
return new GrpcOperationSnapshot(operation);
}
Expand Down
3 changes: 2 additions & 1 deletion ...a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ProtoOperationTransformers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ public ResponseT apply(OperationSnapshot operationSnapshot) {
+ operationSnapshot.getErrorMessage(),
null,
operationSnapshot.getErrorCode(),
false);
false,
operationSnapshot.getErrorDetails());
}

if (!(operationSnapshot.getResponse() instanceof Any)) {
Expand Down
33 changes: 29 additions & 4 deletions ...x-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ProtoOperationTransformersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,17 @@
import com.google.api.gax.grpc.ProtoOperationTransformers.MetadataTransformer;
import com.google.api.gax.grpc.ProtoOperationTransformers.ResponseTransformer;
import com.google.api.gax.longrunning.OperationSnapshot;
import com.google.api.gax.rpc.ErrorDetails;
import com.google.api.gax.rpc.UnavailableException;
import com.google.api.gax.rpc.UnknownException;
import com.google.common.truth.Truth;
import com.google.longrunning.Operation;
import com.google.protobuf.Any;
import com.google.rpc.ErrorInfo;
import com.google.rpc.Status;
import com.google.type.Color;
import com.google.type.Money;
import io.grpc.Status.Code;
import java.util.Collections;
import org.junit.jupiter.api.Test;

class ProtoOperationTransformersTest {
Expand All @@ -64,11 +66,13 @@ void testAnyResponseTransformer_exception() {
OperationSnapshot operationSnapshot =
GrpcOperationSnapshot.create(
Operation.newBuilder().setResponse(Any.pack(inputMoney)).setError(status).build());
Exception exception =
UnavailableException exception =
assertThrows(UnavailableException.class, () -> transformer.apply(operationSnapshot));
Truth.assertThat(exception)
.hasMessageThat()
.contains("failed with status = GrpcStatusCode{transportCode=UNAVAILABLE}");
Truth.assertThat(exception.getErrorDetails())
.isEqualTo(ErrorDetails.builder().setRawErrorMessages(Collections.emptyList()).build());
}

@Test
Expand All @@ -78,7 +82,7 @@ void testAnyResponseTransformer_mismatchedTypes() {
OperationSnapshot operationSnapshot =
GrpcOperationSnapshot.create(
Operation.newBuilder()
.setResponse(Any.pack(Color.getDefaultInstance()))
.setResponse(Any.pack(ErrorInfo.getDefaultInstance()))
.setError(status)
.build());
Exception exception =
Expand All @@ -103,11 +107,32 @@ void testAnyMetadataTransformer_mismatchedTypes() {
OperationSnapshot operationSnapshot =
GrpcOperationSnapshot.create(
Operation.newBuilder()
.setMetadata(Any.pack(Color.getDefaultInstance()))
.setMetadata(Any.pack(ErrorInfo.getDefaultInstance()))
.setError(status)
.build());
Exception exception =
assertThrows(UnknownException.class, () -> transformer.apply(operationSnapshot));
Truth.assertThat(exception).hasMessageThat().contains("encountered a problem unpacking it");
}

@Test
void testAnyResponseTransformer_exceptionWithErrorDetails() {
ResponseTransformer<Money> transformer = ResponseTransformer.create(Money.class);
Money inputMoney = Money.newBuilder().setCurrencyCode("USD").build();
ErrorInfo errorInfo =
ErrorInfo.newBuilder().setReason("TEST_REASON").setDomain("googleapis.com").build();
Status status =
Status.newBuilder()
.setCode(Code.UNAVAILABLE.value())
.addDetails(Any.pack(errorInfo))
.build();
OperationSnapshot operationSnapshot =
GrpcOperationSnapshot.create(
Operation.newBuilder().setResponse(Any.pack(inputMoney)).setError(status).build());

UnavailableException exception =
assertThrows(UnavailableException.class, () -> transformer.apply(operationSnapshot));
Truth.assertThat(exception.getErrorDetails()).isNotNull();
Truth.assertThat(exception.getErrorDetails().getErrorInfo()).isEqualTo(errorInfo);
}
}
31 changes: 29 additions & 2 deletions ...ava/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@

import com.google.api.core.InternalApi;
import com.google.api.gax.longrunning.OperationSnapshot;
import com.google.api.gax.rpc.ErrorDetails;
import com.google.api.gax.rpc.StatusCode;
import com.google.api.gax.rpc.StatusCode.Code;
import com.google.longrunning.Operation;
import java.util.Collections;
import org.jspecify.annotations.NullMarked;

/**
Expand All @@ -50,20 +52,23 @@ public class HttpJsonOperationSnapshot implements OperationSnapshot {
private final Object response;
private final StatusCode errorCode;
private final String errorMessage;
private final ErrorDetails errorDetails;

private HttpJsonOperationSnapshot(
String name,
Object metadata,
boolean done,
Object response,
StatusCode errorCode,
String errorMessage) {
String errorMessage,
ErrorDetails errorDetails) {
this.name = name;
this.metadata = metadata;
this.done = done;
this.response = response;
this.errorCode = errorCode;
this.errorMessage = errorMessage;
this.errorDetails = errorDetails;
}

/** {@inheritDoc} */
Expand Down Expand Up @@ -102,6 +107,12 @@ public String getErrorMessage() {
return this.errorMessage;
}

/** {@inheritDoc} */
@Override
public ErrorDetails getErrorDetails() {
Comment thread
nnicolee marked this conversation as resolved.
return this.errorDetails;
}

public static HttpJsonOperationSnapshot create(Operation operation) {
return newBuilder().setOperation(operation).build();
}
Expand All @@ -117,6 +128,19 @@ public static class Builder {
private Object response;
private StatusCode errorCode;
private String errorMessage;
private ErrorDetails errorDetails =
ErrorDetails.builder().setRawErrorMessages(Collections.emptyList()).build();

/**
* Sets the LRO error details.
*
* @param errorDetails the LRO error details
* @return the builder instance
*/
Builder setErrorDetails(final ErrorDetails errorDetails) {
this.errorDetails = errorDetails;
return this;
}

public Builder setName(String name) {
this.name = name;
Expand Down Expand Up @@ -153,11 +177,14 @@ private Builder setOperation(Operation operation) {
this.errorCode =
HttpJsonStatusCode.of(com.google.rpc.Code.forNumber(operation.getError().getCode()));
this.errorMessage = operation.getError().getMessage();
this.errorDetails =
ErrorDetails.builder().setRawErrorMessages(operation.getError().getDetailsList()).build();
return this;
}

public HttpJsonOperationSnapshot build() {
return new HttpJsonOperationSnapshot(name, metadata, done, response, errorCode, errorMessage);
return new HttpJsonOperationSnapshot(
name, metadata, done, response, errorCode, errorMessage, errorDetails);
}
}
}
3 changes: 2 additions & 1 deletion ...va/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoOperationTransformers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ public ResponseT apply(OperationSnapshot operationSnapshot) {
+ operationSnapshot.getErrorMessage(),
null,
operationSnapshot.getErrorCode(),
false);
false,
operationSnapshot.getErrorDetails());
}

if (!(operationSnapshot.getResponse() instanceof Any)) {
Expand Down
22 changes: 22 additions & 0 deletions ...gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshotTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,12 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

import com.google.api.gax.rpc.ErrorDetails;
import com.google.api.gax.rpc.StatusCode.Code;
import com.google.protobuf.Any;
import com.google.protobuf.Empty;
import java.util.ArrayList;
import java.util.Collections;
import org.junit.jupiter.api.Test;

class HttpJsonOperationSnapshotTest {
Expand Down Expand Up @@ -86,4 +90,22 @@ void newBuilderTestNotDone() {
assertEquals(HttpJsonStatusCode.of(Code.OK), testOperationSnapshot.getErrorCode());
assertFalse(testOperationSnapshot.isDone());
}

@Test
void newBuilderTestWithErrorDetails() {
ErrorDetails errorDetails =
ErrorDetails.builder()
.setRawErrorMessages(Collections.singletonList(Any.pack(Empty.getDefaultInstance())))
.build();
HttpJsonOperationSnapshot testOperationSnapshot =
HttpJsonOperationSnapshot.newBuilder()
.setName("snapshot-details")
.setMetadata("Dallas")
.setDone(true)
.setError(400, "Bad Request")
.setErrorDetails(errorDetails)
.build();

assertEquals(errorDetails, testOperationSnapshot.getErrorDetails());
}
}
33 changes: 29 additions & 4 deletions ...ax-httpjson/src/test/java/com/google/api/gax/httpjson/ProtoOperationTransformersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,17 @@
import com.google.api.gax.httpjson.ProtoOperationTransformers.MetadataTransformer;
import com.google.api.gax.httpjson.ProtoOperationTransformers.ResponseTransformer;
import com.google.api.gax.longrunning.OperationSnapshot;
import com.google.api.gax.rpc.ErrorDetails;
import com.google.api.gax.rpc.UnavailableException;
import com.google.api.gax.rpc.UnknownException;
import com.google.common.truth.Truth;
import com.google.longrunning.Operation;
import com.google.protobuf.Any;
import com.google.rpc.Code;
import com.google.rpc.ErrorInfo;
import com.google.rpc.Status;
import com.google.type.Color;
import com.google.type.Money;
import java.util.Collections;
import org.junit.jupiter.api.Test;

class ProtoOperationTransformersTest {
Expand Down Expand Up @@ -96,11 +98,13 @@ void testAnyResponseTransformer_exception() {
HttpJsonOperationSnapshot.create(
Operation.newBuilder().setResponse(Any.pack(inputMoney)).setError(status).build());

Exception exception =
UnavailableException exception =
assertThrows(UnavailableException.class, () -> transformer.apply(operationSnapshot));
Truth.assertThat(exception)
.hasMessageThat()
.contains("failed with status = HttpJsonStatusCode{statusCode=UNAVAILABLE}");
Truth.assertThat(exception.getErrorDetails())
.isEqualTo(ErrorDetails.builder().setRawErrorMessages(Collections.emptyList()).build());
}

@Test
Expand All @@ -110,7 +114,7 @@ void testAnyResponseTransformer_mismatchedTypes() {
OperationSnapshot operationSnapshot =
HttpJsonOperationSnapshot.create(
Operation.newBuilder()
.setResponse(Any.pack(Color.getDefaultInstance()))
.setResponse(Any.pack(ErrorInfo.getDefaultInstance()))
.setError(status)
.build());
Exception exception =
Expand All @@ -135,11 +139,32 @@ void testAnyMetadataTransformer_mismatchedTypes() {
OperationSnapshot operationSnapshot =
HttpJsonOperationSnapshot.create(
Operation.newBuilder()
.setMetadata(Any.pack(Color.getDefaultInstance()))
.setMetadata(Any.pack(ErrorInfo.getDefaultInstance()))
.setError(status)
.build());
Exception exception =
assertThrows(UnknownException.class, () -> transformer.apply(operationSnapshot));
Truth.assertThat(exception).hasMessageThat().contains("encountered a problem unpacking it");
}

@Test
void testAnyResponseTransformer_exceptionWithErrorDetails() {
ResponseTransformer<Money> transformer = ResponseTransformer.create(Money.class);
Money inputMoney = Money.newBuilder().setCurrencyCode("USD").build();
ErrorInfo errorInfo =
ErrorInfo.newBuilder().setReason("TEST_REASON").setDomain("googleapis.com").build();
Status status =
Status.newBuilder()
.setCode(Code.UNAVAILABLE.getNumber())
.addDetails(Any.pack(errorInfo))
.build();
OperationSnapshot operationSnapshot =
HttpJsonOperationSnapshot.create(
Operation.newBuilder().setResponse(Any.pack(inputMoney)).setError(status).build());

UnavailableException exception =
assertThrows(UnavailableException.class, () -> transformer.apply(operationSnapshot));
Truth.assertThat(exception.getErrorDetails()).isNotNull();
Truth.assertThat(exception.getErrorDetails().getErrorInfo()).isEqualTo(errorInfo);
}
}
12 changes: 12 additions & 0 deletions ...orm-java/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationSnapshot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
*/
package com.google.api.gax.longrunning;

import com.google.api.gax.rpc.ErrorDetails;
import com.google.api.gax.rpc.StatusCode;
import java.util.Collections;
import org.jspecify.annotations.NullMarked;

/**
Expand Down Expand Up @@ -67,4 +69,14 @@ public interface OperationSnapshot {
* or if it succeeded, returns null.
*/
String getErrorMessage();

/**
* If the operation is done and it failed, returns the ErrorDetails; if the operation is not done
* or if it succeeded, returns an empty ErrorDetails object.
*
* @return the error details if the operation failed, or an empty ErrorDetails object
*/
default ErrorDetails getErrorDetails() {
return ErrorDetails.builder().setRawErrorMessages(Collections.emptyList()).build();
}
}
Loading