From 60727dedf605fad2ed4be844cb2ec44e305257f0 Mon Sep 17 00:00:00 2001 From: Valdir Stumm Junior Date: Wed, 31 May 2017 15:00:38 -0300 Subject: [PATCH 1/4] verify if Request callback is callable --- scrapy/http/request/__init__.py | 4 ++++ tests/test_http_request.py | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 1435d91de..b9c5f8541 100644 --- a/scrapy/http/request/__init__.py +++ b/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 function, got %s' % type(callback).__name__) + if errback is not None and not callable(errback): + raise TypeError('errback must be a function, got %s' % type(errback).__name__) assert callback or not errback, "Cannot use errback without a callback" self.callback = callback self.errback = errback diff --git a/tests/test_http_request.py b/tests/test_http_request.py index bbce537f4..9b0ee63dc 100644 --- a/tests/test_http_request.py +++ b/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): From 0f6f486769d7761054274509de08fcf455d492de Mon Sep 17 00:00:00 2001 From: Valdir Stumm Junior Date: Mon, 5 Jun 2017 16:19:00 -0300 Subject: [PATCH 2/4] fix parse command issue with callback as a string --- scrapy/commands/parse.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scrapy/commands/parse.py b/scrapy/commands/parse.py index 5264982b6..a90095146 100644 --- a/scrapy/commands/parse.py +++ b/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: From 4b6f68b9ee2830534c25b824103b13d502005fe1 Mon Sep 17 00:00:00 2001 From: Valdir Stumm Junior Date: Mon, 5 Jun 2017 17:26:52 -0300 Subject: [PATCH 3/4] make reqser tests create Request with proper callback/errback --- tests/test_utils_reqser.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_utils_reqser.py b/tests/test_utils_reqser.py index 073baadc2..dcc070b8f 100644 --- a/tests/test_utils_reqser.py +++ b/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") From bb0bd691d9dd42725af6291a15e357f0146de17f Mon Sep 17 00:00:00 2001 From: Valdir Stumm Junior Date: Mon, 24 Jul 2017 11:12:09 -0300 Subject: [PATCH 4/4] Improve error message when callback is not callable --- scrapy/http/request/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index b9c5f8541..13a92ffa0 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -28,9 +28,9 @@ class Request(object_ref): self.priority = priority if callback is not None and not callable(callback): - raise TypeError('callback must be a function, got %s' % type(callback).__name__) + 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 function, got %s' % type(errback).__name__) + 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