-
Notifications
You must be signed in to change notification settings - Fork 86
[PLUGIN-1870] Add BigtableErrorDetailsProvider #1526
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
Conversation
04321a2 to
3f2ead3
Compare
itsankit-google
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls add screenshot of RPC code returned by BigTableAPI client.
77e903d to
45e50e2
Compare
| private static final String ERROR_MESSAGE_FORMAT = "Error occurred in the phase: '%s'. Error message: %s"; | ||
|
|
||
| static Map<Status.Code, Integer> actionErrorMap = new HashMap<>(); | ||
|
|
||
| static { | ||
| actionErrorMap.put(Status.Code.CANCELLED, 499); | ||
| actionErrorMap.put(Status.Code.UNKNOWN, 500); | ||
| actionErrorMap.put(Status.Code.INVALID_ARGUMENT, 400); | ||
| actionErrorMap.put(Status.Code.DEADLINE_EXCEEDED, 504); | ||
| actionErrorMap.put(Status.Code.NOT_FOUND, 404); | ||
| actionErrorMap.put(Status.Code.ALREADY_EXISTS, 409); | ||
| actionErrorMap.put(Status.Code.PERMISSION_DENIED, 403); | ||
| actionErrorMap.put(Status.Code.UNAUTHENTICATED, 401); | ||
| actionErrorMap.put(Status.Code.RESOURCE_EXHAUSTED, 429); | ||
| actionErrorMap.put(Status.Code.FAILED_PRECONDITION, 400); | ||
| actionErrorMap.put(Status.Code.ABORTED, 409); | ||
| actionErrorMap.put(Status.Code.OUT_OF_RANGE, 400); | ||
| actionErrorMap.put(Status.Code.UNIMPLEMENTED, 501); | ||
| actionErrorMap.put(Status.Code.INTERNAL, 500); | ||
| actionErrorMap.put(Status.Code.UNAVAILABLE, 503); | ||
| actionErrorMap.put(Status.Code.DATA_LOSS, 500); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is duplicated, can we move this to Util class and use it everywhere?
google-cloud/src/main/java/io/cdap/plugin/gcp/spanner/common/SpannerErrorDetailsProvider.java
Lines 38 to 59 in 741fe90
| private static final String ERROR_MESSAGE_FORMAT = "Error occurred in the phase: '%s'. Error message: %s"; | |
| static Map<ErrorCode, Integer> actionErrorMap = new HashMap<>(); | |
| static { | |
| actionErrorMap.put(ErrorCode.CANCELLED, 499); | |
| actionErrorMap.put(ErrorCode.UNKNOWN, 500); | |
| actionErrorMap.put(ErrorCode.INVALID_ARGUMENT, 400); | |
| actionErrorMap.put(ErrorCode.DEADLINE_EXCEEDED, 504); | |
| actionErrorMap.put(ErrorCode.NOT_FOUND, 404); | |
| actionErrorMap.put(ErrorCode.ALREADY_EXISTS, 409); | |
| actionErrorMap.put(ErrorCode.PERMISSION_DENIED, 403); | |
| actionErrorMap.put(ErrorCode.UNAUTHENTICATED, 401); | |
| actionErrorMap.put(ErrorCode.RESOURCE_EXHAUSTED, 429); | |
| actionErrorMap.put(ErrorCode.FAILED_PRECONDITION, 400); | |
| actionErrorMap.put(ErrorCode.ABORTED, 409); | |
| actionErrorMap.put(ErrorCode.OUT_OF_RANGE, 400); | |
| actionErrorMap.put(ErrorCode.UNIMPLEMENTED, 501); | |
| actionErrorMap.put(ErrorCode.INTERNAL, 500); | |
| actionErrorMap.put(ErrorCode.UNAVAILABLE, 503); | |
| actionErrorMap.put(ErrorCode.DATA_LOSS, 500); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, moved the code to common place.
| if (innerCause instanceof StatusRuntimeException) { | ||
| return getProgramFailureExceptionFromBigTableException((StatusRuntimeException) innerCause); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for every innerCause we can just call this.getExceptionDetails(innerCause, errorContext), if it is not null, we can continue in the loop otherwise return the exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now calling return this.getExceptionDetails((Exception) innerCause, errorContext), i don't get what is meant by if not null we can continue ?
BigTable [GRPC = 7][
{
"stageName": "Bigtable",
"errorCategory": "Plugin-'Bigtable'",
"errorReason": "403 PERMISSION_DENIED: Access denied. Missing IAM permission: bigtable.tables.mutateRows.. Please check you have permission to access this resource. For more details, see https://cloud.google.com/bigtable/docs/status-codes.",
"errorMessage": "com.google.bigtable.repackaged.io.grpc.StatusRuntimeException: [ErrorCode='403'] PERMISSION_DENIED: Access denied. Missing IAM permission: bigtable.tables.mutateRows.",
"errorType": "USER",
"dependency": "true",
"errorCodeType": "HTTP",
"errorCode": "403",
"supportedDocumentationUrl": "https://cloud.google.com/bigtable/docs/status-codes"
}
]Spanner [GRPC = 7][
{
"stageName": "Spanner2",
"errorCategory": "Plugin-\\u0027Spanner2\\u0027",
"errorReason": "403 PERMISSION_DENIED: io.grpc.StatusRuntimeException: PERMISSION_DENIED: Caller is missing IAM permission spanner.databases.updateDdl on resource projects/cdf-entcon/instances/e2e-20231011-09-35-048376917/databases/e2e-source-db-a8e7a64a-c.. Please check you have permission to access this resource. For more details, see https://cloud.google.com/spanner/docs/error-codes.",
"errorMessage": "com.google.cloud.spanner.SpannerException: [ErrorCode\\u003d\\u0027403\\u0027] PERMISSION_DENIED: io.grpc.StatusRuntimeException: PERMISSION_DENIED: Caller is missing IAM permission spanner.databases.updateDdl on resource projects/cdf-entcon/instances/e2e-20231011-09-35-048376917/databases/e2e-source-db-a8e7a64a-c.",
"errorType": "USER",
"dependency": "true",
"errorCodeType": "HTTP",
"errorCode": "403",
"supportedDocumentationUrl": "https://cloud.google.com/spanner/docs/error-codes"
}
] |
| public static final Map<Integer, Integer> GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP = new HashMap<>(); | ||
|
|
||
| static { | ||
| // https://github.com/grpc/grpc/blob/master/doc/statuscodes.md | ||
| // OK 0 <--> HTTP 200 (OK) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(0, 200); | ||
| // CANCELLED 1 <--> HTTP 499 (Client Closed Request) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(1, 499); | ||
| // UNKNOWN 2 <--> HTTP 500 (Internal Server Error) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(2, 500); | ||
| // INVALID_ARGUMENT 3 <--> HTTP 400 (Bad Request) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(3, 400); | ||
| // DEADLINE_EXCEEDED 4 <--> HTTP 504 (Gateway Timeout) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(4, 504); | ||
| // NOT_FOUND 5 <--> HTTP 404 (Not Found) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(5, 404); | ||
| // ALREADY_EXISTS 6 <--> HTTP 409 (Conflict) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(6, 409); | ||
| // PERMISSION_DENIED 7 <--> HTTP 403 (Forbidden) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(7, 403); | ||
| // RESOURCE_EXHAUSTED 8 <--> HTTP 429 (Too Many Requests) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(8, 429); | ||
| // FAILED_PRECONDITION 9 <--> HTTP 400 (Bad Request) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(9, 400); | ||
| // ABORTED 10 <--> HTTP 409 (Conflict) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(10, 409); | ||
| // OUT_OF_RANGE 11 <--> HTTP 400 (Bad Request) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(11, 400); | ||
| // UNIMPLEMENTED 12 <--> HTTP 501 (Not Implemented) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(12, 501); | ||
| // INTERNAL 13 <--> HTTP 500 (Internal Server Error) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(13, 500); | ||
| // UNAVAILABLE 14 <--> HTTP 503 (Service Unavailable) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(14, 503); | ||
| // DATA_LOSS 15 <--> HTTP 500 (Internal Server Error) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(15, 500); | ||
| // UNAUTHENTICATED 16 <--> HTTP 401 (Unauthorized) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(16, 401); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please convert it into a static immutable map as done in reference shared in above comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// https://github.com/grpc/grpc/blob/master/doc/statuscodes.md
public static final Map<Integer, Integer> GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP = Collections.unmodifiableMap(
new HashMap<Integer, Integer>() {{
put(3, 400); // INVALID_ARGUMENT <--> HTTP 400 (Bad Request)
put(4, 504); // DEADLINE_EXCEEDED <--> HTTP 504 (Gateway Timeout)
put(5, 404); // NOT_FOUND <--> HTTP 404 (Not Found)
put(6, 409); // ALREADY_EXISTS <--> HTTP 409 (Conflict)
put(7, 403); // PERMISSION_DENIED <--> HTTP 403 (Forbidden)
put(8, 429); // RESOURCE_EXHAUSTED <--> HTTP 429 (Too Many Requests)
put(9, 400); // FAILED_PRECONDITION <--> HTTP 400 (Bad Request)
put(10, 409); // ABORTED <--> HTTP 409 (Conflict)
put(11, 400); // OUT_OF_RANGE <--> HTTP 400 (Bad Request)
put(12, 501); // UNIMPLEMENTED <--> HTTP 501 (Not Implemented)
put(13, 500); // INTERNAL <--> HTTP 500 (Internal Server Error)
put(14, 503); // UNAVAILABLE <--> HTTP 503 (Service Unavailable)
put(15, 500); // DATA_LOSS <--> HTTP 500 (Internal Server Error)
put(16, 401); // UNAUTHENTICATED <--> HTTP 401 (Unauthorized)
}});| // UNAUTHENTICATED 16 <--> HTTP 401 (Unauthorized) | ||
| GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.put(16, 401); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove empty line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed!
| import io.cdap.plugin.gcp.common.GCPErrorDetailsProvider; | ||
| import io.cdap.plugin.gcp.common.GCPErrorDetailsProviderUtil; | ||
| import io.cdap.plugin.gcp.common.GCPUtils; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove empty line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed!
| List<Throwable> innerCauses = r.getCauses(); | ||
| for (Throwable innerCause : innerCauses) { | ||
| if (innerCause instanceof Exception) { | ||
| return this.getExceptionDetails((Exception) innerCause, errorContext); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure that all innerCauses will contain the actual exception we are looking for?
It is still possible that innerCauses[0] returns null and does not contain StatusRuntimeException, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed to Exception so that even the parent class can handle if there is a known error,
innerCause being null won't cause an issue as we check with instanceof Exception, it will be skipped anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was not about innerCause being null, this.getExceptionDetails((Exception) innerCause, errorContext) can still return null and loop will break and we will not traverse forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, got it, added a null check.
if (innerCause instanceof Exception) {
ProgramFailureException pfe = this.getExceptionDetails((Exception) innerCause, errorContext);
if (pfe != null) {
return pfe;
}
}|
|
||
| public static ProgramFailureException getProgramFailureExceptionByGrpcStatusCode(int grpcErrorCodeValue, | ||
| String grpcErrorReason, String grpcErrorMessage, String supportedDocUrl, Exception se) { | ||
| int httpStatusCode = GCPErrorDetailsProviderUtil.GCP_GRPC_ERROR_CODE_HTTP_STATUS_CODE_MAP.get(grpcErrorCodeValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the value is not found in map use 500.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| actionErrorPair != null ? actionErrorPair.getErrorType() : ErrorType.UNKNOWN, true, ErrorCodeType.HTTP, | ||
| String.valueOf(httpStatusCode), supportedDocUrl, se); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: remove empty line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done


ErrorDetailsProvider - BigTable [Source|Sink] plugin
Jira : PLUGIN-1870
Description
Implement Program Failure Exception Handling in BigTable Source/Sink plugin to catch known errors.
Code change
BigtableErrorDetailsProvider.javaBigtableSink.javaBigtableSource.javaGCPErrorDetailsProvider.javaGCPUtils.javaTests
[ { "stageName": "Bigtable", "errorCategory": "Plugin-'Bigtable'", "errorReason": "403 PERMISSION_DENIED: Access denied. Missing IAM permission: bigtable.tables.mutateRows.. Please check you have permission to access this resource. For more details, see https://cloud.google.com/bigtable/docs/status-codes.", "errorMessage": "com.google.bigtable.repackaged.io.grpc.StatusRuntimeException: [ErrorCode='403'] PERMISSION_DENIED: Access denied. Missing IAM permission: bigtable.tables.mutateRows.", "errorType": "USER", "dependency": "true", "errorCodeType": "HTTP", "errorCode": "403", "supportedDocumentationUrl": "https://cloud.google.com/spanner/docs/error-codes" } ]