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

Return None on NoSuchKey error from s3 to cache the file. #159

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

logart
Copy link

@logart logart commented Jul 24, 2023

Hi there,

we had an issue while using tc_aws in our thumbor 7.5.0 set up.

The issue is it fails with 500 when file is not in s3. However I've configured it to use HttpLoader to cache not existing files.

While investigating the issue I've found out that it fails with

  result = await self._fetch(self.context.request.image_url)
  File "/usr/local/lib/python3.11/site-packages/thumbor/handlers/__init__.py", line 212, in get_image
2023-07-20 14:35:27 botocore.hooks:DEBUG Event needs-retry.s3.GetObject: calling handler <bound method S3RegionRedirector.redirect_from_error of <botocore.utils.S3RegionRedirector object at 0x7f32752e1250>>
2023-07-20 14:35:27 thumbor:ERROR [BaseHandler] get_image failed for url `external_url`. error: `An error occurred (NoSuchKey) when calling the GetObject operation: Unknown` 
2023-07-20 14:35:27 botocore.retryhandler:DEBUG No retry needed.

and code is cathing the BotoCoreError.

I've assumed that something has changed in the error handling or some other issue occured in this code block.

What I did is try to also catch NoSuchKey error. This solved the issue and now my thumbor works fine.

I would like to contribute this to the upstream so anyone who can also experience this issue will benefit from the patch.

I am not a python guru so if there is anything I could fix about my code just let me know and I would happily do it.

Wish you a great day,
Artem

@r0bdiabl0
Copy link

I am having this issue as well. This patch solves this. Can we merge in please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants