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

Replace CurlError with HTTPClientError #1572

Merged
merged 2 commits into from Jun 20, 2023
Merged

Conversation

RaphaelVRossi
Copy link
Member

The following error:

2023-06-19 19:27:42 thumbor:ERROR ERROR: Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/thumbor/handlers/__init__.py", line 212, in get_image
    result = await self._fetch(self.context.request.image_url)
  File "/usr/local/lib/python3.9/dist-packages/thumbor/handlers/__init__.py", line 884, in _fetch
    loader_result = await self.context.modules.loader.load(
  File "/usr/local/lib/python3.9/dist-packages/thumbor/loaders/http_loader.py", line 202, in load
    response = await client.fetch(req, raise_error=True)
tornado.httpclient.HTTPClientError: HTTP 404: Not Found

Happens because HTTP_LOADER_CURL_ASYNC_HTTP_CLIENT is True and the upstream host returned a 404 status error.

The type of exc_type is tornado.httpclient.HTTPClientError and not the expected tornado.curl_httpclient.CurlError. (CurlError is an extension of HTTPError)

@coveralls
Copy link

coveralls commented Jun 19, 2023

Pull Request Test Coverage Report for Build 5326467908

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.006%) to 86.389%

Totals Coverage Status
Change from base Build 5324675497: -0.006%
Covered Lines: 3935
Relevant Lines: 4555

💛 - Coveralls

Copy link
Contributor

@devppjr devppjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✅

@sonarcloud
Copy link

sonarcloud bot commented Jun 20, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@RaphaelVRossi RaphaelVRossi merged commit 1bc4ed1 into master Jun 20, 2023
35 checks passed
@RaphaelVRossi RaphaelVRossi deleted the change-curl_error-type branch June 20, 2023 19:43
@scorphus
Copy link
Member

I'm late to the party but I'm asking anyway — I'm prolly missing something. Are we 100% sure tornado.curl_httpclient.CurlError is not raised in any circumstance?

@RaphaelVRossi
Copy link
Member Author

@scorphus Even this error happening, it will be caught by HTTPClientError block. Since CurlError extends from HTTPError (alias to HTTPClientError)

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.

None yet

5 participants