Skip to content

Commit

Permalink
fix: update StorageException translation of an ApiException to includ…
Browse files Browse the repository at this point in the history 
…e error details

Update StorageException logic for coalescing ApiExceptions to add a formatted string including fields from error details as a suppressed exception on the api exception.

This keeps the diagnostic information at the "gapic layer" in the printed stacktrace similar to the json document being on the "apiary layer" of the cause.
  • Loading branch information
@BenWhitehead
BenWhitehead committed Jan 9, 2025
1 parent 958c21f commit 7bd8545
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 3 deletions.
42 changes: 39 additions & 3 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/StorageException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,19 @@
import com.google.api.gax.grpc.GrpcStatusCode;
import com.google.api.gax.rpc.ApiException;
import com.google.api.gax.rpc.ApiExceptions;
import com.google.api.gax.rpc.ErrorDetails;
import com.google.api.gax.rpc.StatusCode;
import com.google.cloud.BaseServiceException;
import com.google.cloud.RetryHelper.RetryHelperException;
import com.google.cloud.http.BaseHttpServiceException;
import com.google.common.collect.ImmutableSet;
import com.google.protobuf.TextFormat;
import io.grpc.StatusException;
import io.grpc.StatusRuntimeException;
import java.io.IOException;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
Expand Down Expand Up @@ -127,9 +131,6 @@ static BaseServiceException coalesce(Throwable t) {
static StorageException asStorageException(ApiException apiEx) {
// https://cloud.google.com/storage/docs/json_api/v1/status-codes
// https://cloud.google.com/apis/design/errors#http_mapping
// https://cloud.google.com/apis/design/errors#error_payloads
// TODO: flush this out more to wire through "error" and "details" when we're able to get real
// errors from GCS
int httpStatusCode = 0;
StatusCode statusCode = apiEx.getStatusCode();
if (statusCode instanceof GrpcStatusCode) {
Expand All @@ -155,12 +156,41 @@ static StorageException asStorageException(ApiException apiEx) {
message = "Error: " + statusCodeName;
}

// https://cloud.google.com/apis/design/errors#error_payloads
attachErrorDetails(apiEx);

// It'd be better to use ExceptionData and BaseServiceException#<init>(ExceptionData) but,
// BaseHttpServiceException does not pass that through so we're stuck using this for now.
// TODO: When we can break the coupling to BaseHttpServiceException replace this
return new StorageException(httpStatusCode, message, apiEx.getReason(), apiEx);
}

private static void attachErrorDetails(ApiException ae) {
if (ae != null && ae.getErrorDetails() != null) {
final StringBuilder sb = new StringBuilder();
ErrorDetails ed = ae.getErrorDetails();
sb.append("ErrorDetails {\n");
Stream.of(
ed.getErrorInfo(),
ed.getDebugInfo(),
ed.getQuotaFailure(),
ed.getPreconditionFailure(),
ed.getBadRequest(),
ed.getHelp())
.filter(Objects::nonNull)
.forEach(
msg ->
sb.append("\t\t")
.append(msg.getClass().getSimpleName())
.append(": {")
.append(TextFormat.printer().shortDebugString(msg))
.append(" }\n"));
sb.append("\t}");

ae.addSuppressed(new ApiExceptionErrorDetailsComment(sb.toString()));
}
}

/**
* Translate IOException to a StorageException representing the cause of the error. This method
* defaults to idempotent always being {@code true}. Additionally, this method translates
Expand Down Expand Up @@ -222,4 +252,10 @@ interface IOExceptionCallable<T> {
interface IOExceptionRunnable {
void run() throws IOException;
}

private static final class ApiExceptionErrorDetailsComment extends Throwable {
private ApiExceptionErrorDetailsComment(String message) {
super(message, null, true, false);
}
}
}
93 changes: 93 additions & 0 deletions ...storage/src/test/java/com/google/cloud/storage/StorageExceptionGrpcCompatibilityTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.cloud.storage;

import static com.google.cloud.storage.TestUtils.assertAll;
import static com.google.common.truth.Truth.assertThat;

import com.google.api.gax.grpc.GrpcStatusCode;
Expand All @@ -25,11 +26,21 @@
import com.google.cloud.BaseServiceException;
import com.google.common.collect.ImmutableList;
import com.google.protobuf.Any;
import com.google.protobuf.TextFormat;
import com.google.protobuf.TextFormat.Printer;
import com.google.rpc.BadRequest;
import com.google.rpc.BadRequest.FieldViolation;
import com.google.rpc.DebugInfo;
import com.google.rpc.ErrorInfo;
import com.google.rpc.Help;
import com.google.rpc.Help.Link;
import com.google.rpc.LocalizedMessage;
import com.google.rpc.PreconditionFailure;
import com.google.rpc.QuotaFailure;
import io.grpc.Status;
import io.grpc.Status.Code;
import io.grpc.StatusRuntimeException;
import java.util.List;
import org.junit.Test;

public final class StorageExceptionGrpcCompatibilityTest {
Expand Down Expand Up @@ -114,6 +125,88 @@ public void testCoalesce_UNAUTHENTICATED() {
doTestCoalesce(401, Code.UNAUTHENTICATED);
}

@Test
public void apiExceptionErrorDetails() throws Exception {
ErrorInfo errorInfo =
ErrorInfo.newBuilder()
.setReason("STACKOUT")
.setDomain("spanner.googlepais.com")
.putMetadata("availableRegions", "us-central1,us-east2")
.build();
DebugInfo debugInfo =
DebugInfo.newBuilder()
.addStackEntries("HEAD")
.addStackEntries("HEAD~1")
.addStackEntries("HEAD~2")
.addStackEntries("HEAD~3")
.setDetail("some detail")
.build();
QuotaFailure quotaFailure =
QuotaFailure.newBuilder()
.addViolations(
QuotaFailure.Violation.newBuilder()
.setSubject("clientip:127.0.3.3")
.setDescription("Daily limit")
.build())
.build();
PreconditionFailure preconditionFailure =
PreconditionFailure.newBuilder()
.addViolations(
PreconditionFailure.Violation.newBuilder()
.setType("TOS")
.setSubject("google.com/cloud")
.setDescription("Terms of service not accepted")
.build())
.build();
BadRequest badRequest =
BadRequest.newBuilder()
.addFieldViolations(
FieldViolation.newBuilder()
.setField("email_addresses[3].type[2]")
.setDescription("duplicate value 'WORK'")
.setReason("INVALID_EMAIL_ADDRESS_TYPE")
.setLocalizedMessage(
LocalizedMessage.newBuilder()
.setLocale("en-US")
.setMessage("Invalid email type: duplicate value")
.build())
.build())
.build();
Help help =
Help.newBuilder()
.addLinks(
Link.newBuilder().setDescription("link1").setUrl("https://google.com").build())
.build();
List<Any> errors =
ImmutableList.of(
Any.pack(errorInfo),
Any.pack(debugInfo),
Any.pack(quotaFailure),
Any.pack(preconditionFailure),
Any.pack(badRequest),
Any.pack(help));
ErrorDetails errorDetails = ErrorDetails.builder().setRawErrorMessages(errors).build();
ApiException ae =
ApiExceptionFactory.createException(
Code.OUT_OF_RANGE.toStatus().asRuntimeException(),
GrpcStatusCode.of(Code.OUT_OF_RANGE),
false,
errorDetails);

BaseServiceException se = StorageException.coalesce(ae);
String message = se.getCause().getSuppressed()[0].getMessage();
Printer printer = TextFormat.printer();
assertAll(
() -> assertThat(message).contains("ErrorDetails {"),
() -> assertThat(message).contains(printer.shortDebugString(errorInfo)),
() -> assertThat(message).contains(printer.shortDebugString(debugInfo)),
() -> assertThat(message).contains(printer.shortDebugString(quotaFailure)),
() -> assertThat(message).contains(printer.shortDebugString(preconditionFailure)),
() -> assertThat(message).contains(printer.shortDebugString(badRequest)),
() -> assertThat(message).contains(printer.shortDebugString(help)),
() -> assertThat(message).contains("\t}"));
}

private void doTestCoalesce(int expectedCode, Code code) {
Status status = code.toStatus();
GrpcStatusCode statusCode = GrpcStatusCode.of(code);
Expand Down

0 comments on commit 7bd8545

Please sign in to comment.