Merge pull request #93 from scrapinghub/pedro-errback

Pedro errback

Author: pawelmhm

docs/source/api.rst

@@ -96,9 +96,16 @@ callback
     - type: string
     - optional
 
-    Should exist as method of scheduled spider, does not need to contain self.
+    Must exist as method of scheduled spider, does not need to contain string "self".
     If not passed or not found on spider default callback `parse`_ will be used.
 
+errback
+    - type: string
+    - optional
+
+    Scrapy errback for request made from spider. It must exist as method of
+    scheduled spider, otherwise exception will be raised. String does not need to contain 'self'.
+
 max_requests
     - type: integer
     - optional

docs/source/conf.py

@@ -55,9 +55,9 @@
 # built documents.
 #
 # The short X.Y version.
-version = '0.10'
+version = '0.12'
 # The full version, including alpha/beta/rc tags.
-release = '0.10'
+release = '0.12'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.

requirements-dev.txt

@@ -5,3 +5,4 @@ mock==1.3.0
 pytest==2.9.1
 pytest-cov==2.2.1
 port-for==0.3.1
+Flask==1.1.1

scrapyrt/core.py

@@ -105,6 +105,9 @@ def __init__(self, spider_name, request_kwargs, max_requests=None, start_request
         # callback will be added after instantiation of crawler object
         # because we need to know if spider has method available
         self.callback_name = request_kwargs.pop('callback', None) or 'parse'
+        # do the same for errback
+        self.errback_name = request_kwargs.pop('errback', None) or 'parse'
+
         if request_kwargs.get("url"):
             self.request = self.create_spider_request(deepcopy(request_kwargs))
         else:
@@ -145,7 +148,7 @@ def spider_idle(self, spider):
         """Handler of spider_idle signal.
 
         Schedule request for url given to api, with optional callback
-        that can be passed as GET parameter.
+        and errback that can be passed as GET parameter.
 
         spider_idle signal is used because we want to optionally enable
         start_requests for the spider and if request is scheduled in
@@ -157,6 +160,10 @@ def spider_idle(self, spider):
             callback = getattr(self.crawler.spider, self.callback_name)
             assert callable(callback), 'Invalid callback'
             self.request = self.request.replace(callback=callback)
+
+            errback = getattr(self.crawler.spider, self.errback_name)
+            assert callable(errback), 'Invalid errback'
+            self.request = self.request.replace(errback=errback)
             modify_request = getattr(
                 self.crawler.spider, "modify_realtime_request", None)
             if callable(modify_request):

tests/sample_data/testproject/testproject/spiders/testspider.py

@@ -11,3 +11,7 @@ class TestSpider(scrapy.Spider):
     def parse(self, response):
         name = response.xpath('//h1/text()').extract()
         return TestprojectItem(name=name)
+
+    def some_errback(self, err):
+        self.logger.error("Logging some error {}".format(err))
+        return

tests/servers.py

@@ -1,18 +1,17 @@
 # -*- coding: utf-8 -*-
-import six
-from subprocess import Popen, PIPE
-from six.moves.urllib.parse import urljoin
 import fcntl
 import os
 import shutil
 import socket
 import sys
 import tempfile
 import time
+from subprocess import Popen, PIPE
 
 import port_for
+from six.moves.urllib.parse import urljoin
 
-from . import SAMPLE_DATA
+from . import TESTS_PATH
 from .utils import get_testenv, generate_project
 
 DEVNULL = open(os.devnull, 'wb')
@@ -30,12 +29,9 @@ def __init__(self, host='localhost', port=None, cwd=None, shell=False,
         self.stdin = stdin
         self.stdout = stdout
         self.stderr = stderr
-        if six.PY2:
-            command = 'SimpleHTTPServer'
-        else:
-            command = 'http.server'
+
         self.arguments = [
-            sys.executable, '-u', '-m', command, str(self.port)
+            'flask', 'run', '-p', str(self.port)
         ]
 
     def start(self):
@@ -126,4 +122,4 @@ class MockServer(BaseTestServer):
 
     def __init__(self, *args, **kwargs):
         super(MockServer, self).__init__(*args, **kwargs)
-        self.cwd = os.path.join(SAMPLE_DATA, 'testsite')
+        self.cwd = os.path.join(TESTS_PATH, 'testsite')

tests/test_crawl_manager.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 import os
+import re
 from time import sleep
 import datetime
 
@@ -29,14 +30,15 @@ def setUp(self):
         self.crawler = MagicMock()
         self.spider = MetaSpider.from_crawler(self.crawler)
         self.crawler.spider = self.spider
-        self.crawl_manager = self._create_crawl_manager()
+        self.crawl_manager = self.create_crawl_manager()
         self.crawl_manager.crawler = self.crawler
         self.item = Item()
         self.response = Response('http://localhost')
         self.another_spider = MetaSpider.from_crawler(self.crawler)
 
-    def _create_crawl_manager(self):
-        crawl_manager = CrawlManager(self.spider.name, self.kwargs.copy())
+    def create_crawl_manager(self, kwargs=None):
+        kwargs = kwargs if kwargs else self.kwargs.copy()
+        crawl_manager = CrawlManager(self.spider.name, kwargs)
         crawl_manager.crawler = self.crawler
         return crawl_manager
 
@@ -136,6 +138,33 @@ def test_modify_realtime_request_is_not_callable(self):
             self.crawl_manager.request, self.spider)
         self.assertNotEqual(self.request, self.crawl_manager.request)
 
+    def test_pass_wrong_spider_errback(self):
+        mng = self.create_crawl_manager(
+            {'url': 'http://localhost', 'errback': 'handle_error'}
+        )
+        try:
+            with pytest.raises(AttributeError) as err:
+                mng.spider_idle(self.spider)
+        except DontCloseSpider:
+            pass
+
+        assert mng.request.errback is None
+        msg = "AttributeError: 'MetaSpider' object has no attribute 'handle_error'"
+        assert re.search(msg, str(err))
+
+    def test_pass_good_spider_errback(self):
+        mng = self.create_crawl_manager(
+            {'url': 'http://localhost', 'errback': 'handle_error'}
+        )
+        self.crawler.spider.handle_error = lambda x: x
+        try:
+            mng.spider_idle(self.spider)
+        except DontCloseSpider:
+            pass
+
+        assert callable(mng.request.errback)
+        assert mng.request.errback('something') == 'something'
+
 
 class TestHandleScheduling(TestCrawlManager):
 
@@ -178,7 +207,7 @@ def test_string_number_timeout_value(self):
         _timeout = settings.TIMEOUT_LIMIT
         try:
             settings.TIMEOUT_LIMIT = '1'
-            self.crawl_manager = self._create_crawl_manager()
+            self.crawl_manager = self.create_crawl_manager()
             self._test_limit_runtime()
         finally:
             settings.TIMEOUT_LIMIT = _timeout

tests/test_resource_crawl.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 import json
+import os
 
 import pytest
 import re
@@ -378,3 +379,24 @@ def test_invalid_json_in_post(self, server):
             assert res_json[k] == v
         msg = "Invalid JSON in POST body"
         assert msg in res_json['message']
+
+    @pytest.mark.parametrize("method", [
+        perform_get, perform_post
+    ])
+    def test_passing_errback(self, server, method):
+        url = server.url("crawl.json")
+        res = method(url,
+                     {"spider_name": "test"},
+                     {"url": server.target_site.url("err/503"),
+                      'errback': 'some_errback'})
+
+        res_json = res.json()
+        assert res_json.get('stats').get('log_count/ERROR') == 1
+        assert res_json['status'] == 'ok'
+        logs_path = os.path.join(server.cwd, 'logs', 'test')
+        logs_files = os.listdir(logs_path)
+        with open(os.path.join(logs_path, logs_files[0])) as f:
+            log_file = f.read()
+
+        msg = 'ERROR: Logging some error'
+        assert re.search(msg, log_file)

tests/testsite/app.py

@@ -0,0 +1,39 @@
+from flask import Flask, abort
+
+
+app = Flask(__name__)
+
+
+def read_file(filename):
+    with open(filename) as f:
+        return f.read()
+
+
+@app.route('/')
+def base():
+    return b'hello'
+
+
+@app.route('/index.html')
+def index():
+    return read_file('index.html')
+
+
+@app.route('/page1.html')
+def page1():
+    return read_file('page1.html')
+
+
+@app.route('/page2.html')
+def page2():
+    return read_file('page2.html')
+
+
+@app.route('/page3.html')
+def page3():
+    return read_file('page3.html')
+
+
+@app.route('/err/<int:code>')
+def return_code(code):
+    abort(code)