Fix JsonItemExporter puts lone comma in the output if encoder fails (#5952)

* Partial fix for #3090 - only addresses JSON feeds.

* Adding test case for #3090 to Json Exporter

* Changing the deliberate-fail JSON example to a complex

* Further tightening JsonItemExporter behaviour to prevent corruption.

Based on Mikhail's observation that to_bytes can fail also, leading
to the same dangling comma as the failure to encode to JSON.

Added a new test case to avoid reversion.

* [scrapy] JsonItemExporter puts lone comma in the output if encoder fails

- Add initial changes from cathal's PR
- https://github.com/scrapy/scrapy/issues/3090

* [scrapy] JsonItemExporter puts lone comma in the output if encoder fails

- Handle exception not to add empty item.
- https://github.com/scrapy/scrapy/issues/3090

* [scrapy] JsonItemExporter puts lone comma in the output if encoder fails

- Add comment for handling the exception
- https://github.com/scrapy/scrapy/issues/3090

* [scrapy] JsonItemExporter puts lone comma in the output if encoder fails

- Remove unused import
- https://github.com/scrapy/scrapy/issues/3090

* [scrapy] JsonItemExporter puts lone comma in the output if encoder fails

- Fix invalid json issue
- https://github.com/scrapy/scrapy/issues/3090

* [scrapy] JsonItemExporter puts lone comma in the output if encoder fails

- Perform CR changes
- https://github.com/scrapy/scrapy/issues/3090

---------

Co-authored-by: Cathal Garvey <cathalgarvey@cathalgarvey.me>

scrapy/exporters.py

@@ -133,6 +133,13 @@ class JsonItemExporter(BaseItemExporter):
         if self.indent is not None:
             self.file.write(b"\n")
 
+    def _add_comma_after_first(self):
+        if self.first_item:
+            self.first_item = False
+        else:
+            self.file.write(b",")
+            self._beautify_newline()
+
     def start_exporting(self):
         self.file.write(b"[")
         self._beautify_newline()
@@ -142,14 +149,10 @@ class JsonItemExporter(BaseItemExporter):
         self.file.write(b"]")
 
     def export_item(self, item):
-        if self.first_item:
-            self.first_item = False
-        else:
-            self.file.write(b",")
-            self._beautify_newline()
         itemdict = dict(self._get_serialized_fields(item))
-        data = self.encoder.encode(itemdict)
-        self.file.write(to_bytes(data, self.encoding))
+        data = to_bytes(self.encoder.encode(itemdict), self.encoding)
+        self._add_comma_after_first()
+        self.file.write(data)
 
 
 class XmlItemExporter(BaseItemExporter):

tests/test_exporters.py

@@ -599,6 +599,20 @@ class JsonItemExporterTest(JsonLinesItemExporterTest):
     def test_two_dict_items(self):
         self.assertTwoItemsExported(ItemAdapter(self.i).asdict())
 
+    def test_two_items_with_failure_between(self):
+        i1 = TestItem(name="Joseph\xa3", age="22")
+        i2 = TestItem(
+            name="Maria", age=1j
+        )  # Invalid datetimes didn't consistently fail between Python versions
+        i3 = TestItem(name="Jesus", age="44")
+        self.ie.start_exporting()
+        self.ie.export_item(i1)
+        self.assertRaises(TypeError, self.ie.export_item, i2)
+        self.ie.export_item(i3)
+        self.ie.finish_exporting()
+        exported = json.loads(to_unicode(self.output.getvalue()))
+        self.assertEqual(exported, [dict(i1), dict(i3)])
+
     def test_nested_item(self):
         i1 = self.item_class(name="Joseph\xa3", age="22")
         i2 = self.item_class(name="Maria", age=i1)
@@ -637,6 +651,24 @@ class JsonItemExporterTest(JsonLinesItemExporterTest):
         self.assertEqual(exported, [item])
 
 
+class JsonItemExporterToBytesTest(BaseItemExporterTest):
+    def _get_exporter(self, **kwargs):
+        kwargs["encoding"] = "latin"
+        return JsonItemExporter(self.output, **kwargs)
+
+    def test_two_items_with_failure_between(self):
+        i1 = TestItem(name="Joseph", age="22")
+        i2 = TestItem(name="\u263a", age="11")
+        i3 = TestItem(name="Jesus", age="44")
+        self.ie.start_exporting()
+        self.ie.export_item(i1)
+        self.assertRaises(UnicodeEncodeError, self.ie.export_item, i2)
+        self.ie.export_item(i3)
+        self.ie.finish_exporting()
+        exported = json.loads(to_unicode(self.output.getvalue(), encoding="latin"))
+        self.assertEqual(exported, [dict(i1), dict(i3)])
+
+
 class JsonItemExporterDataclassTest(JsonItemExporterTest):
     item_class = TestDataClass
     custom_field_item_class = CustomFieldDataclass