Skip to content
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.

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

feat: Error Details Improvement #1929

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.cloud.spanner;

import com.google.api.gax.rpc.ApiException;
import javax.annotation.Nullable;

/**
Expand All @@ -34,6 +35,15 @@ public class AbortedException extends SpannerException {
/** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */
AbortedException(
DoNotConstructDirectly token, @Nullable String message, @Nullable Throwable cause) {
super(token, ErrorCode.ABORTED, IS_RETRYABLE, message, cause);
this(token, message, cause, null);
}

/** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */
AbortedException(
DoNotConstructDirectly token,
@Nullable String message,
@Nullable Throwable cause,
@Nullable ApiException apiException) {
super(token, ErrorCode.ABORTED, IS_RETRYABLE, message, cause, apiException);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.cloud.spanner;

import com.google.api.gax.rpc.ApiException;
import javax.annotation.Nullable;

/**
Expand All @@ -31,6 +32,15 @@ public class AdminRequestsPerMinuteExceededException extends SpannerException {
/** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */
AdminRequestsPerMinuteExceededException(
DoNotConstructDirectly token, @Nullable String message, @Nullable Throwable cause) {
super(token, ErrorCode.RESOURCE_EXHAUSTED, true, message, cause);
this(token, message, cause, null);
}

/** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */
AdminRequestsPerMinuteExceededException(
DoNotConstructDirectly token,
@Nullable String message,
@Nullable Throwable cause,
@Nullable ApiException apiException) {
super(token, ErrorCode.RESOURCE_EXHAUSTED, true, message, cause, apiException);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.cloud.spanner;

import com.google.api.gax.rpc.ApiException;
import com.google.cloud.spanner.SpannerException.ResourceNotFoundException;
import com.google.rpc.ResourceInfo;
import javax.annotation.Nullable;
Expand All @@ -34,6 +35,16 @@ public class DatabaseNotFoundException extends ResourceNotFoundException {
@Nullable String message,
ResourceInfo resourceInfo,
@Nullable Throwable cause) {
super(token, message, resourceInfo, cause);
this(token, message, resourceInfo, cause, null);
}

/** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */
DatabaseNotFoundException(
DoNotConstructDirectly token,
@Nullable String message,
ResourceInfo resourceInfo,
@Nullable Throwable cause,
@Nullable ApiException apiException) {
super(token, message, resourceInfo, cause, apiException);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.cloud.spanner;

import com.google.api.gax.rpc.ApiException;
import com.google.cloud.spanner.SpannerException.ResourceNotFoundException;
import com.google.rpc.ResourceInfo;
import javax.annotation.Nullable;
Expand All @@ -34,6 +35,15 @@ public class InstanceNotFoundException extends ResourceNotFoundException {
@Nullable String message,
ResourceInfo resourceInfo,
@Nullable Throwable cause) {
super(token, message, resourceInfo, cause);
this(token, message, resourceInfo, cause, null);
}
/** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */
InstanceNotFoundException(
DoNotConstructDirectly token,
@Nullable String message,
ResourceInfo resourceInfo,
@Nullable Throwable cause,
@Nullable ApiException apiException) {
super(token, message, resourceInfo, cause, apiException);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.cloud.spanner;

import com.google.api.gax.rpc.ApiException;
import com.google.cloud.spanner.SpannerException.ResourceNotFoundException;
import com.google.rpc.ResourceInfo;
import javax.annotation.Nullable;
Expand All @@ -34,6 +35,16 @@ public class SessionNotFoundException extends ResourceNotFoundException {
@Nullable String message,
ResourceInfo resourceInfo,
@Nullable Throwable cause) {
super(token, message, resourceInfo, cause);
this(token, message, resourceInfo, cause, null);
}

/** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */
SessionNotFoundException(
DoNotConstructDirectly token,
@Nullable String message,
ResourceInfo resourceInfo,
@Nullable Throwable cause,
@Nullable ApiException apiException) {
super(token, message, resourceInfo, cause, apiException);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ public abstract static class ResourceNotFoundException extends SpannerException
DoNotConstructDirectly token,
@Nullable String message,
ResourceInfo resourceInfo,
@Nullable Throwable cause) {
super(token, ErrorCode.NOT_FOUND, /* retryable */ false, message, cause);
@Nullable Throwable cause,
@Nullable ApiException apiException) {
super(token, ErrorCode.NOT_FOUND, /* retryable */ false, message, cause, apiException);
this.resourceInfo = resourceInfo;
}

Expand All @@ -63,24 +64,23 @@ public String getResourceName() {
boolean retryable,
@Nullable String message,
@Nullable Throwable cause) {
super(
message,
// If the cause is instance of APIException then using its cause to avoid the breaking
// change, because earlier we were passing APIException's cause to constructor.
cause instanceof ApiException ? cause.getCause() : cause,
code.getCode(),
retryable);
this(token, code, retryable, message, cause, null);
}

/** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */
SpannerException(
DoNotConstructDirectly token,
ErrorCode code,
boolean retryable,
@Nullable String message,
@Nullable Throwable cause,
@Nullable ApiException apiException) {
super(message, cause, code.getCode(), retryable);
if (token != DoNotConstructDirectly.ALLOWED) {
throw new AssertionError("Do not construct directly: use SpannerExceptionFactory");
}

if (cause instanceof ApiException) {
this.apiException = (ApiException) cause;
} else {
this.apiException = null;
}
this.code = Preconditions.checkNotNull(code);
this.apiException = apiException;
}

/** Returns the error code associated with this exception. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,15 @@ static StatusRuntimeException createAbortedExceptionWithRetryDelay(
}

static SpannerException newSpannerExceptionPreformatted(
ErrorCode code, @Nullable String message, @Nullable Throwable cause) {
ErrorCode code,
@Nullable String message,
@Nullable Throwable cause,
@Nullable ApiException apiException) {
// This is the one place in the codebase that is allowed to call constructors directly.
DoNotConstructDirectly token = DoNotConstructDirectly.ALLOWED;
switch (code) {
case ABORTED:
return new AbortedException(token, message, cause);
return new AbortedException(token, message, cause, apiException);
case RESOURCE_EXHAUSTED:
ErrorInfo info = extractErrorInfo(cause);
if (info != null
Expand All @@ -265,26 +268,35 @@ static SpannerException newSpannerExceptionPreformatted(
&& AdminRequestsPerMinuteExceededException.ADMIN_REQUESTS_LIMIT_VALUE.equals(
info.getMetadataMap()
.get(AdminRequestsPerMinuteExceededException.ADMIN_REQUESTS_LIMIT_KEY))) {
return new AdminRequestsPerMinuteExceededException(token, message, cause);
return new AdminRequestsPerMinuteExceededException(token, message, cause, apiException);
}
case NOT_FOUND:
ResourceInfo resourceInfo = extractResourceInfo(cause);
if (resourceInfo != null) {
switch (resourceInfo.getResourceType()) {
case SESSION_RESOURCE_TYPE:
return new SessionNotFoundException(token, message, resourceInfo, cause);
return new SessionNotFoundException(
token, message, resourceInfo, cause, apiException);
case DATABASE_RESOURCE_TYPE:
return new DatabaseNotFoundException(token, message, resourceInfo, cause);
return new DatabaseNotFoundException(
token, message, resourceInfo, cause, apiException);
case INSTANCE_RESOURCE_TYPE:
return new InstanceNotFoundException(token, message, resourceInfo, cause);
return new InstanceNotFoundException(
token, message, resourceInfo, cause, apiException);
}
}
// Fall through to the default.
default:
return new SpannerException(token, code, isRetryable(code, cause), message, cause);
return new SpannerException(
token, code, isRetryable(code, cause), message, cause, apiException);
}
}

static SpannerException newSpannerExceptionPreformatted(
ErrorCode code, @Nullable String message, @Nullable Throwable cause) {
return newSpannerExceptionPreformatted(code, message, cause, null);
}

private static SpannerException fromApiException(ApiException exception) {
Status.Code code;
if (exception.getStatusCode() instanceof GrpcStatusCode) {
Expand All @@ -295,8 +307,14 @@ private static SpannerException fromApiException(ApiException exception) {
code = Status.Code.UNKNOWN;
}
ErrorCode errorCode = ErrorCode.fromGrpcStatus(Status.fromCode(code));
return SpannerExceptionFactory.newSpannerException(
errorCode, exception.getMessage(), exception);

Throwable exceptionCause = null;

if (exception.getCause() != null) {
exceptionCause = exception.getCause();
}
gauravpurohit06 marked this conversation as resolved.
Show resolved Hide resolved
return SpannerExceptionFactory.newSpannerExceptionPreformatted(
errorCode, formatMessage(errorCode, exception.getMessage()), exceptionCause, exception);
}

private static boolean isRetryable(ErrorCode code, @Nullable Throwable cause) {
Expand Down