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

Inconsistent error handling in S3: old boto3.exceptions.S3UploadFailedError replaces modern botocore.exceptions.ClientError #4299

Open
1 task
glerb opened this issue Oct 11, 2024 · 8 comments
Assignees
Labels
bug This issue is a confirmed bug. p3 This is a minor priority issue s3

Comments

@glerb
Copy link
Contributor

glerb commented Oct 11, 2024

Describe the bug

The boto3 S3 client does not process S3 service errors like every other service client. It throws a different error than other service clients do when encountering AWS service errors.

This is a bump of #1986 from 2019(!).

Regression Issue

  • Select this option if this issue appears to be a regression.

Expected Behavior

A ClientError object should be returned to an except ClientError as error: clause when an S3 service error is returned to botocore, per current docs (which apparently haven't changed for years).

Current Behavior

The boto3 client raises a second error, S3UploadFailedError, from the boto3 exceptions when executing boto3.client('s3').upload_file():

$ python test_copy.py 
Traceback (most recent call last):
  File "<my_path>.venv/lib64/python3.9/site-packages/boto3/s3/transfer.py", line 372, in upload_file
    future.result()
  File "<my_path>.venv/lib64/python3.9/site-packages/s3transfer/futures.py", line 103, in result
    return self._coordinator.result()
  File "<my_path>.venv/lib64/python3.9/site-packages/s3transfer/futures.py", line 266, in result
    raise self._exception
  File "<my_path>.venv/lib64/python3.9/site-packages/s3transfer/tasks.py", line 139, in __call__
    return self._execute_main(kwargs)
  File "<my_path>.venv/lib64/python3.9/site-packages/s3transfer/tasks.py", line 162, in _execute_main
    return_value = self._main(**kwargs)
  File "<my_path>.venv/lib64/python3.9/site-packages/s3transfer/upload.py", line 764, in _main
    client.put_object(Bucket=bucket, Key=key, Body=body, **extra_args)
  File "<my_path>.venv/lib64/python3.9/site-packages/botocore/client.py", line 569, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "<my_path>.venv/lib64/python3.9/site-packages/botocore/client.py", line 1023, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (InvalidRequest) when calling the PutObject operation: Content-MD5 OR x-amz-checksum- HTTP header is required for Put Object requests with Object Lock parameters

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<my_path>test_copy.py", line 15, in <module>
    response = s3.upload_file(
  File "<my_path>.venv/lib64/python3.9/site-packages/boto3/s3/inject.py", line 145, in upload_file
    return transfer.upload_file(
  File "<my_path>.venv/lib64/python3.9/site-packages/boto3/s3/transfer.py", line 378, in upload_file
    raise S3UploadFailedError(
boto3.exceptions.S3UploadFailedError: Failed to upload ./test.txt to <my_bucket>/./test.txt: An error occurred (InvalidRequest) when calling the PutObject operation: Content-MD5 OR x-amz-checksum- HTTP header is required for Put Object requests with Object Lock parameters

Reproduction Steps

import boto3
import botocore.exceptions

BUCKET_NAME = '<bucket_name>'
PROFILE_NAME = '<profile_name>'
REGION_NAME = '<region>'
FILE_NAME = './test.txt'

boto3.setup_default_session(profile_name=PROFILE_NAME, region_name=REGION_NAME)
s3 = boto3.client('s3')

try:
    response = s3.upload_file(
        Filename=FILE_NAME,
        Bucket=BUCKET_NAME,
        Key=FILE_NAME
    )
    print(response)
except botocore.exceptions.ClientError as error:
    print(f"exiting: S3 error uploading file to s3://{BUCKET_NAME}: "
          f"{error.response['Error']['Code']}: "
          f"{error.response['Error']['Message']}"
          )

Possible Solution

The reason this is a "bug" is that there is some "backwards compatibility" error handler in boto3/s3/transfer.py that raises the (old?) S3UploadFailedError when the (modern?) botocore/client.py raises a ClientError. Thus, the S3 client does not behave as the current error handling instructions suggest, and there is no additional information in any of the documentation about this. Additionally, this old error handler does not return the full response dictionary in a way that is expected by developers catching a ClientError. Instead, the original error is returned as its __str__/__repr__ by the S3UploadFailedError. Developers have to go spelunking through the library to discover this.

Additional Information/Context

No response

SDK version used

1.35.25

Environment details (OS name and version, etc.)

Linux 5.15.0-202.135.2.el9uek.x86_64 #2 SMP x86_64 x86_64 x86_64 GNU/Linux

@glerb glerb added bug This issue is a confirmed bug. needs-triage This issue or PR still needs to be triaged. labels Oct 11, 2024
@adev-code adev-code self-assigned this Oct 24, 2024
@adev-code adev-code added s3 investigating This issue is being investigated and/or work is in progress to resolve the issue. p3 This is a minor priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Oct 24, 2024
@adev-code
Copy link

Hi @glerb, thanks for reaching out. I have used the code that you have provided and I was not able to replicate the errors. For a further look, please provide the full debug logs by adding boto3.set_stream_logger('') to your code and redacting any sensitive information. Also, please let me know the type of file you were uploading, content type, and its size. Thanks.

@adev-code adev-code added response-requested Waiting on additional information or feedback. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Oct 25, 2024
@glerb
Copy link
Contributor Author

glerb commented Oct 25, 2024

@adev-code the error I used above was just an example intentionally designed to create an error (in this case, trying to upload a file to a bucket with Object Lock enabled, but without specifying a checksum algorithm). The specific error is almost certainly irrelevant. You can probably cause an error any way you want. The issue I'm flagging is the handling of errors generally.

@adev-code adev-code added needs-review and removed response-requested Waiting on additional information or feedback. labels Oct 25, 2024
@adev-code
Copy link

Hi @glerb, thanks for the update. Boto3 is throwing a ClientError as documented, and it's throwing the S3UploadFailedError to provide additional information. This behavior is expected; please let me know if you have any follow-up questions. Thanks!

@adev-code adev-code added response-requested Waiting on additional information or feedback. and removed needs-review labels Oct 29, 2024
@glerb
Copy link
Contributor Author

glerb commented Oct 29, 2024

Boto3 is throwing a ClientError as documented, and it's throwing the S3UploadFailedError to provide additional information

It isn't, though. Or rather, the ClientError is being handled by a handler inside boto3 that then raises the "old" S3UploadFailedError:

https://github.com/boto/boto3/blob/develop/boto3/s3/transfer.py#L377

Because e in L380 is inside a format(), the ClientError dict is lost, and only the __str__ of the error is available for upstream error handlers.

@github-actions github-actions bot removed the response-requested Waiting on additional information or feedback. label Oct 30, 2024
@nateprewitt
Copy link
Contributor

Hi @glerb, the error being raised is what many users expect currently although I understand the confusion caused by the asymmetry. The underlying issue is upload*() and download*() operations on the S3 client are not actually S3 operations. They're higher level abstractions through our s3transfer client to simplify more complex workflows.

Changing the more specialized exception to a ClientError is going to be a breaking change. What we could look into doing here is raising the S3UploadFailedError from the ClientError instead of simply copying the message. That will give you access to the underlying ClientError via Python's __cause__ dunder allowing access to the originating exception. Would that be a reasonable alternative to get the response information?

We can also look at making this caveat clearer in the specific S3transfer operations attached to the S3 client so there is less surprise when this get raised.

@glerb
Copy link
Contributor Author

glerb commented Nov 1, 2024

Sounds good. That should be mentioned prominently in the docs, though, preferably in the form of an example. That is something the average SDK user is not going to be able to figure out on their own. Another way of doing it without any changes being necessary in boto3 might be using the traceback lib but again that is beyond the average Python user (and I don't know how to do it off the top of my head).

Would another solution be setting S3UploadError to also inherit from ClientError?

@nateprewitt
Copy link
Contributor

Would another solution be setting S3UploadError to also inherit from ClientError?

That will unfortunately have the same breaking behaviors as anything currently catching ClientError before this will potentially start handling it differently. ClientError (despite it's naming) is specifically for errors returned from a service and while it's coming from a ClientError, it isn't one itself.

Another way of doing it without any changes being necessary in boto3 might be using the traceback lib but again that is beyond the average Python user (and I don't know how to do it off the top of my head).

Yep, that's definitely an option, but I agree it's not something all Python devs are going to be comfortable with. The _cause_ dunder is a middle ground so you can access a variable, without needing extra tooling. Docs will be important to making sure this is easy to handle and will go on the 4 operations that have this quirk.

We'll work on drafting a PR for that up and revisit this once it's ready for review.

@adev-code adev-code added the response-requested Waiting on additional information or feedback. label Nov 8, 2024
@glerb
Copy link
Contributor Author

glerb commented Nov 13, 2024

Something to get you started: #4346

@github-actions github-actions bot removed the response-requested Waiting on additional information or feedback. label Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a confirmed bug. p3 This is a minor priority issue s3
Projects
None yet
Development

No branches or pull requests

3 participants