More robust file closing in feed storages.

scrapy/extensions/feedexport.py

@@ -250,20 +250,22 @@ class S3FeedStorage(BlockingFeedStorage):
 
     def _store_in_thread(self, file: IO[bytes]) -> None:
         file.seek(0)
-        if self.acl:
-            self.s3_client.upload_fileobj(
-                Bucket=self.bucketname,
-                Key=self.keyname,
-                Fileobj=file,
-                ExtraArgs={"ACL": self.acl},
-            )
-        else:
-            self.s3_client.upload_fileobj(
-                Bucket=self.bucketname,
-                Key=self.keyname,
-                Fileobj=file,
-            )
-        file.close()
+        try:
+            if self.acl:
+                self.s3_client.upload_fileobj(
+                    Bucket=self.bucketname,
+                    Key=self.keyname,
+                    Fileobj=file,
+                    ExtraArgs={"ACL": self.acl},
+                )
+            else:
+                self.s3_client.upload_fileobj(
+                    Bucket=self.bucketname,
+                    Key=self.keyname,
+                    Fileobj=file,
+                )
+        finally:
+            file.close()
 
 
 class GCSFeedStorage(BlockingFeedStorage):

scrapy/utils/ftp.py

@@ -1,4 +1,5 @@
 import posixpath
+from contextlib import closing
 from ftplib import FTP, error_perm
 from posixpath import dirname
 from typing import IO
@@ -32,7 +33,7 @@ def ftp_store_file(
     """Opens a FTP connection with passed credentials,sets current directory
     to the directory extracted from given path, then uploads the file to server
     """
-    with FTP() as ftp:
+    with FTP() as ftp, closing(file):
         ftp.connect(host, port)
         ftp.login(username, password)
         if use_active_mode:
@@ -42,4 +43,3 @@ def ftp_store_file(
         ftp_makedirs_cwd(ftp, dirname)
         command = "STOR" if overwrite else "APPE"
         ftp.storbinary(f"{command} {filename}", file)
-        file.close()

tests/test_feedexport.py

@@ -87,6 +87,7 @@ class DummyBlockingFeedStorage(BlockingFeedStorage):
 
 class FailingBlockingFeedStorage(DummyBlockingFeedStorage):
     def _store_in_thread(self, file):
+        file.close()
         raise OSError("Cannot store")
 
 
@@ -479,7 +480,7 @@ class TestFeedExport(TestFeedExportBase):
         crawler = get_crawler(ItemSpider, settings)
         with mock.patch(
             "scrapy.extensions.feedexport.FileFeedStorage.store",
-            side_effect=KeyError("foo"),
+            side_effect=KeyError("foo"),  # TODO this needs file.close()
         ):
             yield crawler.crawl(mockserver=self.mockserver)
         assert "feedexport/failed_count/FileFeedStorage" in crawler.stats.get_stats()