diff --git a/admin/templates/download_events/download_events.html b/admin/templates/download_events/download_events.html
index bb1563a39ec..0149733d2a8 100644
--- a/admin/templates/download_events/download_events.html
+++ b/admin/templates/download_events/download_events.html
@@ -92,7 +92,7 @@
Download telemetry dashboard
{{ download_events_dashboard.split.file.gb|floatformat:2 }} / {{ download_events_dashboard.split.zip.gb|floatformat:2 }}
-
Failed zip downloads (server error)
+
Failed zip downloads (server or provider error)
{{ download_events_dashboard.summary.failed_zips }}
diff --git a/osf/admin.py b/osf/admin.py
index 284b8695f66..ad63d6bd03d 100644
--- a/osf/admin.py
+++ b/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):
diff --git a/tests/test_download_events_dashboard.py b/tests/test_download_events_dashboard.py
index 5cdfefabcad..2d54c1f6edb 100644
--- a/tests/test_download_events_dashboard.py
+++ b/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):