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

Keep Record of Original Exception in S3 Callbacks #700

Merged
merged 27 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,48 +11,62 @@ public class S3FinishedResponseContext {
private final ChecksumAlgorithm checksumAlgorithm;
private final boolean didValidateChecksum;

private final Throwable cause;

/*
* errorCode The CRT error code
* responseStatus statusCode of the HTTP response
* errorPayload body of the error response. Can be null if the request completed successfully
* checksumAlgorithm, the algorithm used to validate the Body, None if not validated
* didValidateChecksum which is true if the response was validated.
* cause of the error such as a Java exception in a callback. Maybe NULL if there was no exception in a callback.
*/
S3FinishedResponseContext(final int errorCode, final int responseStatus, final byte[] errorPayload, final ChecksumAlgorithm checksumAlgorithm, final boolean didValidateChecksum) {
S3FinishedResponseContext(final int errorCode, final int responseStatus, final byte[] errorPayload, final ChecksumAlgorithm checksumAlgorithm, final boolean didValidateChecksum, Throwable cause) {
this.errorCode = errorCode;
this.responseStatus = responseStatus;
this.errorPayload = errorPayload;
this.checksumAlgorithm = checksumAlgorithm;
this.didValidateChecksum = didValidateChecksum;
this.cause = cause;
}

public int getErrorCode () {
public int getErrorCode() {
return this.errorCode;
}

/*
* If the request didn't receive a response due to a connection
* failure or some other issue the response status will be 0.
*/
public int getResponseStatus () {
public int getResponseStatus() {
return this.responseStatus;
}

/*
* In the case of a failed http response get the payload of the response.
*/
public byte[] getErrorPayload () {
public byte[] getErrorPayload() {
return this.errorPayload;
}

/*
* if no checksum is found, or the request finished with an error the Algorithm will be None,
* otherwise the algorithm will correspond to the one attached to the object when uploaded.
*/
public ChecksumAlgorithm getChecksumAlgorithm () {
public ChecksumAlgorithm getChecksumAlgorithm() {
return this.checksumAlgorithm;
}
public boolean isChecksumValidated () {

public boolean isChecksumValidated() {
return this.didValidateChecksum;
}

/**
* Cause of the error, such as a Java exception from a callback. May be NULL if there was no exception in a callback.
* @return throwable
*/
public Throwable getCause() {
return cause;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ int onResponseBody(byte[] bodyBytesIn, long objectRangeStart, long objectRangeEn
return this.responseHandler.onResponseBody(ByteBuffer.wrap(bodyBytesIn), objectRangeStart, objectRangeEnd);
}

void onFinished(int errorCode, int responseStatus, byte[] errorPayload, int checksumAlgorithm, boolean didValidateChecksum) {
S3FinishedResponseContext context = new S3FinishedResponseContext(errorCode, responseStatus, errorPayload, ChecksumAlgorithm.getEnumValueFromInteger(checksumAlgorithm), didValidateChecksum);
void onFinished(int errorCode, int responseStatus, byte[] errorPayload, int checksumAlgorithm, boolean didValidateChecksum, Throwable cause) {
S3FinishedResponseContext context = new S3FinishedResponseContext(errorCode, responseStatus, errorPayload, ChecksumAlgorithm.getEnumValueFromInteger(checksumAlgorithm), didValidateChecksum, cause);
this.responseHandler.onFinished(context);
}

Expand Down
10 changes: 10 additions & 0 deletions src/native/crt.c
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,16 @@ bool aws_jni_check_and_clear_exception(JNIEnv *env) {
return exception_pending;
}

bool aws_jni_get_and_clear_exception(JNIEnv *env, jthrowable *out) {
bool exception_pending = (*env)->ExceptionCheck(env);
if (exception_pending) {
(*env)->DeleteGlobalRef(env, *out);
waahm7 marked this conversation as resolved.
Show resolved Hide resolved
*out = (jthrowable)(*env)->NewGlobalRef(env, (*env)->ExceptionOccurred(env));
(*env)->ExceptionClear(env);
}
return exception_pending;
}

int aws_size_t_from_java(JNIEnv *env, size_t *out_size, jlong java_long, const char *errmsg_prefix) {
if (java_long < 0) {
aws_jni_throw_illegal_argument_exception(env, "%s cannot be negative", errmsg_prefix);
Expand Down
15 changes: 15 additions & 0 deletions src/native/crt.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,21 @@ void aws_jni_throw_illegal_argument_exception(JNIEnv *env, const char *msg, ...)
******************************************************************************/
bool aws_jni_check_and_clear_exception(JNIEnv *env);

/*******************************************************************************
* Checks whether or not an exception is pending on the stack.
* If the exception is pending, deletes existing global reference of `out`, sets `out` to the new exception and clears
* it.
*
* @param env A pointer to the JNI environment, used to interact with the JVM.
* @param out A pointer to a jthrowable object. If an exception is pending, the function
* deletes any existing global reference pointed to by 'out', and sets 'out'
* to point to the new exception. Must not be NULL.
*
* @return true if an exception was pending and has been cleared; false otherwise.
*
******************************************************************************/
bool aws_jni_get_and_clear_exception(JNIEnv *env, jthrowable *out);

/*******************************************************************************
* Set a size_t based on a jlong.
* If conversion fails, a java IllegalArgumentException is thrown like
Expand Down
2 changes: 1 addition & 1 deletion src/native/java_class_ids.c
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ static void s_cache_s3_meta_request_response_handler_native_adapter_properties(J
AWS_FATAL_ASSERT(s3_meta_request_response_handler_native_adapter_properties.onResponseBody);

s3_meta_request_response_handler_native_adapter_properties.onFinished =
(*env)->GetMethodID(env, cls, "onFinished", "(II[BIZ)V");
(*env)->GetMethodID(env, cls, "onFinished", "(II[BIZLjava/lang/Throwable;)V");
AWS_FATAL_ASSERT(s3_meta_request_response_handler_native_adapter_properties.onFinished);

s3_meta_request_response_handler_native_adapter_properties.onResponseHeaders =
Expand Down
15 changes: 11 additions & 4 deletions src/native/s3_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ struct s3_client_make_meta_request_callback_data {
jobject java_s3_meta_request_response_handler_native_adapter;
struct aws_input_stream *input_stream;
struct aws_signing_config_data signing_config_data;
jthrowable java_exception;
};

static void s_on_s3_client_shutdown_complete_callback(void *user_data);
Expand Down Expand Up @@ -354,10 +355,10 @@ static int s_on_s3_meta_request_body_callback(
range_start,
range_end);

if (aws_jni_check_and_clear_exception(env)) {
if (aws_jni_get_and_clear_exception(env, &(callback_data->java_exception))) {
AWS_LOGF_ERROR(
AWS_LS_S3_META_REQUEST,
"id=%p: Ignored Exception from S3MetaRequest.onResponseBody callback",
"id=%p: Received exception from S3MetaRequest.onResponseBody callback",
(void *)meta_request);
aws_raise_error(AWS_ERROR_HTTP_CALLBACK_FAILURE);
goto cleanup;
Expand Down Expand Up @@ -432,7 +433,7 @@ static int s_on_s3_meta_request_headers_callback(
response_status,
java_headers_buffer);

if (aws_jni_check_and_clear_exception(env)) {
if (aws_jni_get_and_clear_exception(env, &(callback_data->java_exception))) {
AWS_LOGF_ERROR(
AWS_LS_S3_META_REQUEST,
"id=%p: Exception thrown from S3MetaRequest.onResponseHeaders callback",
Expand Down Expand Up @@ -481,6 +482,10 @@ static void s_on_s3_meta_request_finish_callback(
error_response_cursor = aws_byte_cursor_from_buf(error_response_body);
}
jbyteArray jni_payload = aws_jni_byte_array_from_cursor(env, &error_response_cursor);
/* Only propagate java_exception if crt error code is callback failure */
jthrowable java_exception =
meta_request_result->error_code == AWS_ERROR_HTTP_CALLBACK_FAILURE ? callback_data->java_exception : NULL;

(*env)->CallVoidMethod(
env,
callback_data->java_s3_meta_request_response_handler_native_adapter,
Expand All @@ -489,7 +494,8 @@ static void s_on_s3_meta_request_finish_callback(
meta_request_result->response_status,
jni_payload,
meta_request_result->validation_algorithm,
meta_request_result->did_validate);
meta_request_result->did_validate,
java_exception);

if (aws_jni_check_and_clear_exception(env)) {
AWS_LOGF_ERROR(
Expand Down Expand Up @@ -571,6 +577,7 @@ static void s_s3_meta_request_callback_cleanup(
if (callback_data) {
(*env)->DeleteGlobalRef(env, callback_data->java_s3_meta_request);
(*env)->DeleteGlobalRef(env, callback_data->java_s3_meta_request_response_handler_native_adapter);
(*env)->DeleteGlobalRef(env, callback_data->java_exception);
aws_signing_config_data_clean_up(&callback_data->signing_config_data, env);
aws_mem_release(aws_jni_get_allocator(), callback_data);
}
Expand Down
Loading
Loading