Enable headers callback invocation for failed requests (#162)

* Enable headers callback invocation for failed requests

For Default meta requests, the response headers callback was called
only for successful responses.

This commit changes that behavior, so the headers callback is invoked
also on error responses to allow CRT customers to obtain
more information about the error.

* Fix race condition on DNS timeout

include/aws/s3/s3_client.h

@@ -24,20 +24,55 @@ struct aws_s3_request;
 struct aws_s3_meta_request;
 struct aws_s3_meta_request_result;
 
+/**
+ * A Meta Request represents a group of generated requests that are being done on behalf of the
+ * original request. For example, one large GetObject request can be transformed into a series
+ * of ranged GetObject requests that are executed in parallel to improve throughput.
+ *
+ * The aws_s3_meta_request_type is a hint of transformation to be applied.
+ */
 enum aws_s3_meta_request_type {
+
+    /**
+     * The Default meta request type sends any request to S3 as-is (with no transformation). For example,
+     * it can be used to pass a CreateBucket request.
+     */
     AWS_S3_META_REQUEST_TYPE_DEFAULT,
+
+    /**
+     * The GetObject request will be split into a series of ranged GetObject requests that are
+     * executed in parallel to improve throughput, when possible.
+     */
     AWS_S3_META_REQUEST_TYPE_GET_OBJECT,
+
+    /**
+     * The PutObject request will be split into MultiPart uploads that are executed in parallel
+     * to improve throughput, when possible.
+     */
     AWS_S3_META_REQUEST_TYPE_PUT_OBJECT,
 
     AWS_S3_META_REQUEST_TYPE_MAX,
 };
 
+/**
+ * Invoked to provide response headers received during execution of the meta request, both for
+ * success and error HTTP status codes.
+ *
+ * Return AWS_OP_SUCCESS to continue processing the request.
+ * Return AWS_OP_ERR to indicate failure and cancel the request.
+ */
 typedef int(aws_s3_meta_request_headers_callback_fn)(
     struct aws_s3_meta_request *meta_request,
     const struct aws_http_headers *headers,
     int response_status,
     void *user_data);
 
+/**
+ * Invoked to provide the request body as it is received.
+ *
+ * Return AWS_OP_SUCCESS to continue processing the request.
+ * Return AWS_OP_ERR to indicate failure and cancel the request.
+ */
 typedef int(aws_s3_meta_request_receive_body_callback_fn)(
     /* The meta request that the callback is being issued for. */
     struct aws_s3_meta_request *meta_request,
@@ -52,6 +87,9 @@ typedef int(aws_s3_meta_request_receive_body_callback_fn)(
     /* User data specified by aws_s3_meta_request_options.*/
     void *user_data);
 
+/**
+ * Invoked when the entire meta request execution is complete.
+ */
 typedef void(aws_s3_meta_request_finish_fn)(
     struct aws_s3_meta_request *meta_request,
     const struct aws_s3_meta_request_result *meta_request_result,
@@ -139,13 +177,23 @@ struct aws_s3_meta_request_options {
     /* User data for all callbacks. */
     void *user_data;
 
-    /* Callback for receiving incoming headers. */
+    /**
+     * Optional.
+     * Invoked to provide response headers received during execution of the meta request.
+     * See `aws_s3_meta_request_headers_callback_fn`.
+     */
     aws_s3_meta_request_headers_callback_fn *headers_callback;
 
-    /* Callback for incoming body data. */
+    /**
+     * Invoked to provide the request body as it is received.
+     * See `aws_s3_meta_request_receive_body_callback_fn`.
+     */
     aws_s3_meta_request_receive_body_callback_fn *body_callback;
 
-    /* Callback for when the meta request is completely finished. */
+    /**
+     * Invoked when the entire meta request execution is complete.
+     * See `aws_s3_meta_request_finish_fn`.
+     */
     aws_s3_meta_request_finish_fn *finish_callback;
 
     /* Callback for when the meta request has completely cleaned up. */

source/s3_auto_ranged_get.c

@@ -569,6 +569,7 @@ static void s_s3_auto_ranged_get_request_finished(
 
                 error_code = aws_last_error_or_unknown();
             }
+            meta_request->headers_callback = NULL;
 
             aws_http_headers_release(response_headers);
         }

source/s3_auto_ranged_put.c

@@ -612,6 +612,7 @@ static void s_s3_auto_ranged_put_request_finished(
 
                     error_code = aws_last_error_or_unknown();
                 }
+                meta_request->headers_callback = NULL;
 
                 aws_http_headers_release(final_response_headers);
             }

source/s3_client.c

@@ -1397,6 +1397,10 @@ static void s_s3_client_acquired_retry_token(
 
     AWS_ASSERT(client->vtable->acquire_http_connection);
 
+    /* client needs to be kept alive until s_s3_client_on_acquire_http_connection completes */
+    /* TODO: not a blocker, consider managing the life time of aws_s3_client from aws_s3_endpoint to simplify usage */
+    aws_s3_client_acquire(client);
+
     client->vtable->acquire_http_connection(
         endpoint->http_connection_manager, s_s3_client_on_acquire_http_connection, connection);
 
@@ -1444,16 +1448,19 @@ static void s_s3_client_on_acquire_http_connection(
 
     connection->http_connection = incoming_http_connection;
     aws_s3_meta_request_send_request(meta_request, connection);
+    aws_s3_client_release(client); /* kept since this callback was registered */
     return;
 
 error_retry:
 
     aws_s3_client_notify_connection_finished(client, connection, error_code, AWS_S3_CONNECTION_FINISH_CODE_RETRY);
+    aws_s3_client_release(client); /* kept since this callback was registered */
     return;
 
 error_fail:
 
     aws_s3_client_notify_connection_finished(client, connection, error_code, AWS_S3_CONNECTION_FINISH_CODE_FAILED);
+    aws_s3_client_release(client); /* kept since this callback was registered */
 }
 
 /* Called by aws_s3_meta_request when it has finished using this connection for a single request. */

source/s3_default_meta_request.c

@@ -255,13 +255,17 @@ static void s_s3_meta_request_default_request_finished(
     AWS_PRECONDITION(meta_request_default);
 
     if (error_code == AWS_ERROR_SUCCESS && meta_request->headers_callback != NULL &&
-        meta_request->headers_callback(
-            meta_request,
-            request->send_data.response_headers,
-            request->send_data.response_status,
-            meta_request->user_data)) {
+        request->send_data.response_headers != NULL) {
+
+        if (meta_request->headers_callback(
+                meta_request,
+                request->send_data.response_headers,
+                request->send_data.response_status,
+                meta_request->user_data)) {
+            error_code = aws_last_error_or_unknown();
+        }
 
-        error_code = aws_last_error_or_unknown();
+        meta_request->headers_callback = NULL;
     }
 
     aws_s3_meta_request_lock_synced_data(meta_request);

source/s3_meta_request.c

@@ -1145,6 +1145,17 @@ void aws_s3_meta_request_finish_default(struct aws_s3_meta_request *meta_request
         aws_s3_request_release(release_request);
     }
 
+    if (meta_request->headers_callback && finish_result.error_response_headers) {
+        if (meta_request->headers_callback(
+                meta_request,
+                finish_result.error_response_headers,
+                finish_result.response_status,
+                meta_request->user_data)) {
+            finish_result.error_code = aws_last_error_or_unknown();
+        }
+        meta_request->headers_callback = NULL;
+    }
+
     AWS_LOGF_DEBUG(
         AWS_LS_S3_META_REQUEST,
         "id=%p Meta request finished with error code %d (%s)",

tests/CMakeLists.txt

@@ -98,6 +98,11 @@ add_net_test_case(test_s3_put_object_fail_body_callback)
 add_net_test_case(test_s3_get_object_fail_headers_callback)
 add_net_test_case(test_s3_get_object_fail_body_callback)
 add_net_test_case(test_s3_default_fail_headers_callback)
+add_net_test_case(test_s3_default_invoke_headers_callback_on_error)
+add_net_test_case(test_s3_default_invoke_headers_callback_cancels_on_error)
+add_net_test_case(test_s3_get_object_invoke_headers_callback_on_error)
+add_net_test_case(test_s3_put_object_invoke_headers_callback_on_error)
+add_net_test_case(test_s3_put_object_invoke_headers_callback_on_error_with_user_cancellation)
 add_net_test_case(test_s3_default_fail_body_callback)
 add_net_test_case(test_s3_error_missing_file)
 add_net_test_case(test_s3_existing_host_entry)