FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: implement snapshot-integrated custom LRO error parsing (POC 3) by nnicolee · Pull Request #14080 · googleapis/google-cloud-java · GitHub

/ google-cloud-java Public

feat: implement snapshot-integrated custom LRO error parsing (POC 3) - #14080

Draft
nnicolee wants to merge 7 commits into
feat/lro-generic-error-propagationfrom
feat/lro-compute-poc3
ConversationCommits (7)ChecksFiles changed
Draft

feat: implement snapshot-integrated custom LRO error parsing (POC 3)#14080
nnicolee wants to merge 7 commits into
feat/lro-generic-error-propagationfrom
feat/lro-compute-poc3

Conversation

nnicolee commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Overview

This PR implements POC 3 for custom LRO error details propagation. In this design, we encapsulate the custom error parsing concern directly inside HttpJsonOperationSnapshot during its instantiation lifecycle.

This approach keeps ProtoOperationTransformers completely clean and aligned with the gRPC implementation, avoiding the need for multiple overloads or passing custom error parser objects down to the response transformer.

Changes

1. GAX Core (gax-httpjson)

  • HttpJsonOperationSnapshot.java: Added .setErrorParser(HttpJsonLroErrorParser) to HttpJsonOperationSnapshot.Builder. If configured, build() invokes the parser on the response object to populate errorDetails and errorMessage on the built snapshot instance.
  • ProtoOperationTransformers.java: Reverted the ResponseTransformer to a single clean implementation. It no longer accepts HttpJsonLroErrorParser and instead directly reads operationSnapshot.getErrorDetails() and operationSnapshot.getErrorMessage().

2. Generator (gapic-generator-java)

  • HttpJsonServiceStubClassComposer.java: Conditionally generates .setErrorParser(new ComputeLroErrorParser()) on the HttpJsonOperationSnapshot builder for long-running compute APIs (ignoring the v1small integration test package to maintain golden safety).
  • RetrySettingsComposer.java: Reverted the POC 2 changes that registered the error parser inside the ResponseTransformer settings factory.

Verification

  • Run integration test suite: bazelisk test //sdk-platform-java/test/integration:compute -> PASSED
  • Run generator unit tests: mvn test -pl :gapic-generator-java -> PASSED

nnicolee changed the base branch from main to feat/lro-generic-error-propagation August 14, 2026 20:31
gemini-code-assist Bot reviewed Aug 14, 2026
View reviewed changes

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Code Review

This pull request introduces support for propagating custom Long-Running Operation (LRO) error details across both gRPC and HTTP/JSON transports. It adds the HttpJsonLroErrorParser interface, implements a ComputeLroErrorParser for Google Cloud Compute, and updates OperationSnapshot and ProtoOperationTransformers to carry ErrorDetails. However, the package matching logic in HttpJsonServiceStubClassComposer incorrectly targets com.google.cloud.compute.v1small as well, which will cause compilation failures in the generated golden files because ComputeLroErrorParser is package-private and unavailable in that package. This should be resolved by explicitly excluding v1small from the generator's condition.

Comment thread ...main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceStubClassComposer.java Outdated
.apply("setError", Arrays.asList(getHttpErrorStatusCodeExpr, getHttpErrorMessageExpr))
.apply(newBuilderExpr);

if (service.pakkage().startsWith("com.google.cloud.compute.v1")) {

gemini-code-assist Bot Aug 14, 2026

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The package check service.pakkage().startsWith("com.google.cloud.compute.v1") evaluates to true for "com.google.cloud.compute.v1small". This causes the generator to incorrectly inject .setErrorParser(new ComputeLroErrorParser()) into the v1small golden files (as seen in HttpJsonAddressesStub.java and HttpJsonRegionOperationsStub.java).

Since ComputeLroErrorParser is a hand-written class in com.google.cloud.compute.v1.stub and is package-private, it is not present or accessible in com.google.cloud.compute.v1small.stub, which will cause compilation failures when building the goldens.

To fix this and properly ignore the v1small package as intended, we should explicitly exclude it in the condition.

Suggested change
if (service.pakkage().startsWith("com.google.cloud.compute.v1")) {
if (service.pakkage().startsWith("com.google.cloud.compute.v1")
&& !service.pakkage().startsWith("com.google.cloud.compute.v1small")) {

sonarqubecloud Bot commented Aug 14, 2026

Copy link
Copy Markdown

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

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

See analysis details on SonarQube Cloud

sonarqubecloud Bot commented Aug 14, 2026

Copy link
Copy Markdown

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

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

See analysis details on SonarQube Cloud

nnicolee force-pushed the feat/lro-compute-poc3 branch from 006e4a7 to 6c6aed1 Compare August 14, 2026 20:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

Back | FazBrowse Home | New Git URL