From 26809d2254ecbceb1352137815400de20fd12c76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Thu, 20 Jun 2013 10:15:23 -0300 Subject: [PATCH] fix download_timeout for servers that returns response headers but hangs sending its body --- scrapy/core/downloader/handlers/http11.py | 17 ++++++---- scrapy/tests/mockserver.py | 41 +++++++++++++++++------ scrapy/tests/spiders.py | 7 ++-- scrapy/tests/test_crawl.py | 7 ++++ scrapy/tests/test_downloader_handlers.py | 15 +++++++-- 5 files changed, 65 insertions(+), 22 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index a9d0a2b8d..bdef52eb5 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -72,14 +72,14 @@ class ScrapyAgent(object): start_time = time() d = agent.request(method, url, headers, bodyproducer) - # check download timeout - self._timeout_cl = reactor.callLater(timeout, d.cancel) - d.addBoth(self._cb_timeout, request, url, timeout) # set download latency d.addCallback(self._cb_latency, request, start_time) # response body is ready to be consumed d.addCallback(self._cb_bodyready, request) d.addCallback(self._cb_bodydone, request, url) + # check download timeout + d.addBoth(self._cb_timeout, request, url, timeout) + self._timeout_cl = reactor.callLater(timeout, d.cancel) return d def _cb_timeout(self, result, request, url, timeout): @@ -97,9 +97,12 @@ class ScrapyAgent(object): if txresponse.length == 0: return txresponse, '', None - finished = defer.Deferred() - txresponse.deliverBody(_ResponseReader(finished, txresponse, request)) - return finished + def _cancel(_): + txresponse._transport._producer.loseConnection() + + d = defer.Deferred(_cancel) + txresponse.deliverBody(_ResponseReader(d, txresponse, request)) + return d def _cb_bodydone(self, result, request, url): txresponse, body, flags = result @@ -139,6 +142,8 @@ class _ResponseReader(protocol.Protocol): self._bodybuf.write(bodyBytes) def connectionLost(self, reason): + if self._finished.called: + return body = self._bodybuf.getvalue() if reason.check(ResponseDone): self._finished.callback((self._txresponse, body, None)) diff --git a/scrapy/tests/mockserver.py b/scrapy/tests/mockserver.py index c293fde49..8b1cc09e8 100644 --- a/scrapy/tests/mockserver.py +++ b/scrapy/tests/mockserver.py @@ -12,6 +12,7 @@ def getarg(request, name, default=None, type=str): else: return default + class Follow(Resource): isLeaf = True @@ -23,8 +24,8 @@ class Follow(Resource): n = getarg(request, "n", total, type=int) if order == "rand": nlist = [random.randint(1, total) for _ in range(show)] - else: # order == "desc" - nlist = range(n, max(n-show, 0), -1) + else: # order == "desc" + nlist = range(n, max(n - show, 0), -1) s = """ """ args = request.args.copy() @@ -35,20 +36,37 @@ class Follow(Resource): s += """""" return s -class Delay(Resource): + +class DeferMixin(Resource): + + def deferRequest(self, request, delay, f, *a, **kw): + def _cancelrequest(_): + # silence CancelledError + d.addErrback(lambda _: None) + d.cancel() + d = deferLater(reactor, delay, f, *a, **kw) + request.notifyFinish().addErrback(_cancelrequest) + return d + + +class Delay(DeferMixin, Resource): isLeaf = True def render_GET(self, request): n = getarg(request, "n", 1, type=float) - d = deferLater(reactor, n, lambda: (request, n)) - d.addCallback(self._delayedRender) + b = getarg(request, "b", 1, type=int) + if b: + # send headers now and delay body + request.write('') + self.deferRequest(request, n, self._delayedRender, request, n) return NOT_DONE_YET - def _delayedRender(self, (request, n)): + def _delayedRender(self, request, n): request.write("Response delayed for %0.3f seconds\n" % n) request.finish() + class Status(Resource): isLeaf = True @@ -58,20 +76,21 @@ class Status(Resource): request.setResponseCode(n) return "" -class Partial(Resource): + +class Partial(DeferMixin, Resource): isLeaf = True def render_GET(self, request): request.setHeader("Content-Length", "1024") - d = deferLater(reactor, 0, lambda: request) - d.addCallback(self._delayedRender) + self.deferRequest(request, 0, self._delayedRender, request) return NOT_DONE_YET def _delayedRender(self, request): request.write("partial content\n") request.finish() + class Drop(Partial): def _delayedRender(self, request): @@ -79,6 +98,7 @@ class Drop(Partial): request.channel.transport.loseConnection() request.finish() + class Root(Resource): def __init__(self): @@ -95,12 +115,13 @@ class Root(Resource): def render(self, request): return 'Scrapy mock HTTP server\n' + class MockServer(): def __enter__(self): from scrapy.utils.test import get_testenv self.proc = Popen([sys.executable, '-u', '-m', 'scrapy.tests.mockserver'], - stdout=PIPE, env=get_testenv()) + stdout=PIPE, env=get_testenv()) self.proc.stdout.readline() def __exit__(self, exc_type, exc_value, traceback): diff --git a/scrapy/tests/spiders.py b/scrapy/tests/spiders.py index 27442d25b..e1d06d85c 100644 --- a/scrapy/tests/spiders.py +++ b/scrapy/tests/spiders.py @@ -45,15 +45,16 @@ class DelaySpider(MetaSpider): name = 'delay' - def __init__(self, n=1, *args, **kwargs): + def __init__(self, n=1, b=0, *args, **kwargs): super(DelaySpider, self).__init__(*args, **kwargs) self.n = n + self.b = b self.t1 = self.t2 = self.t2_err = 0 def start_requests(self): self.t1 = time.time() - yield Request("http://localhost:8998/delay?n=%s" % self.n, \ - callback=self.parse, errback=self.errback) + url = "http://localhost:8998/delay?n=%s&b=%s" % (self.n, self.b) + yield Request(url, callback=self.parse, errback=self.errback) def parse(self, response): self.t2 = time.time() diff --git a/scrapy/tests/test_crawl.py b/scrapy/tests/test_crawl.py index 8417844e5..5b3b8a0e2 100644 --- a/scrapy/tests/test_crawl.py +++ b/scrapy/tests/test_crawl.py @@ -51,6 +51,13 @@ class CrawlTestCase(TestCase): self.assertTrue(spider.t2 == 0) self.assertTrue(spider.t2_err > 0) self.assertTrue(spider.t2_err > spider.t1) + # server hangs after receiving response headers + spider = DelaySpider(n=0.5, b=1) + yield docrawl(spider, {"DOWNLOAD_TIMEOUT": 0.35}) + self.assertTrue(spider.t1 > 0) + self.assertTrue(spider.t2 == 0) + self.assertTrue(spider.t2_err > 0) + self.assertTrue(spider.t2_err > spider.t1) @defer.inlineCallbacks def test_retry_503(self): diff --git a/scrapy/tests/test_downloader_handlers.py b/scrapy/tests/test_downloader_handlers.py index 472817777..18be4ed35 100644 --- a/scrapy/tests/test_downloader_handlers.py +++ b/scrapy/tests/test_downloader_handlers.py @@ -58,6 +58,7 @@ class HttpTestCase(unittest.TestCase): r = static.File(name) r.putChild("redirect", util.Redirect("/file")) r.putChild("wait", ForeverTakingResource()) + r.putChild("hang-after-headers", ForeverTakingResource(write=True)) r.putChild("nolength", NoLengthResource()) r.putChild("host", HostHeaderResource()) r.putChild("payload", PayloadResource()) @@ -106,10 +107,18 @@ class HttpTestCase(unittest.TestCase): d.addCallback(self.assertEquals, 302) return d + @defer.inlineCallbacks def test_timeout_download_from_spider(self): - request = Request(self.getURL('wait'), meta=dict(download_timeout=0.1)) - d = self.download_request(request, BaseSpider('foo')) - return self.assertFailure(d, defer.TimeoutError, error.TimeoutError) + spider = BaseSpider('foo') + meta = {'download_timeout': 0.2} + # client connects but no data is received + request = Request(self.getURL('wait'), meta=meta) + d = self.download_request(request, spider) + yield self.assertFailure(d, defer.TimeoutError, error.TimeoutError) + # client connects, server send headers and some body bytes but hangs + request = Request(self.getURL('hang-after-headers'), meta=meta) + d = self.download_request(request, spider) + yield self.assertFailure(d, defer.TimeoutError, error.TimeoutError) def test_host_header_not_in_request_headers(self): def _test(response):