Use a non-zero exit code when a pipeline's open_spider method throws an exception (#4207)

* fix issue 4175 - Scrapy does not use a non-zero exit code when pipeline's open_spider throws the exception

* remove extra blank lines

* remove redundant code

* remove blank line at end of file

* more suitable naming for response and make if-condition shorter

* avoid error - AttributeError: 'Deferred' object has no attribute 'result'

* use getattr to make code concisely

* add test

* remove useless file

* modify test class name

* remove unneccessary files

* Fix Flake8-reported issue

* fix these items which are suggested by Gallaecio
・Sort those imports at tests/test_cmdline_crawl_with_pipeline/__init__.py
・Remove the unused setUp method.
・Remove comments generated by Scrapy’s project generation tool.
・Remove the [deploy] section from the scrapy.cfg file (I don’t think it’s needed here)
・Remove BOT_NAME and NEWSPIDER_MODULE from settings.py (I think there are not needed either, although I’m less sure about NEWSPIDER_MODULE)

* have to reserve BOT_NAME, SPIDER_MODULES in settings.py

* Remove unneeded empty lines

* Empty __init__.py file with unneeded comments

* Remove an unneeded empty line at the end

* Remove unneeed empty line from __init__.py file

* Update __init__.py

* Update __init__.py

* Update exception.py

* Update normal.py

* Update __init__.py

* Update __init__.py

* fix W391 blank line at end of file

Co-authored-by: Adrián Chaves <adrian@chaves.io>

scrapy/commands/crawl.py

@@ -54,8 +54,13 @@ class Command(ScrapyCommand):
             raise UsageError("running 'scrapy crawl' with more than one spider is no longer supported")
         spname = args[0]
 
-        self.crawler_process.crawl(spname, **opts.spargs)
-        self.crawler_process.start()
+        crawl_defer = self.crawler_process.crawl(spname, **opts.spargs)
 
-        if self.crawler_process.bootstrap_failed:
+        if getattr(crawl_defer, 'result', None) is not None and issubclass(crawl_defer.result.type, Exception):
             self.exitcode = 1
+        else:
+            self.crawler_process.start()
+
+            if self.crawler_process.bootstrap_failed or \
+                    (hasattr(self.crawler_process, 'has_exception') and self.crawler_process.has_exception):
+                self.exitcode = 1

tests/test_cmdline_crawl_with_pipeline/__init__.py

@@ -0,0 +1,20 @@
+import os
+import sys
+import unittest
+from subprocess import Popen, PIPE
+
+
+class CmdlineCrawlPipelineTest(unittest.TestCase):
+
+    def _execute(self, spname):
+        args = (sys.executable, '-m', 'scrapy.cmdline', 'crawl', spname)
+        cwd = os.path.dirname(os.path.abspath(__file__))
+        proc = Popen(args, stdout=PIPE, stderr=PIPE, cwd=cwd)
+        proc.communicate()
+        return proc.returncode
+
+    def test_open_spider_normally_in_pipeline(self):
+        self.assertEqual(self._execute('normal'), 0)
+
+    def test_exception_at_open_spider_in_pipeline(self):
+        self.assertEqual(self._execute('exception'), 1)

tests/test_cmdline_crawl_with_pipeline/scrapy.cfg

@@ -0,0 +1,2 @@
+[settings]
+default = test_spider.settings

tests/test_cmdline_crawl_with_pipeline/test_spider/pipelines.py

@@ -0,0 +1,16 @@
+class TestSpiderPipeline(object):
+
+    def open_spider(self, spider):
+        pass
+
+    def process_item(self, item, spider):
+        return item
+
+
+class TestSpiderExceptionPipeline(object):
+
+    def open_spider(self, spider):
+        raise Exception('exception')
+
+    def process_item(self, item, spider):
+        return item

tests/test_cmdline_crawl_with_pipeline/test_spider/settings.py

@@ -0,0 +1,2 @@
+BOT_NAME = 'test_spider'
+SPIDER_MODULES = ['test_spider.spiders']

tests/test_cmdline_crawl_with_pipeline/test_spider/spiders/exception.py

@@ -0,0 +1,14 @@
+import scrapy
+
+
+class ExceptionSpider(scrapy.Spider):
+    name = 'exception'
+
+    custom_settings = {
+        'ITEM_PIPELINES': {
+            'test_spider.pipelines.TestSpiderExceptionPipeline': 300
+        }
+    }
+
+    def parse(self, response):
+        pass

tests/test_cmdline_crawl_with_pipeline/test_spider/spiders/normal.py

@@ -0,0 +1,14 @@
+import scrapy
+
+
+class NormalSpider(scrapy.Spider):
+    name = 'normal'
+
+    custom_settings = {
+        'ITEM_PIPELINES': {
+            'test_spider.pipelines.TestSpiderPipeline': 300
+        }
+    }
+
+    def parse(self, response):
+        pass