Repository: scrapy/scrapy

Issue:
Handle it gracefully when start_url is used instead of start_urls
Over the last year I’ve seen a few cases ([recent example](https://stackoverflow.com/q/58664004/939364)) of this, people missing the `s` at the end of the `start_urls`.

It may be nice to find a way to gracefully let the developer know where the issue is, why there is no crawling happening.

Discussion:
mabelvj: I am having trouble with the tests since it seems we should start a crawling for getting the result from `start_requests` -> https://github.com/scrapy/scrapy/pull/4170/commits/96f7e976fe4a9302b05ab5cec8def480bf58a862.

Maybe just checking when trying to set the variable could be a good solution.
codecov[bot]: # [Codecov](https://codecov.io/gh/scrapy/scrapy/pull/4170?src=pr&el=h1) Report
> Merging [#4170](https://codecov.io/gh/scrapy/scrapy/pull/4170?src=pr&el=desc) into [master](https://codecov.io/gh/scrapy/scrapy/commit/78ad01632f16a97fb180d8c2972075b19f471380?src=pr&el=desc) will **decrease** coverage by `0.38%`.
> The diff coverage is `77.77%`.


```diff
@@            Coverage Diff            @@
##           master   #4170      +/-   ##
=========================================
- Coverage   83.69%   83.3%   -0.39%     
=========================================
  Files         165     165              
  Lines        9717    9616     -101     
  Branches     1445    1447       +2     
=========================================
- Hits         8133    8011     -122     
- Misses       1336    1355      +19     
- Partials      248     250       +2
```

| [Impacted Files](https://codecov.io/gh/scrapy/scrapy/pull/4170?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [scrapy/spiders/\_\_init\_\_.py](https://codecov.io/gh/scrapy/scrapy/pull/4170/diff?src=pr&el=tree#diff-c2NyYXB5L3NwaWRlcnMvX19pbml0X18ucHk=) | `95.52% <77.77%> (-2.9%)` | :arrow_down: |
| [scrapy/robotstxt.py](https://codecov.io/gh/scrapy/scrapy/pull/4170/diff?src=pr&el=tree#diff-c2NyYXB5L3JvYm90c3R4dC5weQ==) | `73.68% <0%> (-23.88%)` | :arrow_down: |
| [scrapy/core/downloader/tls.py](https://codecov.io/gh/scrapy/scrapy/pull/4170/diff?src=pr&el=tree#diff-c2NyYXB5L2NvcmUvZG93bmxvYWRlci90bHMucHk=) | `79.06% <0%> (-6.98%)` | :arrow_down: |
| [scrapy/utils/trackref.py](https://codecov.io/gh/scrapy/scrapy/pull/4170/diff?src=pr&el=tree#diff-c2NyYXB5L3V0aWxzL3RyYWNrcmVmLnB5) | `83.78% <0%> (-2.71%)` | :arrow_down: |
| [scrapy/extensions/memdebug.py](https://codecov.io/gh/scrapy/scrapy/pull/4170/diff?src=pr&el=tree#diff-c2NyYXB5L2V4dGVuc2lvbnMvbWVtZGVidWcucHk=) | `47.61% <0%> (-2.39%)` | :arrow_down: |
| [scrapy/extensions/debug.py](https://codecov.io/gh/scrapy/scrapy/pull/4170/diff?src=pr&el=tree#diff-c2NyYXB5L2V4dGVuc2lvbnMvZGVidWcucHk=) | `44.73% <0%> (-1.42%)` | :arrow_down: |
| [scrapy/extensions/throttle.py](https://codecov.io/gh/scrapy/scrapy/pull/4170/diff?src=pr&el=tree#diff-c2NyYXB5L2V4dGVuc2lvbnMvdGhyb3R0bGUucHk=) | `44.89% <0%> (-1.11%)` | :arrow_down: |
| [scrapy/pipelines/\_\_init\_\_.py](https://codecov.io/gh/scrapy/scrapy/pull/4170/diff?src=pr&el=tree#diff-c2NyYXB5L3BpcGVsaW5lcy9fX2luaXRfXy5weQ==) | `91.66% <0%> (-0.65%)` | :arrow_down: |
| [scrapy/extensions/telnet.py](https://codecov.io/gh/scrapy/scrapy/pull/4170/diff?src=pr&el=tree#diff-c2NyYXB5L2V4dGVuc2lvbnMvdGVsbmV0LnB5) | `80.95% <0%> (-0.59%)` | :arrow_down: |
| [scrapy/extensions/memusage.py](https://codecov.io/gh/scrapy/scrapy/pull/4170/diff?src=pr&el=tree#diff-c2NyYXB5L2V4dGVuc2lvbnMvbWVtdXNhZ2UucHk=) | `48.35% <0%> (-0.57%)` | :arrow_down: |
| ... and [59 more](https://codecov.io/gh/scrapy/scrapy/pull/4170/diff?src=pr&el=tree-more) | |

kmike: Hey! There are real-world spiders which use start_url option, as a way to customize things (found them using our internal code search). Is there a way to distinguish between intended and unintended usage? It is not good to show a warning for a perfectly valid use case.

I wonder if a better fix would be to have a warning in the default start_requests implementation: if start_urls is None, but start_url is present, then show a warning. There won't be a warning if start_requests function is using `start_url` attribute in this case, as it'd be overridden. 

It could make sense to check real spiders which use start_request attribute, to see how any change we make affect them.
mabelvj: You are right @kmike, that was one of my other options. Setting this warning in `start_requests` will allow overriding it when using `start_url`.

Patch:
diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py
index 94095bc27..517576ca6 100644
--- a/scrapy/spiders/__init__.py
+++ b/scrapy/spiders/__init__.py
@@ -58,6 +58,11 @@ class Spider(object_ref):
 
     def start_requests(self):
         cls = self.__class__
+        if not self.start_urls and hasattr(self, 'start_url'):
+            raise AttributeError(
+                "Crawling could not start: 'start_urls' not found "
+                "or empty (but found 'start_url' attribute instead, "
+                "did you miss an 's'?)")
         if method_is_overridden(cls, Spider, 'make_requests_from_url'):
             warnings.warn(
                 "Spider.make_requests_from_url method is deprecated; it "


Test patch:
diff --git a/tests/test_spider.py b/tests/test_spider.py
index c0fccfdd6..3cebdf7ca 100644
--- a/tests/test_spider.py
+++ b/tests/test_spider.py
@@ -385,6 +385,14 @@ class CrawlSpiderTest(SpiderTest):
         self.assertTrue(hasattr(spider, '_follow_links'))
         self.assertFalse(spider._follow_links)
 
+    def test_start_url(self):
+        spider = self.spider_class("example.com")
+        spider.start_url = 'https://www.example.com'
+
+        with self.assertRaisesRegex(AttributeError,
+                                    r'^Crawling could not start.*$'):
+            list(spider.start_requests())
+
 
 class SitemapSpiderTest(SpiderTest):