Skip to content

Commit

Permalink
S3ResponseError (#522)
Browse files Browse the repository at this point in the history
Issue:
The exceptions delivered from the S3Client were lacking information like the HTTP response code, headers, & body. You could get the extra information from the on_done callback, but it would be nice to get it from the exception, so you could use the request.finished_future instead of the on_done callback if you wanted.

Description of changes:
Add new error type: awscrt.s3.S3ResponseError, which inherits from awscrt.exceptions.AwsCrtError. If we have information about the failed request, then use an S3ResponseError instead.
  • Loading branch information
graebm authored Nov 10, 2023
1 parent d479a89 commit 2e4397c
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 10 deletions.
51 changes: 43 additions & 8 deletions awscrt/s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import awscrt.exceptions
from dataclasses import dataclass
import threading
from typing import Optional
from typing import List, Optional, Tuple
from enum import IntEnum


Expand Down Expand Up @@ -454,6 +454,35 @@ def cancel(self):
_awscrt.s3_meta_request_cancel(self)


class S3ResponseError(awscrt.exceptions.AwsCrtError):
'''
An error response from S3.
Subclasses :class:`awscrt.exceptions.AwsCrtError`.
Attributes:
status_code (int): HTTP response status code.
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.
code (int): CRT error code.
name (str): CRT error name.
message (str): CRT error message.
'''

def __init__(self, *,
code: int,
name: str,
message: str,
status_code: List[Tuple[str, str]] = None,
headers: List[Tuple[str, str]] = None,
body: Optional[bytes] = None):
super().__init__(code, name, message)
self.status_code = status_code
self.headers = headers
self.body = body


class _S3ClientCore:
'''
Private class to keep all the related Python object alive until C land clean up for S3Client
Expand Down Expand Up @@ -517,13 +546,19 @@ def _on_finish(self, error_code, status_code, error_headers, error_body):
error = None
if error_code:
error = awscrt.exceptions.from_code(error_code)
if error_body:
# TODO The error body is XML, will need to parse it to something prettier.
try:
extra_message = ". Body from error request is: " + str(error_body)
error.message = error.message + extra_message
except BaseException:
pass

# If the failure was due to a response, make it into an S3ResponseError.
# When failure is due to a response, its headers are always included.
if isinstance(error, awscrt.exceptions.AwsCrtError) \
and status_code is not None \
and error_headers is not None:
error = S3ResponseError(
code=error.code,
name=error.name,
message=error.message,
status_code=status_code,
headers=error_headers,
body=error_body)
self._finished_future.set_exception(error)
else:
self._finished_future.set_result(None)
Expand Down
2 changes: 1 addition & 1 deletion source/auth_credentials.c
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ static int s_credentials_provider_delegate_get_credentials(
PyGILState_Release(state);

if (!native_credentials) {
return aws_raise_error(AWS_ERROR_CRT_CALLBACK_EXCEPTION);
return aws_raise_error(AWS_AUTH_CREDENTIALS_PROVIDER_DELEGATE_FAILURE);
}

callback(native_credentials, AWS_ERROR_SUCCESS, callback_user_data);
Expand Down
2 changes: 2 additions & 0 deletions source/s3_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ PyObject *aws_py_s3_cross_process_lock_acquire(PyObject *self, PyObject *args) {
}

PyObject *aws_py_s3_cross_process_lock_release(PyObject *self, PyObject *args) {
(void)self;

PyObject *lock_capsule; /* O */

if (!PyArg_ParseTuple(args, "O", &lock_capsule)) {
Expand Down
8 changes: 7 additions & 1 deletion test/test_s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
S3ChecksumLocation,
S3Client,
S3RequestType,
S3ResponseError,
CrossProcessLock,
create_default_s3_signing_config,
)
Expand Down Expand Up @@ -288,6 +289,8 @@ def _validate_successful_response(self, is_put_object):
self.assertEqual(self.done_status_code, self.response_status_code,
"status-code from on_done doesn't match code from on_headers")
self.assertIsNone(self.done_error)
self.assertIsNone(self.done_error_headers)
self.assertIsNone(self.done_error_body)
headers = HttpHeaders(self.response_headers)
self.assertIsNone(headers.get("Content-Range"))
body_length = headers.get("Content-Length")
Expand Down Expand Up @@ -596,12 +599,15 @@ def test_multipart_upload_with_invalid_request(self):
self._test_s3_put_get_object(request, S3RequestType.PUT_OBJECT, "AWS_ERROR_S3_INVALID_RESPONSE_STATUS")

# check that data from on_done callback came through correctly
self.assertIsNotNone(self.done_error)
self.assertIsInstance(self.done_error, S3ResponseError)
self.assertEqual(self.done_status_code, 400)
self.assertEqual(self.done_error.status_code, 400)
self.assertIsNotNone(self.done_error_headers)
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.assertTrue(b"InvalidDigest" in self.done_error_body)
self.assertEqual(self.done_error_body, self.done_error.body)

put_body_stream.close()

Expand Down

0 comments on commit 2e4397c

Please sign in to comment.