Skip to content

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

Open
nnicolee wants to merge 13 commits into
mainfrom
feat/lro-generic-error-propagation
Open

fix(gax): propagate structured LRO error details to ApiException#14022
nnicolee wants to merge 13 commits into
mainfrom
feat/lro-generic-error-propagation

Conversation

@nnicolee

@nnicolee nnicolee commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Description:

When a Long-Running Operation (LRO) completes with a failure, structured error details inside the operation's error payload were previously dropped. This made it impossible for client applications to access fine-grained provider error messages (such as quota or usage violations) via the public ApiException.getErrorDetails() API.

This PR implements Phase 1 of the LRO error propagation design by establishing the transport-agnostic mechanism to propagate structured LRO error details in GAX for both gRPC and HTTP/JSON (REST) transports.

Design doc: go/sdk:java-lro-error-details

Key Changes:

  1. Core GAX Interfaces:
    • Added support to fetch structured error details from the LRO operation snapshots.
  2. gRPC Transport:
    • Implemented error details extraction from LRO operation error payloads (only populated if structured error details are present).
    • Updated the gRPC response transformer to pass these error details when converting failed operations into exception objects.
    • Added unit test validation checking details propagation for LRO failures over gRPC.
  3. HTTP/JSON (REST) Transport:
    • Implemented error details extraction and storage in REST LRO operation snapshots (only populated if structured error details are present).
    • Updated the REST response transformer to fetch and pass the snapshot's error details.
    • Added unit test validation checking details propagation for LRO failures over HTTP/JSON.
  4. Integration & Unit Testing:
    • Added a Showcase integration test verifying successful end-to-end propagation of error details over gRPC.
    • Note on HTTP/JSON Integration Test: Deferring the end-to-end Showcase integration test for HTTP/JSON LROs to Phase 2 because HTTP/JSON relies on a TypeRegistry in generated stubs to parse custom/standard payload types packed in Any details, the test client fails to deserialize these details. Once we roll out the generator changes in Phase 2 to automatically add error details types to generated stub registries, the Showcase REST integration test will pass out-of-the-box. We have added unit tests to cover the HTTP/JSON LRO response parsing pathway in the interim.

Testing:

  • Verified that all gRPC and HTTP/JSON transformer unit tests pass successfully:
    mvn test -pl sdk-platform-java/gax-java/gax-grpc,sdk-platform-java/gax-java/gax-httpjson -Dtest=ProtoOperationTransformersTest
  • Verified that the Showcase LRO integration test passes successfully:
    mvn test -pl java-showcase/gapic-showcase -Dtest=ITLongRunningOperation

@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 adds support for propagating error details in long-running operations (LRO) for both gRPC and HTTP/JSON transports. This is achieved by adding getErrorDetails() to the OperationSnapshot interface and implementing it in GrpcOperationSnapshot and HttpJsonOperationSnapshot. The ProtoOperationTransformers are updated to pass these error details when throwing exceptions, and corresponding integration and unit tests are added. Feedback on the changes includes renaming a misleadingly named test that asserts error propagation rather than dropping, and ensuring that ErrorDetails is only instantiated and returned in GrpcOperationSnapshot and HttpJsonOperationSnapshot if there are actual details present (i.e., checking that the details count is greater than zero).

@nnicolee

Copy link
Copy Markdown
Contributor 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 adds support for propagating error details from long-running operations (LROs) across both gRPC and HTTP/JSON transports by introducing a getErrorDetails() method to the OperationSnapshot interface and implementing it in GrpcOperationSnapshot and HttpJsonOperationSnapshot. It also updates ProtoOperationTransformers to pass these error details when throwing exceptions and adds corresponding integration and unit tests. Feedback on the changes highlights a potential issue in HttpJsonOperationSnapshot.Builder.setOperation where errorDetails is not reset to null if the operation has no error, which could lead to stale state if the builder is reused.

@nnicolee

Copy link
Copy Markdown
Contributor 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 adds support for propagating error details in long-running operations (LRO) for both gRPC and HTTP/JSON transports. It introduces the getErrorDetails() method to the OperationSnapshot interface and implements it in GrpcOperationSnapshot and HttpJsonOperationSnapshot. Additionally, it updates the ProtoOperationTransformers to include these error details when constructing exceptions, and adds corresponding unit and integration tests. The reviewer suggested adding a timeout to the operationFuture.get() call in the new integration test to prevent the test suite from hanging indefinitely.

@nnicolee
nnicolee marked this pull request as ready for review August 10, 2026 14:21
@nnicolee
nnicolee requested review from a team as code owners August 10, 2026 14:21
@nnicolee
nnicolee requested a review from lqiu96 August 10, 2026 14:21
@@ -193,4 +199,35 @@ void testHttpJson_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineExce
TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
}
}

@Test
void testGRPC_LROErrorResponse_propagatesErrorDetails() throws Exception {

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?

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!

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!

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?

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.

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?

@nnicolee nnicolee added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 11, 2026
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 11, 2026
@nnicolee nnicolee added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 11, 2026
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed for 'gapic-generator-java-root'

Failed conditions
68.8% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed for 'gapic-generator-java-root'

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 11, 2026
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.

4 participants