From a07444a9181cd74bcb88f41124fe038d564b5e58 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Sun, 21 Jun 2026 18:14:22 +0500 Subject: [PATCH] More robust file closing in feed storages. --- scrapy/extensions/feedexport.py | 30 ++++++++++++++++-------------- scrapy/utils/ftp.py | 4 ++-- tests/test_feedexport.py | 3 ++- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 8029f85c9..fa708a2e1 100644 --- a/scrapy/extensions/feedexport.py +++ b/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): diff --git a/scrapy/utils/ftp.py b/scrapy/utils/ftp.py index 152f3374e..a3e7a4306 100644 --- a/scrapy/utils/ftp.py +++ b/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() diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index c1d6f04eb..3f503cb83 100644 --- a/tests/test_feedexport.py +++ b/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()