Prevent create_instance() returning None (#4532)

Currently create_instance() can return None if an extension is
incorrectly implemented, but the extension will still show up as
enabled in the logs. This can cause confusion, as in the linked bug.

This change prevents this occurring by throwing an error if
create_instance() will return None.

scrapy/utils/misc.py

@@ -137,17 +137,26 @@ def create_instance(objcls, settings, crawler, *args, **kwargs):
     ``*args`` and ``**kwargs`` are forwarded to the constructors.
 
     Raises ``ValueError`` if both ``settings`` and ``crawler`` are ``None``.
+
+    Raises ``TypeError`` if the resulting instance is ``None`` (e.g. if an
+    extension has not been implemented correctly).
     """
     if settings is None:
         if crawler is None:
             raise ValueError("Specify at least one of settings and crawler.")
         settings = crawler.settings
     if crawler and hasattr(objcls, 'from_crawler'):
-        return objcls.from_crawler(crawler, *args, **kwargs)
+        instance = objcls.from_crawler(crawler, *args, **kwargs)
+        method_name = 'from_crawler'
     elif hasattr(objcls, 'from_settings'):
-        return objcls.from_settings(settings, *args, **kwargs)
+        instance = objcls.from_settings(settings, *args, **kwargs)
+        method_name = 'from_settings'
     else:
-        return objcls(*args, **kwargs)
+        instance = objcls(*args, **kwargs)
+        method_name = '__new__'
+    if instance is None:
+        raise TypeError("%s.%s returned None" % (objcls.__qualname__, method_name))
+    return instance
 
 
 @contextmanager

tests/test_utils_misc/__init__.py

@@ -114,8 +114,12 @@ class UtilsMiscTestCase(unittest.TestCase):
         #   2. with from_settings() constructor
         #   3. with from_crawler() constructor
         #   4. with from_settings() and from_crawler() constructor
-        spec_sets = ([], ['from_settings'], ['from_crawler'],
-                     ['from_settings', 'from_crawler'])
+        spec_sets = (
+            ['__qualname__'],
+            ['__qualname__', 'from_settings'],
+            ['__qualname__', 'from_crawler'],
+            ['__qualname__', 'from_settings', 'from_crawler'],
+        )
         for specs in spec_sets:
             m = mock.MagicMock(spec_set=specs)
             _test_with_settings(m, settings)
@@ -123,7 +127,7 @@ class UtilsMiscTestCase(unittest.TestCase):
             _test_with_crawler(m, settings, crawler)
 
         # Check adoption of crawler settings
-        m = mock.MagicMock(spec_set=['from_settings'])
+        m = mock.MagicMock(spec_set=['__qualname__', 'from_settings'])
         create_instance(m, None, crawler, *args, **kwargs)
         m.from_settings.assert_called_once_with(crawler.settings, *args,
                                                 **kwargs)
@@ -131,6 +135,10 @@ class UtilsMiscTestCase(unittest.TestCase):
         with self.assertRaises(ValueError):
             create_instance(m, None, None)
 
+        m.from_settings.return_value = None
+        with self.assertRaises(TypeError):
+            create_instance(m, settings, None)
+
     def test_set_environ(self):
         assert os.environ.get('some_test_environ') is None
         with set_environ(some_test_environ='test_value'):