Merge pull request #2769 from stummjr/issue-2766

[MRG+1] Add verification to check if Request callback is callable

scrapy/commands/parse.py

@@ -142,7 +142,8 @@ class Command(ScrapyCommand):
                 logger.error('Unable to find spider for: %(url)s',
                              {'url': url})
 
-        request = Request(url, opts.callback)
+        # Request requires callback argument as callable or None, not string
+        request = Request(url, None)
         _start_requests = lambda s: [self.prepare_request(s, request, opts)]
         self.spidercls.start_requests = _start_requests
 
@@ -164,7 +165,9 @@ class Command(ScrapyCommand):
             # determine real callback
             cb = response.meta['_callback']
             if not cb:
-                if opts.rules and self.first_response == response:
+                if opts.callback:
+                    cb = opts.callback
+                elif opts.rules and self.first_response == response:
                     cb = self.get_callback_from_rules(spider, response)
 
                     if not cb:

scrapy/http/request/__init__.py

@@ -27,6 +27,10 @@ class Request(object_ref):
         assert isinstance(priority, int), "Request priority not an integer: %r" % priority
         self.priority = priority
 
+        if callback is not None and not callable(callback):
+            raise TypeError('callback must be a callable, got %s' % type(callback).__name__)
+        if errback is not None and not callable(errback):
+            raise TypeError('errback must be a callable, got %s' % type(errback).__name__)
         assert callback or not errback, "Cannot use errback without a callback"
         self.callback = callback
         self.errback = errback

tests/test_http_request.py

@@ -235,6 +235,26 @@ class RequestTest(unittest.TestCase):
         self.assertRaises(AttributeError, setattr, r, 'url', 'http://example2.com')
         self.assertRaises(AttributeError, setattr, r, 'body', 'xxx')
 
+    def test_callback_is_callable(self):
+        def a_function():
+            pass
+        r = self.request_class('http://example.com')
+        self.assertIsNone(r.callback)
+        r = self.request_class('http://example.com', a_function)
+        self.assertIs(r.callback, a_function)
+        with self.assertRaises(TypeError):
+            self.request_class('http://example.com', 'a_function')
+
+    def test_errback_is_callable(self):
+        def a_function():
+            pass
+        r = self.request_class('http://example.com')
+        self.assertIsNone(r.errback)
+        r = self.request_class('http://example.com', a_function, errback=a_function)
+        self.assertIs(r.errback, a_function)
+        with self.assertRaises(TypeError):
+            self.request_class('http://example.com', a_function, errback='a_function')
+
 
 class FormRequestTest(RequestTest):
 

tests/test_utils_reqser.py

@@ -17,8 +17,8 @@ class RequestSerializationTest(unittest.TestCase):
 
     def test_all_attributes(self):
         r = Request("http://www.example.com",
-            callback='parse_item',
-            errback='handle_error',
+            callback=self.spider.parse_item,
+            errback=self.spider.handle_error,
             method="POST",
             body=b"some body",
             headers={'content-encoding': 'text/html; charset=latin-1'},
@@ -27,7 +27,7 @@ class RequestSerializationTest(unittest.TestCase):
             priority=20,
             meta={'a': 'b'},
             flags=['testFlag'])
-        self._assert_serializes_ok(r)
+        self._assert_serializes_ok(r, spider=self.spider)
 
     def test_latin1_body(self):
         r = Request("http://www.example.com", body=b"\xa3")