From 8d1e0e09bb6fdeb4f1348b408a268c92dc9e7a8f Mon Sep 17 00:00:00 2001 From: Mabel Villalba Date: Thu, 20 Jun 2019 10:06:06 +0200 Subject: [PATCH 01/11] [itemloader-errors] added error message in get_value --- scrapy/loader/__init__.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/scrapy/loader/__init__.py b/scrapy/loader/__init__.py index 20f0f90c3..5055de015 100644 --- a/scrapy/loader/__init__.py +++ b/scrapy/loader/__init__.py @@ -106,11 +106,17 @@ class ItemLoader(object): value = arg_to_iter(value) value = flatten(extract_regex(regex, x) for x in value) - for proc in processors: + for _proc in processors: if value is None: break - proc = wrap_loader_context(proc, self.context) - value = proc(value) + proc = wrap_loader_context(_proc, self.context) + try: + value = proc(value) + except Exception as e: + raise ValueError( + "Error with processor %s value=%r error='%s: %s'" % + (_proc.__class__.__name__, value, type(e).__name__, + str(e))) return value def load_item(self): From 663352b2a5250c377bbbe2d53c5d5b7da3a1836a Mon Sep 17 00:00:00 2001 From: Mabel Villalba Date: Thu, 20 Jun 2019 10:10:16 +0200 Subject: [PATCH 02/11] [itemloader-errors] added error message to _process_input_value --- scrapy/loader/__init__.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/scrapy/loader/__init__.py b/scrapy/loader/__init__.py index 5055de015..fc8b10bee 100644 --- a/scrapy/loader/__init__.py +++ b/scrapy/loader/__init__.py @@ -155,9 +155,15 @@ class ItemLoader(object): return proc def _process_input_value(self, field_name, value): - proc = self.get_input_processor(field_name) - proc = wrap_loader_context(proc, self.context) - return proc(value) + _proc = self.get_input_processor(field_name) + proc = wrap_loader_context(_proc, self.context) + try: + return proc(value) + except Exception as e: + raise ValueError( + "Error with inputput processor %s: field=%r value=%r " + "error='%s: %s'" % (_proc.__class__.__name__, field_name, + value, type(e).__name__, str(e))) def _get_item_field_attr(self, field_name, key, default=None): if isinstance(self.item, Item): From 859008a10b8e126844662867195c6823c4a59e4f Mon Sep 17 00:00:00 2001 From: Mabel Villalba Date: Thu, 20 Jun 2019 10:13:59 +0200 Subject: [PATCH 03/11] [itemloader-errors] added error message to Compose and MapCompose Fixes issue #3836 --- scrapy/loader/__init__.py | 16 ++++++++-------- scrapy/loader/processors.py | 17 ++++++++++++++--- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/scrapy/loader/__init__.py b/scrapy/loader/__init__.py index fc8b10bee..ec4102b11 100644 --- a/scrapy/loader/__init__.py +++ b/scrapy/loader/__init__.py @@ -128,14 +128,6 @@ class ItemLoader(object): return item - def get_output_value(self, field_name): - proc = self.get_output_processor(field_name) - proc = wrap_loader_context(proc, self.context) - try: - return proc(self._values[field_name]) - except Exception as e: - raise ValueError("Error with output processor: field=%r value=%r error='%s: %s'" % \ - (field_name, self._values[field_name], type(e).__name__, str(e))) def get_collected_values(self, field_name): return self._values[field_name] @@ -145,6 +137,14 @@ class ItemLoader(object): if not proc: proc = self._get_item_field_attr(field_name, 'input_processor', \ self.default_input_processor) + def get_output_value(self, field_name): + proc = self.get_output_processor(field_name) + proc = wrap_loader_context(proc, self.context) + try: + return proc(self._values[field_name]) + except Exception as e: + raise ValueError("Error with output processor: field=%r value=%r error='%s: %s'" % \ + (field_name, self._values[field_name], type(e).__name__, str(e))) return proc def get_output_processor(self, field_name): diff --git a/scrapy/loader/processors.py b/scrapy/loader/processors.py index bf7c74bfe..85ac2c832 100644 --- a/scrapy/loader/processors.py +++ b/scrapy/loader/processors.py @@ -25,8 +25,13 @@ class MapCompose(object): for func in wrapped_funcs: next_values = [] for v in values: - next_values += arg_to_iter(func(v)) - values = next_values + try: + next_values += arg_to_iter(func(v)) + except Exception as e: + raise ValueError("Error in MapCompose with " + "function %s value=%r error='%s: %s'" % + (func.__name__, value, + type(e).__name__, str(e))) return values @@ -46,7 +51,13 @@ class Compose(object): for func in wrapped_funcs: if value is None and self.stop_on_none: break - value = func(value) + try: + value = func(value) + except Exception as e: + raise ValueError("Error in Compose with " + "function %s value=%r error='%s: %s'" % + (func.__name__, value, + type(e).__name__, str(e))) return value From e5d17b4efe23a617eacadf602559e481641d3590 Mon Sep 17 00:00:00 2001 From: Mabel Villalba Date: Tue, 25 Jun 2019 13:42:40 +0200 Subject: [PATCH 04/11] [itemloader-errors] reordered method #3836 --- scrapy/loader/__init__.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/scrapy/loader/__init__.py b/scrapy/loader/__init__.py index ec4102b11..38afa951a 100644 --- a/scrapy/loader/__init__.py +++ b/scrapy/loader/__init__.py @@ -128,6 +128,15 @@ class ItemLoader(object): return item + def get_output_value(self, field_name): + proc = self.get_output_processor(field_name) + proc = wrap_loader_context(proc, self.context) + try: + return proc(self._values[field_name]) + except Exception as e: + raise ValueError("Error with output processor: field=%r value=%r error='%s: %s'" % \ + (field_name, self._values[field_name], type(e).__name__, str(e))) + return proc def get_collected_values(self, field_name): return self._values[field_name] @@ -137,14 +146,6 @@ class ItemLoader(object): if not proc: proc = self._get_item_field_attr(field_name, 'input_processor', \ self.default_input_processor) - def get_output_value(self, field_name): - proc = self.get_output_processor(field_name) - proc = wrap_loader_context(proc, self.context) - try: - return proc(self._values[field_name]) - except Exception as e: - raise ValueError("Error with output processor: field=%r value=%r error='%s: %s'" % \ - (field_name, self._values[field_name], type(e).__name__, str(e))) return proc def get_output_processor(self, field_name): From f134b1daf3fb36372f8c1755aaa4c29ac65aced3 Mon Sep 17 00:00:00 2001 From: Mabel Villalba Date: Tue, 25 Jun 2019 13:45:06 +0200 Subject: [PATCH 05/11] [itemloader-errors] reordered method #3836 --- scrapy/loader/__init__.py | 1 - scrapy/loader/processors.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/loader/__init__.py b/scrapy/loader/__init__.py index 38afa951a..fc8b10bee 100644 --- a/scrapy/loader/__init__.py +++ b/scrapy/loader/__init__.py @@ -136,7 +136,6 @@ class ItemLoader(object): except Exception as e: raise ValueError("Error with output processor: field=%r value=%r error='%s: %s'" % \ (field_name, self._values[field_name], type(e).__name__, str(e))) - return proc def get_collected_values(self, field_name): return self._values[field_name] diff --git a/scrapy/loader/processors.py b/scrapy/loader/processors.py index 85ac2c832..56f2bc564 100644 --- a/scrapy/loader/processors.py +++ b/scrapy/loader/processors.py @@ -32,6 +32,7 @@ class MapCompose(object): "function %s value=%r error='%s: %s'" % (func.__name__, value, type(e).__name__, str(e))) + values = next_values return values From ad8c980010b60d1d9ca3eb30c27fb60751b10508 Mon Sep 17 00:00:00 2001 From: Mabel Villalba Date: Tue, 25 Jun 2019 13:54:47 +0200 Subject: [PATCH 06/11] [itemloader-errors] undo _proc #3836 --- scrapy/loader/__init__.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/scrapy/loader/__init__.py b/scrapy/loader/__init__.py index fc8b10bee..30ff5a717 100644 --- a/scrapy/loader/__init__.py +++ b/scrapy/loader/__init__.py @@ -106,17 +106,17 @@ class ItemLoader(object): value = arg_to_iter(value) value = flatten(extract_regex(regex, x) for x in value) - for _proc in processors: + for proc in processors: if value is None: break - proc = wrap_loader_context(_proc, self.context) + _proc = proc + proc = wrap_loader_context(proc, self.context) try: value = proc(value) except Exception as e: - raise ValueError( - "Error with processor %s value=%r error='%s: %s'" % - (_proc.__class__.__name__, value, type(e).__name__, - str(e))) + raise ValueError("Error with processor %s value=%r error='%s: %s'" % + (_proc.__class__.__name__, value, + type(e).__name__, str(e))) return value def load_item(self): @@ -155,8 +155,9 @@ class ItemLoader(object): return proc def _process_input_value(self, field_name, value): - _proc = self.get_input_processor(field_name) - proc = wrap_loader_context(_proc, self.context) + proc = self.get_input_processor(field_name) + _proc = proc + proc = wrap_loader_context(proc, self.context) try: return proc(value) except Exception as e: From ef56e34a4a9e02dd4bd02b3b82c5bd9b24453e49 Mon Sep 17 00:00:00 2001 From: Mabel Villalba Date: Tue, 25 Jun 2019 13:56:53 +0200 Subject: [PATCH 07/11] [itemloader-errors] fixed typo #3836 --- scrapy/loader/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/loader/__init__.py b/scrapy/loader/__init__.py index 30ff5a717..844e3828c 100644 --- a/scrapy/loader/__init__.py +++ b/scrapy/loader/__init__.py @@ -162,7 +162,7 @@ class ItemLoader(object): return proc(value) except Exception as e: raise ValueError( - "Error with inputput processor %s: field=%r value=%r " + "Error with input processor %s: field=%r value=%r " "error='%s: %s'" % (_proc.__class__.__name__, field_name, value, type(e).__name__, str(e))) From 99eb03a810273b7451d0ce3614d18b20cb604912 Mon Sep 17 00:00:00 2001 From: Mabel Villalba Date: Tue, 25 Jun 2019 14:17:54 +0200 Subject: [PATCH 08/11] [itemloader-errors] adapted compose test for the new error being returned by compose #3836 --- tests/test_loader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_loader.py b/tests/test_loader.py index 5a8ee1b2e..230242592 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -482,7 +482,7 @@ class ProcessorsTest(unittest.TestCase): proc = Compose(str.upper) self.assertEqual(proc(None), None) proc = Compose(str.upper, stop_on_none=False) - self.assertRaises(TypeError, proc, None) + self.assertRaises(ValueError, proc, None) def test_mapcompose(self): filter_world = lambda x: None if x == 'world' else x From a753ea7e161e049a090d24b0807a4f20ad7cf36f Mon Sep 17 00:00:00 2001 From: Mabel Villalba Date: Tue, 25 Jun 2019 14:35:49 +0200 Subject: [PATCH 09/11] [itemloader-errors] added test for Compose and MapCompose #3836 --- tests/test_loader.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_loader.py b/tests/test_loader.py index 230242592..0f9070b8c 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -483,12 +483,21 @@ class ProcessorsTest(unittest.TestCase): self.assertEqual(proc(None), None) proc = Compose(str.upper, stop_on_none=False) self.assertRaises(ValueError, proc, None) + proc = Compose(str.upper, lambda x: x + 1) + self.assertRaises(ValueError, proc, 'hello') def test_mapcompose(self): filter_world = lambda x: None if x == 'world' else x proc = MapCompose(filter_world, six.text_type.upper) self.assertEqual(proc([u'hello', u'world', u'this', u'is', u'scrapy']), [u'HELLO', u'THIS', u'IS', u'SCRAPY']) + proc = MapCompose(filter_world, six.text_type.upper) + self.assertEqual(proc(None), []) + proc = MapCompose(filter_world, six.text_type.upper) + self.assertRaises(ValueError, proc, [1]) + proc = MapCompose(filter_world, lambda x: x + 1) + self.assertRaises(ValueError, proc, 'hello') + class SelectortemLoaderTest(unittest.TestCase): From b1e348b2813880c616191d0135e759f4edcec64c Mon Sep 17 00:00:00 2001 From: Mabel Villalba Date: Tue, 25 Jun 2019 17:00:40 +0200 Subject: [PATCH 10/11] [itemloader-errors] updated Compose and MapCompose messages and added tests #3836 --- scrapy/loader/processors.py | 12 ++++++++---- tests/test_loader.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/scrapy/loader/processors.py b/scrapy/loader/processors.py index 56f2bc564..4b6d8624c 100644 --- a/scrapy/loader/processors.py +++ b/scrapy/loader/processors.py @@ -28,10 +28,12 @@ class MapCompose(object): try: next_values += arg_to_iter(func(v)) except Exception as e: + type_name = type(func).__name__ + _name = (func.__name__ if type_name == 'function' + else type_name) raise ValueError("Error in MapCompose with " "function %s value=%r error='%s: %s'" % - (func.__name__, value, - type(e).__name__, str(e))) + (_name, value, type(e).__name__, str(e))) values = next_values return values @@ -55,10 +57,12 @@ class Compose(object): try: value = func(value) except Exception as e: + type_name = type(func).__name__ + _name = (func.__name__ if type_name == 'function' + else type_name) raise ValueError("Error in Compose with " "function %s value=%r error='%s: %s'" % - (func.__name__, value, - type(e).__name__, str(e))) + (_name, value, type(e).__name__, str(e))) return value diff --git a/tests/test_loader.py b/tests/test_loader.py index 0f9070b8c..ce0fa0701 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -456,6 +456,42 @@ class BasicItemLoaderTest(unittest.TestCase): 'title': [u'Test item title 3', u'Test item 4'], }) + def test_error_input_processor(self): + class TestItem(Item): + name = Field() + + class TestItemLoader(ItemLoader): + default_item_class = TestItem + name_in = MapCompose(float) + + il = TestItemLoader() + self.assertRaises(ValueError, il.add_value, 'name', + [u'marta', u'other']) + + def test_error_output_processor(self): + class TestItem(Item): + name = Field() + + class TestItemLoader(ItemLoader): + default_item_class = TestItem + name_out = Compose(Join(), float) + + il = TestItemLoader() + il.add_value('name', u'marta') + with self.assertRaises(ValueError): + il.load_item() + + def test_error_processor_as_argument(self): + class TestItem(Item): + name = Field() + + class TestItemLoader(ItemLoader): + default_item_class = TestItem + + il = TestItemLoader() + self.assertRaises(ValueError, il.add_value, 'name', + [u'marta', u'other'], Compose(float)) + class ProcessorsTest(unittest.TestCase): From 268a37cb7b40737a855e217e3ebe8a2b91238dba Mon Sep 17 00:00:00 2001 From: Mabel Villalba Date: Wed, 26 Jun 2019 13:23:10 +0200 Subject: [PATCH 11/11] [itemloader-errors] updated Compose and MapCompose function names #3836 --- scrapy/loader/processors.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/scrapy/loader/processors.py b/scrapy/loader/processors.py index 4b6d8624c..468aec2cc 100644 --- a/scrapy/loader/processors.py +++ b/scrapy/loader/processors.py @@ -28,12 +28,10 @@ class MapCompose(object): try: next_values += arg_to_iter(func(v)) except Exception as e: - type_name = type(func).__name__ - _name = (func.__name__ if type_name == 'function' - else type_name) raise ValueError("Error in MapCompose with " - "function %s value=%r error='%s: %s'" % - (_name, value, type(e).__name__, str(e))) + "%s value=%r error='%s: %s'" % + (str(func), value, type(e).__name__, + str(e))) values = next_values return values @@ -57,12 +55,9 @@ class Compose(object): try: value = func(value) except Exception as e: - type_name = type(func).__name__ - _name = (func.__name__ if type_name == 'function' - else type_name) raise ValueError("Error in Compose with " - "function %s value=%r error='%s: %s'" % - (_name, value, type(e).__name__, str(e))) + "%s value=%r error='%s: %s'" % + (str(func), value, type(e).__name__, str(e))) return value