Add S3ResponseError.operation_name (#530)

awscrt/s3.py

@@ -192,7 +192,7 @@ class S3Client(NativeResource):
     def __init__(
             self,
             *,
-            bootstrap,
+            bootstrap=None,
             region,
             tls_mode=None,
             signing_config=None,
@@ -264,6 +264,7 @@ def make_request(
             *,
             type,
             request,
+            operation_name=None,
             recv_filepath=None,
             send_filepath=None,
             signing_config=None,
@@ -284,6 +285,11 @@ def make_request(
             request (HttpRequest): The overall outgoing API request for S3 operation.
                 If the request body is a file, set send_filepath for better performance.
 
+            operation_name(Optional[str]): Optional S3 operation name (e.g. "CreateBucket").
+                This will only be used when `type` is :attr:`~S3RequestType.DEFAULT`;
+                it is automatically populated for other types.
+                This name is used to fill out details in metrics and error reports.
+
             recv_filepath (Optional[str]): Optional file path. If set, the
                 response body is written directly to a file and the
                 `on_body` callback is not invoked. This should give better
@@ -342,6 +348,16 @@ def make_request(
                         side sent an unsuccessful response, the body of the response is
                         provided here. Else None will be returned.
 
+                    *   `error_operation_name` (Optional[str]): If request failed
+                        because server side sent and unsuccessful response, this
+                        is the name of the S3 operation it was responding to.
+                        For example, if a :attr:`~S3RequestType.PUT_OBJECT` fails
+                        this could be "PutObject", "CreateMultipartUpload", "UploadPart",
+                        "CompleteMultipartUpload", or others. For :attr:`~S3RequestType.DEFAULT`,
+                        this is the `operation_name` passed to :meth:`S3Client.make_request()`.
+                        This will be None if the request failed for another reason,
+                        or the S3 operation name is unknown.
+
                     *   `status_code` (Optional[int]): HTTP response status code (if available).
                         If request failed because server side sent an unsuccessful response,
                         this is its status code. If the operation was successful,
@@ -364,6 +380,7 @@ def make_request(
             client=self,
             type=type,
             request=request,
+            operation_name=operation_name,
             recv_filepath=recv_filepath,
             send_filepath=send_filepath,
             signing_config=signing_config,
@@ -398,6 +415,7 @@ def __init__(
             client,
             type,
             request,
+            operation_name=None,
             recv_filepath=None,
             send_filepath=None,
             signing_config=None,
@@ -445,6 +463,7 @@ def __init__(
             client,
             request,
             type,
+            operation_name,
             signing_config,
             credential_provider,
             recv_filepath,
@@ -474,6 +493,12 @@ class S3ResponseError(awscrt.exceptions.AwsCrtError):
         headers (list[tuple[str, str]]): Headers from HTTP response.
         body (Optional[bytes]): Body of HTTP response (if any).
             This is usually XML. It may be None in the case of a HEAD response.
+        operation_name (Optional[str]): Name of the S3 operation that failed (if known).
+            For example, if a :attr:`~S3RequestType.PUT_OBJECT` fails
+            this could be "PutObject", "CreateMultipartUpload", "UploadPart",
+            "CompleteMultipartUpload", or others. For :attr:`~S3RequestType.DEFAULT`,
+            this is the `operation_name` passed to :meth:`S3Client.make_request()`.
+            If the S3 operation name is unknown, this will be None.
         code (int): CRT error code.
         name (str): CRT error name.
         message (str): CRT error message.
@@ -485,11 +510,13 @@ def __init__(self, *,
                  message: str,
                  status_code: List[Tuple[str, str]] = None,
                  headers: List[Tuple[str, str]] = None,
-                 body: Optional[bytes] = None):
+                 body: Optional[bytes] = None,
+                 operation_name: Optional[str] = None):
         super().__init__(code, name, message)
         self.status_code = status_code
         self.headers = headers
         self.body = body
+        self.operation_name = operation_name
 
 
 class _S3ClientCore:
@@ -559,7 +586,7 @@ def _on_body(self, chunk, offset):
     def _on_shutdown(self):
         self._shutdown_event.set()
 
-    def _on_finish(self, error_code, status_code, error_headers, error_body):
+    def _on_finish(self, error_code, status_code, error_headers, error_body, error_operation_name):
         # If C layer gives status_code 0, that means "unknown"
         if status_code == 0:
             status_code = None
@@ -582,12 +609,18 @@ def _on_finish(self, error_code, status_code, error_headers, error_body):
                         message=error.message,
                         status_code=status_code,
                         headers=error_headers,
-                        body=error_body)
+                        body=error_body,
+                        operation_name=error_operation_name)
             self._finished_future.set_exception(error)
         else:
             self._finished_future.set_result(None)
         if self._on_done_cb:
-            self._on_done_cb(error=error, error_headers=error_headers, error_body=error_body, status_code=status_code)
+            self._on_done_cb(
+                error=error,
+                error_headers=error_headers,
+                error_body=error_body,
+                error_operation_name=error_operation_name,
+                status_code=status_code)
 
     def _on_progress(self, progress):
         if self._on_progress_cb:

source/s3_meta_request.c

@@ -254,15 +254,22 @@ static void s_s3_request_on_finish(
     if (meta_request_result->error_response_body) {
         error_body = *(meta_request_result->error_response_body);
     }
+
+    const char *operation_name = NULL;
+    if (meta_request_result->error_response_operation_name != NULL) {
+        operation_name = aws_string_c_str(meta_request_result->error_response_operation_name);
+    }
+
     result = PyObject_CallMethod(
         request_binding->py_core,
         "_on_finish",
-        "(iiOy#)",
+        "(iiOy#s)",
         error_code,
         meta_request_result->response_status,
         header_list ? header_list : Py_None,
         (const char *)(error_body.buffer),
-        (Py_ssize_t)error_body.len);
+        (Py_ssize_t)error_body.len,
+        operation_name);
 
     if (result) {
         Py_DECREF(result);
@@ -356,6 +363,7 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) {
     PyObject *s3_client_py;                            /* O */
     PyObject *http_request_py;                         /* O */
     int type;                                          /* i */
+    const char *operation_name;                        /* z */
     PyObject *signing_config_py;                       /* O */
     PyObject *credential_provider_py;                  /* O */
     const char *recv_filepath;                         /* z */
@@ -367,11 +375,12 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) {
     PyObject *py_core;                                 /* O */
     if (!PyArg_ParseTuple(
             args,
-            "OOOiOOzzs#iipO",
+            "OOOizOOzzs#iipO",
             &py_s3_request,
             &s3_client_py,
             &http_request_py,
             &type,
+            &operation_name,
             &signing_config_py,
             &credential_provider_py,
             &recv_filepath,
@@ -451,6 +460,7 @@ PyObject *aws_py_s3_client_make_meta_request(PyObject *self, PyObject *args) {
 
     struct aws_s3_meta_request_options s3_meta_request_opt = {
         .type = type,
+        .operation_name = aws_byte_cursor_from_c_str(operation_name),
         .message = http_request,
         .signing_config = signing_config,
         .checksum_config = &checksum_config,

test/test_s3.py

@@ -248,6 +248,7 @@ def setUp(self):
         self.done_status_code = None
         self.done_error_headers = None
         self.done_error_body = None
+        self.done_error_operation_name = None
 
         self.files = FileCreator()
         self.temp_put_obj_file_path = self.files.create_file_with_size("temp_put_obj_10mb", 10 * MB)
@@ -282,10 +283,11 @@ def _on_request_headers(self, status_code, headers, **kargs):
     def _on_request_body(self, chunk, offset, **kargs):
         self.received_body_len = self.received_body_len + len(chunk)
 
-    def _on_request_done(self, error, error_headers, error_body, status_code, **kwargs):
+    def _on_request_done(self, error, error_headers, error_body, error_operation_name, status_code, **kwargs):
         self.done_error = error
         self.done_error_headers = error_headers
         self.done_error_body = error_body
+        self.done_error_operation_name = error_operation_name
         self.done_status_code = status_code
 
     def _on_progress(self, progress):
@@ -298,6 +300,7 @@ def _validate_successful_response(self, is_put_object):
         self.assertIsNone(self.done_error)
         self.assertIsNone(self.done_error_headers)
         self.assertIsNone(self.done_error_body)
+        self.assertIsNone(self.done_error_operation_name)
         headers = HttpHeaders(self.response_headers)
         self.assertIsNone(headers.get("Content-Range"))
         body_length = headers.get("Content-Length")
@@ -607,7 +610,7 @@ def test_multipart_put_object_cancel(self):
     def test_put_object_quick_cancel(self):
         return self._put_object_cancel_helper(False)
 
-    def test_multipart_upload_with_invalid_request(self):
+    def test_singlepart_upload_with_invalid_request(self):
         # send upload with incorrect Content-MD5
         # need to do single-part upload so the Content-MD5 header is sent along as-is.
         content_length = 100
@@ -625,11 +628,31 @@ def test_multipart_upload_with_invalid_request(self):
         self.assertTrue(any(h[0].lower() == 'x-amz-request-id' for h in self.done_error_headers))
         self.assertListEqual(self.done_error_headers, self.done_error.headers)
         self.assertIsNotNone(self.done_error_body)
+        self.assertEqual(self.done_error_operation_name, "PutObject")
+        self.assertEqual(self.done_error_operation_name, self.done_error.operation_name)
         self.assertTrue(b"InvalidDigest" in self.done_error_body)
         self.assertEqual(self.done_error_body, self.done_error.body)
 
         put_body_stream.close()
 
+    def test_default_request_failure(self):
+        # send invalid DEFAULT S3Request
+        # ensure error info (including custom operation_name) comes through correctly
+        s3_client = S3Client(region=self.region)
+
+        # send invalid request to S3.
+        http_request = HttpRequest(method="GET", path="/obviously-invalid-path-object-does-not-exist")
+        http_request.headers.add("host", self._build_endpoint_string(self.region, self.bucket_name))
+        http_request.headers.add("content-length", "0")
+        s3_request = s3_client.make_request(
+            type=S3RequestType.DEFAULT,
+            request=http_request,
+            operation_name="MyNewOperationName")
+
+        exception = s3_request.finished_future.exception(self.timeout)
+        self.assertIsInstance(exception, S3ResponseError)
+        self.assertEqual(exception.operation_name, "MyNewOperationName")
+
     def test_on_headers_callback_failure(self):
         def _explode(**kwargs):
             raise RuntimeError("Error in on_headers callback")