Fix AlreadyCalledError replacing a request in shell command. closes scrapy#407

Author: dangra

scrapy/shell.py

@@ -5,7 +5,7 @@
 """
 import signal
 
-from twisted.internet import reactor, threads
+from twisted.internet import reactor, threads, defer
 from twisted.python import threadable
 from w3lib.url import any_to_uri
 
@@ -14,7 +14,6 @@
 from scrapy.selector import XPathSelector, XmlXPathSelector, HtmlXPathSelector
 from scrapy.utils.spider import create_spider_for_request
 from scrapy.utils.misc import load_object
-from scrapy.utils.request import request_deferred
 from scrapy.utils.response import open_in_browser
 from scrapy.utils.console import start_python_console
 from scrapy.settings import Settings
@@ -55,17 +54,20 @@ def start(self, url=None, request=None, response=None, spider=None):
 
     def _schedule(self, request, spider):
         spider = self._open_spider(request, spider)
-        d = request_deferred(request)
+        d = _request_deferred(request)
         d.addCallback(lambda x: (x, spider))
         self.crawler.engine.crawl(request, spider)
         return d
 
     def _open_spider(self, request, spider):
         if self.spider:
             return self.spider
+
         if spider is None:
-            spider = create_spider_for_request(self.crawler.spiders, request,
-                BaseSpider('default'), log_multiple=True)
+            spider = create_spider_for_request(self.crawler.spiders,
+                                               request,
+                                               BaseSpider('default'),
+                                               log_multiple=True)
         spider.set_crawler(self.crawler)
         self.crawler.engine.open_spider(spider, close_if_idle=False)
         self.spider = spider
@@ -127,3 +129,30 @@ def inspect_response(response, spider=None):
     """Open a shell to inspect the given response"""
     from scrapy.project import crawler
     Shell(crawler).start(response=response, spider=spider)
+
+
+def _request_deferred(request):
+    """Wrap a request inside a Deferred.
+
+    This function is harmful, do not use it until you know what you are doing.
+
+    This returns a Deferred whose first pair of callbacks are the request
+    callback and errback. The Deferred also triggers when the request
+    callback/errback is executed (ie. when the request is downloaded)
+
+    WARNING: Do not call request.replace() until after the deferred is called.
+    """
+    request_callback = request.callback
+    request_errback = request.errback
+    def _restore_callbacks(result):
+        request.callback = request_callback
+        request.errback = request_errback
+        return result
+
+    d = defer.Deferred()
+    d.addBoth(_restore_callbacks)
+    if request.callback:
+        d.addCallbacks(request.callback, request.errback)
+
+    request.callback, request.errback = d.callback, d.errback
+    return d

scrapy/tests/test_command_shell.py

@@ -45,3 +45,9 @@ def test_redirect(self):
         _, out, _ = yield self.execute([self.url('/redirect'), '-c', 'response.url'])
         assert out.strip().endswith('/redirected')
 
+    @defer.inlineCallbacks
+    def test_request_replace(self):
+        url = self.url('/text')
+        code = "fetch('{0}') or fetch(response.request.replace(method='POST'))"
+        errcode, out, _ = yield self.execute(['-c', code.format(url)])
+        self.assertEqual(errcode, 0, out)

scrapy/utils/request.py

@@ -18,10 +18,10 @@
 def request_fingerprint(request, include_headers=None):
     """
     Return the request fingerprint.
-    
+
     The request fingerprint is a hash that uniquely identifies the resource the
     request points to. For example, take the following two urls:
-    
+
     http://www.example.com/query?id=111&cat=222
     http://www.example.com/query?cat=222&id=111
 
@@ -30,13 +30,13 @@ def request_fingerprint(request, include_headers=None):
 
     Another example are cookies used to store session ids. Suppose the
     following page is only accesible to authenticated users:
-    
+
     http://www.example.com/members/offers.html
 
     Lot of sites use a cookie to store the session id, which adds a random
     component to the HTTP Request and thus should be ignored when calculating
-    the fingerprint. 
-    
+    the fingerprint.
+
     For this reason, request headers are ignored by default when calculating
     the fingeprint. If you want to include specific headers use the
     include_headers argument, which is a list of Request headers to include.
@@ -81,15 +81,3 @@ def request_httprepr(request):
     s += request.body
     return s
 
-def request_deferred(request):
-    """Wrap a request inside a Deferred.
-
-    This returns a Deferred whose first pair of callbacks are the request
-    callback and errback. The Deferred also triggers when the request
-    callback/errback is executed (ie. when the request is downloaded)
-    """
-    d = Deferred()
-    if request.callback:
-        d.addCallbacks(request.callback, request.errback)
-    request.callback, request.errback = d.callback, d.errback
-    return d