fix: http clients detect 4xx as errors by default by vdusek · Pull Request #498 · apify/crawlee-python

vdusek · 2024-09-04T14:04:44Z

Description

Error codes between 400 and 599 are considered as errors by default.
Before this change only codes between 500 and 599 were handled by default.

Issues

Closes: Throw an exception when we receive a 4xx status code #496

Testing

I added a new test to test that 4xx status codes are not being retried.

Checklist

CI passed

github-actions

⚠️ Pull Request Tookit has failed!

github-actions

⚠️ Pull Request Tookit has failed!

github-actions

⚠️ Pull Request Tookit has failed!

janbuchar · 2024-09-04T14:41:55Z

I haven't read the code yet, but does it retry on server error and fail immediately on client error?

src/crawlee/http_clients/_base.py

vdusek · 2024-09-05T09:53:13Z

No, we do retry for all error codes.

github-actions

⚠️ Pull Request Tookit has failed!

github-actions

⚠️ Pull Request Tookit has failed!

janbuchar · 2024-09-05T10:26:21Z

Well, I tried running JS crawlee against httpbin/status/{4xx, 5xx} with CheerioCrawler, this is what I got:

INFO  CheerioCrawler: Starting the crawler.
WARN  CheerioCrawler: Reclaiming failed request back to the list or queue. 533 - Internal Server Error:
 {"id":"w6oAmtuwtHhtL97","url":"https://httpbin.org/status/533","retryCount":1}
ERROR CheerioCrawler: Request failed and reached maximum retries. Error: Resource https://httpbin.org/status/406 is not available in the format requested by the Accept header. Skipping resource.
    at CheerioCrawler._abortDownloadOfBody (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@crawlee/src/internals/http-crawler.ts:894:19)
    at HttpCrawler.postNavigationHooks (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@crawlee/src/internals/http-crawler.ts:447:45)
    at CheerioCrawler._executeHooks (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@crawlee/src/internals/basic-crawler.ts:1599:23)
    at CheerioCrawler._handleNavigation (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@crawlee/src/internals/http-crawler.ts:625:20)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
    at async CheerioCrawler._runRequestHandler (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@crawlee/src/internals/http-crawler.ts:509:13)
    at async CheerioCrawler._runRequestHandler (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@crawlee/src/internals/cheerio-crawler.ts:235:9)
    at async wrap (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@apify/src/index.ts:77:27) {"id":"S2gfgn5uedS6zfo","url":"https://httpbin.org/status/406","method":"GET","uniqueKey":"https://httpbin.org/status/406"}
WARN  CheerioCrawler: Reclaiming failed request back to the list or queue. 533 - Internal Server Error:
 {"id":"w6oAmtuwtHhtL97","url":"https://httpbin.org/status/533","retryCount":2}
WARN  CheerioCrawler: Reclaiming failed request back to the list or queue. 533 - Internal Server Error:
 {"id":"w6oAmtuwtHhtL97","url":"https://httpbin.org/status/533","retryCount":3}
ERROR CheerioCrawler: Request failed and reached maximum retries. Error: 533 - Internal Server Error:
    at CheerioCrawler._parseResponse (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@crawlee/src/internals/http-crawler.ts:753:19)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
    at async CheerioCrawler._runRequestHandler (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@crawlee/src/internals/http-crawler.ts:512:28)
    at async CheerioCrawler._runRequestHandler (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@crawlee/src/internals/cheerio-crawler.ts:235:9)
    at async wrap (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@apify/src/index.ts:77:27) {"id":"w6oAmtuwtHhtL97","url":"https://httpbin.org/status/533","method":"GET","uniqueKey":"https://httpbin.org/status/533"}
INFO  CheerioCrawler: All requests from the queue have been processed, the crawler will shut down.
INFO  CheerioCrawler: Final request statistics: {"requestsFinished":0,"requestsFailed":2,"retryHistogram":[1,null,null,1],"requestAvgFailedDurationMillis":442,"requestAvgFinishedDurationMillis":null,"requestsFinishedPerMinute":0,"requestsFailedPerMinute":103,"requestTotalDurationMillis":883,"requestsTotal":2,"crawlerRuntimeMillis":1155}
INFO  CheerioCrawler: Error analysis: {"totalErrors":2,"uniqueErrors":2,"mostCommonErrors":["1x: Resource https://httpbin.org/status/406 is not available in the format requested by the Accept header. Skipping resource. (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@crawlee/src/internals/http-crawler.ts:894:19)","1x: 533 - Internal Server Error: (/home/janbuchar/Projekty/Apify/repro-mtggoldfish-js/node_modules/@crawlee/src/internals/http-crawler.ts:753:19)"]}
INFO  CheerioCrawler: Finished! Total 2 requests: 0 succeeded, 2 failed. {"terminal":true}

We should keep the parity here...

