From 80f5003c88f8ee4c5529c1a6d7fc3981efef511d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 22 Feb 2021 16:38:38 +0100 Subject: [PATCH] Add tests for get_retry_request --- scrapy/downloadermiddlewares/retry.py | 13 +- tests/test_downloadermiddleware_retry.py | 498 ++++++++++++++++++++--- 2 files changed, 459 insertions(+), 52 deletions(-) diff --git a/scrapy/downloadermiddlewares/retry.py b/scrapy/downloadermiddlewares/retry.py index 046e3ea71..0f24e5d28 100644 --- a/scrapy/downloadermiddlewares/retry.py +++ b/scrapy/downloadermiddlewares/retry.py @@ -10,6 +10,7 @@ Failed pages are collected on the scraping process and rescheduled at the end, once the spider has finished crawling all regular (non failed) pages. """ import logging +from inspect import isclass from twisted.internet import defer from twisted.internet.error import ( @@ -81,8 +82,8 @@ def get_retry_request( retry_times = request.meta.get('retry_times', 0) + 1 if max_retry_times is None: max_retry_times = request.meta.get('max_retry_times') - if max_retry_times is None: - max_retry_times = settings.getint('RETRY_TIMES') + if max_retry_times is None: + max_retry_times = settings.getint('RETRY_TIMES') if retry_times <= max_retry_times: logger.debug( "Retrying %(request)s (failed %(retry_times)d times): %(reason)s", @@ -96,6 +97,8 @@ def get_retry_request( priority_adjust = settings.getint('RETRY_PRIORITY_ADJUST') new_request.priority = request.priority + priority_adjust + if isclass(reason): + reason = reason() if isinstance(reason, Exception): reason = global_object_name(reason.__class__) @@ -149,10 +152,12 @@ class RetryMiddleware: return self._retry(request, exception, spider) def _retry(self, request, reason, spider): + max_retry_times = request.meta.get('max_retry_times', self.max_retry_times) + priority_adjust = request.meta.get('priority_adjust', self.priority_adjust) return get_retry_request( request, reason=reason, spider=spider, - max_retry_times=self.max_retry_times, - priority_adjust=self.priority_adjust, + max_retry_times=max_retry_times, + priority_adjust=priority_adjust, ) diff --git a/tests/test_downloadermiddleware_retry.py b/tests/test_downloadermiddleware_retry.py index 364ce0c89..cf01a7dff 100644 --- a/tests/test_downloadermiddleware_retry.py +++ b/tests/test_downloadermiddleware_retry.py @@ -4,18 +4,22 @@ from twisted.internet.error import ( ConnectError, ConnectionDone, ConnectionLost, - ConnectionRefusedError, DNSLookupError, TCPTimedOutError, - TimeoutError, ) from twisted.web.client import ResponseFailed -from scrapy.downloadermiddlewares.retry import RetryMiddleware -from scrapy.spiders import Spider +from scrapy.downloadermiddlewares.retry import ( + get_retry_request, + RetryMiddleware, +) +from scrapy.exceptions import IgnoreRequest from scrapy.http import Request, Response +from scrapy.spiders import Spider from scrapy.utils.test import get_crawler +from testfixtures import LogCapture + class RetryTest(unittest.TestCase): def setUp(self): @@ -119,82 +123,480 @@ class RetryTest(unittest.TestCase): class MaxRetryTimesTest(unittest.TestCase): - def setUp(self): - self.crawler = get_crawler(Spider) - self.spider = self.crawler._create_spider('foo') - self.mw = RetryMiddleware.from_crawler(self.crawler) - self.mw.max_retry_times = 2 - self.invalid_url = 'http://www.scrapytest.org/invalid_url' + + invalid_url = 'http://www.scrapytest.org/invalid_url' + + def get_spider_and_middleware(self, settings=None): + crawler = get_crawler(Spider, settings or {}) + spider = crawler._create_spider('foo') + middleware = RetryMiddleware.from_crawler(crawler) + return spider, middleware def test_with_settings_zero(self): - - # SETTINGS: RETRY_TIMES = 0 - self.mw.max_retry_times = 0 - + max_retry_times = 0 + settings = {'RETRY_TIMES': max_retry_times} + spider, middleware = self.get_spider_and_middleware(settings) req = Request(self.invalid_url) - self._test_retry(req, DNSLookupError('foo'), self.mw.max_retry_times) + self._test_retry( + req, + DNSLookupError('foo'), + max_retry_times, + spider=spider, + middleware=middleware, + ) def test_with_metakey_zero(self): - - # SETTINGS: meta(max_retry_times) = 0 - meta_max_retry_times = 0 - - req = Request(self.invalid_url, meta={'max_retry_times': meta_max_retry_times}) - self._test_retry(req, DNSLookupError('foo'), meta_max_retry_times) + max_retry_times = 0 + spider, middleware = self.get_spider_and_middleware() + meta = {'max_retry_times': max_retry_times} + req = Request(self.invalid_url, meta=meta) + self._test_retry( + req, + DNSLookupError('foo'), + max_retry_times, + spider=spider, + middleware=middleware, + ) def test_without_metakey(self): - - # SETTINGS: RETRY_TIMES is NON-ZERO - self.mw.max_retry_times = 5 - + max_retry_times = 5 + settings = {'RETRY_TIMES': max_retry_times} + spider, middleware = self.get_spider_and_middleware(settings) req = Request(self.invalid_url) - self._test_retry(req, DNSLookupError('foo'), self.mw.max_retry_times) + self._test_retry( + req, + DNSLookupError('foo'), + max_retry_times, + spider=spider, + middleware=middleware, + ) def test_with_metakey_greater(self): - - # SETINGS: RETRY_TIMES < meta(max_retry_times) - self.mw.max_retry_times = 2 meta_max_retry_times = 3 + middleware_max_retry_times = 2 req1 = Request(self.invalid_url, meta={'max_retry_times': meta_max_retry_times}) req2 = Request(self.invalid_url) - self._test_retry(req1, DNSLookupError('foo'), meta_max_retry_times) - self._test_retry(req2, DNSLookupError('foo'), self.mw.max_retry_times) + settings = {'RETRY_TIMES': middleware_max_retry_times} + spider, middleware = self.get_spider_and_middleware(settings) + + self._test_retry( + req1, + DNSLookupError('foo'), + meta_max_retry_times, + spider=spider, + middleware=middleware, + ) + self._test_retry( + req2, + DNSLookupError('foo'), + middleware_max_retry_times, + spider=spider, + middleware=middleware, + ) def test_with_metakey_lesser(self): - - # SETINGS: RETRY_TIMES > meta(max_retry_times) - self.mw.max_retry_times = 5 meta_max_retry_times = 4 + middleware_max_retry_times = 5 req1 = Request(self.invalid_url, meta={'max_retry_times': meta_max_retry_times}) req2 = Request(self.invalid_url) - self._test_retry(req1, DNSLookupError('foo'), meta_max_retry_times) - self._test_retry(req2, DNSLookupError('foo'), self.mw.max_retry_times) + settings = {'RETRY_TIMES': middleware_max_retry_times} + spider, middleware = self.get_spider_and_middleware(settings) + + self._test_retry( + req1, + DNSLookupError('foo'), + meta_max_retry_times, + spider=spider, + middleware=middleware, + ) + self._test_retry( + req2, + DNSLookupError('foo'), + middleware_max_retry_times, + spider=spider, + middleware=middleware, + ) def test_with_dont_retry(self): + max_retry_times = 4 + spider, middleware = self.get_spider_and_middleware() + meta = { + 'max_retry_times': max_retry_times, + 'dont_retry': True, + } + req = Request(self.invalid_url, meta=meta) + self._test_retry( + req, + DNSLookupError('foo'), + 0, + spider=spider, + middleware=middleware, + ) - # SETTINGS: meta(max_retry_times) = 4 - meta_max_retry_times = 4 - - req = Request(self.invalid_url, meta={ - 'max_retry_times': meta_max_retry_times, 'dont_retry': True - }) - - self._test_retry(req, DNSLookupError('foo'), 0) - - def _test_retry(self, req, exception, max_retry_times): + def _test_retry( + self, + req, + exception, + max_retry_times, + spider=None, + middleware=None, + ): + spider = spider or self.spider + middleware = middleware or self.mw for i in range(0, max_retry_times): - req = self.mw.process_exception(req, exception, self.spider) + req = middleware.process_exception(req, exception, spider) assert isinstance(req, Request) # discard it - req = self.mw.process_exception(req, exception, self.spider) + req = middleware.process_exception(req, exception, spider) self.assertEqual(req, None) +class GetRetryRequestTest(unittest.TestCase): + + def get_spider(self, settings=None): + crawler = get_crawler(Spider, settings or {}) + return crawler._create_spider('foo') + + def test_basic_usage(self): + request = Request('https://example.com') + spider = self.get_spider() + with LogCapture() as log: + new_request = get_retry_request( + request, + spider=spider, + ) + self.assertIsInstance(new_request, Request) + self.assertNotEqual(new_request, request) + self.assertEqual(new_request.dont_filter, True) + expected_retry_times = 1 + self.assertEqual(new_request.meta['retry_times'], expected_retry_times) + self.assertEqual(new_request.priority, -1) + expected_reason = "unspecified" + for stat in ('retry/count', f'retry/reason_count/{expected_reason}'): + self.assertEqual(spider.crawler.stats.get_value(stat), 1) + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "DEBUG", + f"Retrying {request} (failed {expected_retry_times} times): " + f"{expected_reason}", + ) + ) + + def test_max_retries_reached(self): + request = Request('https://example.com') + spider = self.get_spider() + max_retry_times = 0 + with LogCapture() as log: + new_request = get_retry_request( + request, + spider=spider, + max_retry_times=max_retry_times, + ) + self.assertEqual(new_request, None) + self.assertEqual( + spider.crawler.stats.get_value('retry/max_reached'), + 1 + ) + failure_count = max_retry_times + 1 + expected_reason = "unspecified" + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "ERROR", + f"Gave up retrying {request} (failed {failure_count} times): " + f"{expected_reason}", + ) + ) + + def test_one_retry(self): + request = Request('https://example.com') + spider = self.get_spider() + with LogCapture() as log: + new_request = get_retry_request( + request, + spider=spider, + max_retry_times=1, + ) + self.assertIsInstance(new_request, Request) + self.assertNotEqual(new_request, request) + self.assertEqual(new_request.dont_filter, True) + expected_retry_times = 1 + self.assertEqual(new_request.meta['retry_times'], expected_retry_times) + self.assertEqual(new_request.priority, -1) + expected_reason = "unspecified" + for stat in ('retry/count', f'retry/reason_count/{expected_reason}'): + self.assertEqual(spider.crawler.stats.get_value(stat), 1) + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "DEBUG", + f"Retrying {request} (failed {expected_retry_times} times): " + f"{expected_reason}", + ) + ) + + def test_two_retries(self): + spider = self.get_spider() + request = Request('https://example.com') + new_request = request + max_retry_times = 2 + for index in range(max_retry_times): + with LogCapture() as log: + new_request = get_retry_request( + new_request, + spider=spider, + max_retry_times=max_retry_times, + ) + self.assertIsInstance(new_request, Request) + self.assertNotEqual(new_request, request) + self.assertEqual(new_request.dont_filter, True) + expected_retry_times = index+1 + self.assertEqual(new_request.meta['retry_times'], expected_retry_times) + self.assertEqual(new_request.priority, -expected_retry_times) + expected_reason = "unspecified" + for stat in ('retry/count', f'retry/reason_count/{expected_reason}'): + value = spider.crawler.stats.get_value(stat) + self.assertEqual(value, expected_retry_times) + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "DEBUG", + f"Retrying {request} (failed {expected_retry_times} times): " + f"{expected_reason}", + ) + ) + + with LogCapture() as log: + new_request = get_retry_request( + new_request, + spider=spider, + max_retry_times=max_retry_times, + ) + self.assertEqual(new_request, None) + self.assertEqual( + spider.crawler.stats.get_value('retry/max_reached'), + 1 + ) + failure_count = max_retry_times + 1 + expected_reason = "unspecified" + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "ERROR", + f"Gave up retrying {request} (failed {failure_count} times): " + f"{expected_reason}", + ) + ) + + def test_no_spider(self): + request = Request('https://example.com') + with self.assertRaises(TypeError): + get_retry_request(request) + + def test_max_retry_times_setting(self): + max_retry_times = 0 + spider = self.get_spider({'RETRY_TIMES': max_retry_times}) + request = Request('https://example.com') + new_request = get_retry_request( + request, + spider=spider, + ) + self.assertEqual(new_request, None) + + def test_max_retry_times_meta(self): + max_retry_times = 0 + spider = self.get_spider({'RETRY_TIMES': max_retry_times + 1}) + meta = {'max_retry_times': max_retry_times} + request = Request('https://example.com', meta=meta) + new_request = get_retry_request( + request, + spider=spider, + ) + self.assertEqual(new_request, None) + + def test_max_retry_times_argument(self): + max_retry_times = 0 + spider = self.get_spider({'RETRY_TIMES': max_retry_times + 1}) + meta = {'max_retry_times': max_retry_times + 1} + request = Request('https://example.com', meta=meta) + new_request = get_retry_request( + request, + spider=spider, + max_retry_times=max_retry_times, + ) + self.assertEqual(new_request, None) + + def test_priority_adjust_setting(self): + priority_adjust = 1 + spider = self.get_spider({'RETRY_PRIORITY_ADJUST': priority_adjust}) + request = Request('https://example.com') + new_request = get_retry_request( + request, + spider=spider, + ) + self.assertEqual(new_request.priority, priority_adjust) + + def test_priority_adjust_argument(self): + priority_adjust = 1 + spider = self.get_spider({'RETRY_PRIORITY_ADJUST': priority_adjust+1}) + request = Request('https://example.com') + new_request = get_retry_request( + request, + spider=spider, + priority_adjust=priority_adjust, + ) + self.assertEqual(new_request.priority, priority_adjust) + + def test_log_extra_retry_success(self): + request = Request('https://example.com') + spider = self.get_spider() + with LogCapture(attributes=('spider',)) as log: + new_request = get_retry_request( + request, + spider=spider, + ) + log.check_present(spider) + + def test_log_extra_retries_exceeded(self): + request = Request('https://example.com') + spider = self.get_spider() + with LogCapture(attributes=('spider',)) as log: + new_request = get_retry_request( + request, + spider=spider, + max_retry_times=0, + ) + log.check_present(spider) + + def test_reason_string(self): + request = Request('https://example.com') + spider = self.get_spider() + expected_reason = 'because' + with LogCapture() as log: + new_request = get_retry_request( + request, + spider=spider, + reason=expected_reason, + ) + expected_retry_times = 1 + for stat in ('retry/count', f'retry/reason_count/{expected_reason}'): + self.assertEqual(spider.crawler.stats.get_value(stat), 1) + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "DEBUG", + f"Retrying {request} (failed {expected_retry_times} times): " + f"{expected_reason}", + ) + ) + + def test_reason_builtin_exception(self): + request = Request('https://example.com') + spider = self.get_spider() + expected_reason = NotImplementedError() + expected_reason_string = 'builtins.NotImplementedError' + with LogCapture() as log: + new_request = get_retry_request( + request, + spider=spider, + reason=expected_reason, + ) + expected_retry_times = 1 + stat = spider.crawler.stats.get_value( + f'retry/reason_count/{expected_reason_string}' + ) + self.assertEqual(stat, 1) + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "DEBUG", + f"Retrying {request} (failed {expected_retry_times} times): " + f"{expected_reason}", + ) + ) + + def test_reason_builtin_exception_class(self): + request = Request('https://example.com') + spider = self.get_spider() + expected_reason = NotImplementedError + expected_reason_string = 'builtins.NotImplementedError' + with LogCapture() as log: + new_request = get_retry_request( + request, + spider=spider, + reason=expected_reason, + ) + expected_retry_times = 1 + stat = spider.crawler.stats.get_value( + f'retry/reason_count/{expected_reason_string}' + ) + self.assertEqual(stat, 1) + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "DEBUG", + f"Retrying {request} (failed {expected_retry_times} times): " + f"{expected_reason}", + ) + ) + + def test_reason_custom_exception(self): + request = Request('https://example.com') + spider = self.get_spider() + expected_reason = IgnoreRequest() + expected_reason_string = 'scrapy.exceptions.IgnoreRequest' + with LogCapture() as log: + new_request = get_retry_request( + request, + spider=spider, + reason=expected_reason, + ) + expected_retry_times = 1 + stat = spider.crawler.stats.get_value( + f'retry/reason_count/{expected_reason_string}' + ) + self.assertEqual(stat, 1) + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "DEBUG", + f"Retrying {request} (failed {expected_retry_times} times): " + f"{expected_reason}", + ) + ) + + def test_reason_custom_exception_class(self): + request = Request('https://example.com') + spider = self.get_spider() + expected_reason = IgnoreRequest + expected_reason_string = 'scrapy.exceptions.IgnoreRequest' + with LogCapture() as log: + new_request = get_retry_request( + request, + spider=spider, + reason=expected_reason, + ) + expected_retry_times = 1 + stat = spider.crawler.stats.get_value( + f'retry/reason_count/{expected_reason_string}' + ) + self.assertEqual(stat, 1) + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "DEBUG", + f"Retrying {request} (failed {expected_retry_times} times): " + f"{expected_reason}", + ) + ) + + if __name__ == "__main__": unittest.main()