fix(osf): count zip 4xx errors as failed, not cancelled

admin/templates/download_events/download_events.html

@@ -92,7 +92,7 @@ <h2>Download telemetry dashboard</h2>
                     <div class="card-value compact-value">{{ download_events_dashboard.split.file.gb|floatformat:2 }} / {{ download_events_dashboard.split.zip.gb|floatformat:2 }}</div>
                 </div>
                 <div class="dashboard-card summary-card">
-                    <div class="card-label">Failed zip downloads (server error)</div>
+                    <div class="card-label">Failed zip downloads (server or provider error)</div>
                     <div class="card-value">{{ download_events_dashboard.summary.failed_zips }}</div>
                 </div>
                 <div class="dashboard-card summary-card">

osf/admin.py

@@ -408,10 +408,12 @@ def user(self, obj):
     user.short_description = 'User'
 
 
-# A zip that WaterButler ended on a 5xx failed through no fault of the user (the server
-# buckled) -- distinct from a cancel, which ends completed=False at 200 because the headers
-# already went out. This is the threshold that separates the two.
-DOWNLOAD_FAILURE_MIN_STATUS = 500
+# A zip that didn't complete failed through no fault of the user whenever it ended on an
+# error status -- a 4xx (e.g. an add-on provider returning 404 for the resource) or a 5xx
+# (the server buckled). A cancel is the other case: completed=False at a success status
+# (200/302), because the headers already went out and the client aborted mid-stream. This
+# threshold is what separates a real failure from a cancel.
+DOWNLOAD_FAILURE_MIN_STATUS = 400
 
 
 class DownloadOutcomeFilter(SimpleListFilter):
@@ -428,7 +430,7 @@ def lookups(self, request, model_admin):
         return [
             (self.COMPLETED, 'Completed'),
             (self.CANCELLED, 'Cancelled mid-download'),
-            (self.FAILED, 'Failed (server error)'),
+            (self.FAILED, 'Failed (server or provider error)'),
         ]
 
     def queryset(self, request, queryset):

tests/test_download_events_dashboard.py

@@ -136,7 +136,7 @@ def test_totals_and_split(self):
         assert data['split']['zip']['count_percent'] == 50
 
     def test_zip_outcomes_split_completed_cancelled_failed(self):
-        # a completed zip, a user cancel (False at 200), and a server failure (False at 5xx)
+        # a completed zip, a user cancel (False at 200), and a failure (False at an error status)
         make_event(download_type=DownloadEvent.FOLDER_ZIP, zip_completed=True, status_code=200)
         make_event(download_type=DownloadEvent.FOLDER_ZIP, zip_completed=False, status_code=200)
         make_event(download_type=DownloadEvent.PROJECT, zip_completed=False, status_code=502)
@@ -147,15 +147,19 @@ def test_zip_outcomes_split_completed_cancelled_failed(self):
 
         assert outcomes == {'completed': 1, 'cancelled': 1, 'failed': 1}
 
-    def test_failed_zips_summary_counts_only_server_errors(self):
+    def test_failed_zips_summary_counts_error_statuses(self):
+        # any 4xx or 5xx that didn't complete is a failure; a 404 from an add-on provider
+        # is just as much a failure as a 500 from our server
+        make_event(download_type=DownloadEvent.FOLDER_ZIP, zip_completed=False, status_code=404)
         make_event(download_type=DownloadEvent.FOLDER_ZIP, zip_completed=False, status_code=500)
         make_event(download_type=DownloadEvent.FOLDER_ZIP, zip_completed=False, status_code=503)
+        # a cancel (ended on a success status) must NOT count as failed
         make_event(download_type=DownloadEvent.FOLDER_ZIP, zip_completed=False, status_code=200)
         make_event(download_type=DownloadEvent.FOLDER_ZIP, zip_completed=True, status_code=200)
 
         data = self.admin.get_dashboard_data(DownloadEvent.objects.all())
 
-        assert data['summary']['failed_zips'] == 2
+        assert data['summary']['failed_zips'] == 3
 
     def test_incomplete_zip_without_a_status_counts_as_cancelled(self):
         """Callbacks from a WaterButler build predating status_code have status None — treat
@@ -169,12 +173,15 @@ def test_incomplete_zip_without_a_status_counts_as_cancelled(self):
     def test_outcome_column_labels(self):
         completed = make_event(download_type=DownloadEvent.FOLDER_ZIP, zip_completed=True)
         cancelled = make_event(download_type=DownloadEvent.FOLDER_ZIP, zip_completed=False, status_code=200)
-        failed = make_event(download_type=DownloadEvent.PROJECT, zip_completed=False, status_code=500)
+        failed_5xx = make_event(download_type=DownloadEvent.PROJECT, zip_completed=False, status_code=500)
+        # an add-on provider 404 is a failure, not a cancel
+        failed_404 = make_event(download_type=DownloadEvent.FOLDER_ZIP, zip_completed=False, status_code=404)
         single = make_event(download_type=DownloadEvent.FILE)
 
         assert self.admin.outcome(completed) == 'Completed'
         assert self.admin.outcome(cancelled) == 'Cancelled'
-        assert self.admin.outcome(failed) == 'Failed'
+        assert self.admin.outcome(failed_5xx) == 'Failed'
+        assert self.admin.outcome(failed_404) == 'Failed'
         assert self.admin.outcome(single) == '—'
 
     def test_storage_provider_breakdown(self):