B4nan · 2024-09-05T10:37:23Z

we also have some special handling for blocked status codes like 403 when it comes to sessions (and this needs to be configurable)

https://github.com/search?q=repo%3Aapify%2Fcrawlee%20blocked%20status%20codes&type=code

github-actions

⚠️ Pull Request Tookit has failed!

github-actions

⚠️ Pull Request Tookit has failed!

github-actions

⚠️ Pull Request Tookit has failed!

vdusek · 2024-09-06T11:11:42Z

Update:

Do not retry for 4xx errors.
a bug fix in BasicCrawler.

AFAIK this is already implemented.

src/crawlee/errors.py

src/crawlee/http_clients/_base.py

janbuchar · 2024-09-06T12:05:25Z

src/crawlee/http_clients/_httpx.py

    This client uses the `HTTPX` library to perform HTTP requests in crawlers (`BasicCrawler` subclasses)
    and to manage sessions, proxies, and error handling.
+
+    The client raises an `HttpStatusCodeError` when it encounters an error response, defined by default as any


I think this should apply to any http client, right? Couldn't we just put this into the base client doc and link to it here?

It should, but the implementation of such behavior is in the specific HTTP clients, not the base. If someone creates their own HTTP client, they are not required to implement it in the same way. As a result, the docstring may not be true.

Valid point. In that case, we may want to pull this logic up into BaseHttpClient though. It seems unlikely you'd want to change this behavior, and at the same time, we don't want to make http-client-authors implement this over and over again.

src/crawlee/http_clients/_base.py

tests/unit/http_crawler/test_http_crawler.py

Co-authored-by: Jan Buchar <[email protected]>

github-actions bot assigned vdusek Sep 4, 2024

github-actions bot added this to the 97th sprint - Tooling team milestone Sep 4, 2024

github-actions bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Sep 4, 2024

github-actions bot reviewed Sep 4, 2024

View reviewed changes

github-actions bot added the tested Temporary label used only programatically for some analytics. label Sep 4, 2024

github-actions bot reviewed Sep 4, 2024

View reviewed changes

vdusek force-pushed the http-clients-detect-4xx branch from 9fbf479 to 600a05a Compare September 4, 2024 14:33

vdusek marked this pull request as ready for review September 4, 2024 14:34

vdusek requested a review from janbuchar September 4, 2024 14:34

github-actions bot reviewed Sep 4, 2024

View reviewed changes

B4nan reviewed Sep 4, 2024

View reviewed changes

src/crawlee/http_clients/_base.py Outdated Show resolved Hide resolved

github-actions bot reviewed Sep 5, 2024

View reviewed changes

vdusek added 2 commits September 6, 2024 12:54

fix: http clients detect 4xx as errors by default

5449cd3

Fix bug in basic crawler and do not retry on 4xx

38af191

vdusek force-pushed the http-clients-detect-4xx branch from 6c3e0a4 to 38af191 Compare September 6, 2024 10:55

github-actions bot reviewed Sep 6, 2024

View reviewed changes

vdusek marked this pull request as draft September 6, 2024 11:02

improve tests

898b436

vdusek force-pushed the http-clients-detect-4xx branch from e5d6893 to 898b436 Compare September 6, 2024 11:08

vdusek marked this pull request as ready for review September 6, 2024 11:09

github-actions bot reviewed Sep 6, 2024

View reviewed changes

vdusek requested a review from B4nan September 6, 2024 11:12

add missing exc constructors

7415a13

janbuchar reviewed Sep 6, 2024

View reviewed changes

vdusek and others added 2 commits September 6, 2024 14:51

Update tests/unit/http_crawler/test_http_crawler.py

4931a11

Co-authored-by: Jan Buchar <[email protected]>

based on reviews

6e3028f

vdusek requested a review from janbuchar September 6, 2024 13:00

based on review 2

73ef6a2

janbuchar approved these changes Sep 6, 2024

View reviewed changes

vdusek merged commit 1895dca into master Sep 6, 2024

vdusek deleted the http-clients-detect-4xx branch September 6, 2024 13:36

Mantisus mentioned this pull request Dec 13, 2024

fix: respect user configuration for work with status codes #812

Merged

Conversation

vdusek commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues

Testing

Checklist

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

janbuchar commented Sep 4, 2024

Uh oh!

Uh oh!

vdusek commented Sep 5, 2024

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

janbuchar commented Sep 5, 2024

Uh oh!

B4nan commented Sep 5, 2024

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

vdusek commented Sep 6, 2024

Uh oh!

Uh oh!

Uh oh!

janbuchar Sep 6, 2024

Choose a reason for hiding this comment

Uh oh!

vdusek Sep 6, 2024

Choose a reason for hiding this comment

Uh oh!

janbuchar Sep 6, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vdusek commented Sep 4, 2024 •

edited

Loading