From 0e78acb65798a1eb4e55a472232e77d55e6c7cd5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io>
Date: Thu, 21 Dec 2023 21:01:07 +0100
Subject: [PATCH] MediaPipeline: log media_to_download errors before stripping
 them (#5068)

---
 scrapy/pipelines/media.py    | 10 +++++-----
 tests/test_pipeline_crawl.py | 24 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py
index 75532034a..fc156ab41 100644
--- a/scrapy/pipelines/media.py
+++ b/scrapy/pipelines/media.py
@@ -112,14 +112,14 @@ class MediaPipeline:
         info.downloading.add(fp)
         dfd = mustbe_deferred(self.media_to_download, request, info, item=item)
         dfd.addCallback(self._check_media_to_download, request, info, item=item)
+        dfd.addErrback(self._log_exception)
         dfd.addBoth(self._cache_result_and_execute_waiters, fp, info)
-        dfd.addErrback(
-            lambda f: logger.error(
-                f.value, exc_info=failure_to_exc_info(f), extra={"spider": info.spider}
-            )
-        )
         return dfd.addBoth(lambda _: wad)  # it must return wad at last
 
+    def _log_exception(self, result):
+        logger.exception(result)
+        return result
+
     def _modify_media_request(self, request):
         if self.handle_httpstatus_list:
             request.meta["handle_httpstatus_list"] = self.handle_httpstatus_list
diff --git a/tests/test_pipeline_crawl.py b/tests/test_pipeline_crawl.py
index ed8483483..c41ab483f 100644
--- a/tests/test_pipeline_crawl.py
+++ b/tests/test_pipeline_crawl.py
@@ -9,6 +9,7 @@ from w3lib.url import add_or_replace_parameter
 
 from scrapy import signals
 from scrapy.crawler import CrawlerRunner
+from scrapy.utils.misc import load_object
 from tests.mockserver import MockServer
 from tests.spiders import SimpleSpider
 
@@ -193,6 +194,29 @@ class FileDownloadCrawlTestCase(TestCase):
             crawler.stats.get_value("downloader/response_status_count/302"), 3
         )
 
+    @defer.inlineCallbacks
+    def test_download_media_file_path_error(self):
+        cls = load_object(self.pipeline_class)
+
+        class ExceptionRaisingMediaPipeline(cls):
+            def file_path(self, request, response=None, info=None, *, item=None):
+                return 1 / 0
+
+        settings = {
+            **self.settings,
+            "ITEM_PIPELINES": {ExceptionRaisingMediaPipeline: 1},
+        }
+        runner = CrawlerRunner(settings)
+        crawler = self._create_crawler(MediaDownloadSpider, runner=runner)
+        with LogCapture() as log:
+            yield crawler.crawl(
+                self.mockserver.url("/files/images/"),
+                media_key=self.media_key,
+                media_urls_key=self.media_urls_key,
+                mockserver=self.mockserver,
+            )
+        self.assertIn("ZeroDivisionError", str(log))
+
 
 skip_pillow: Optional[str]
 try: