fix(download): stop an empty download completing and then hanging the player
Two halves of one failure, either of which is enough to produce an
offline item that never starts.
The worker marked a transfer `completed` without checking it produced
any bytes, so a server that answered 200 with no body — an error page, a
transcode that yielded nothing — renamed a zero-byte `.part` into place
and published it as available offline. That is worse than failing: the
retry budget never applies and the UI shows the item as ready.
The media server then answered a request for that file with a span of
`{ start: 0, end: 0 }`. `end` is inclusive, so `Span::len()` reported
**one** byte: the response declared `Content-Length: 1` and streamed
nothing, which Chromium's media loader waits on forever. The user sees a
downloaded item that just never plays, with nothing explaining why.
A zero-length file has no satisfiable range, so `span_for` now returns
`None` and the server answers 416. An empty transfer is rejected as a
network error, which keeps the `.part` for a resume and lets the existing
retry budget do its job.
This commit is contained in:
@@ -183,6 +183,16 @@ impl DownloadWorker {
|
||||
.await
|
||||
.map_err(|e| DownloadError::FileSystem(e.to_string()))?;
|
||||
|
||||
// A media file is never legitimately empty, and completing one is worse
|
||||
// than failing: the row goes `completed`, the item shows as available
|
||||
// offline, and playback then stalls on a file with nothing in it. A
|
||||
// server that answered 200 with no body — an error page, a transcode
|
||||
// that produced nothing — used to land here. Treat it as the network
|
||||
// failure it is so the retry budget applies and the `.part` is kept.
|
||||
if let Some(reason) = rejects_as_empty(downloaded) {
|
||||
return Err(DownloadError::Network(reason.to_string()));
|
||||
}
|
||||
|
||||
// Move from .part to final location
|
||||
fs::rename(&temp_path, &task.target_path)
|
||||
.await
|
||||
@@ -229,6 +239,23 @@ pub fn resume_offset(existing_bytes: u64, status: u16) -> u64 {
|
||||
}
|
||||
}
|
||||
|
||||
/// Why a finished transfer must not be accepted, if it must not be.
|
||||
///
|
||||
/// A media file is never legitimately empty, and *completing* an empty one is
|
||||
/// worse than failing: the row goes `completed`, the item shows as available
|
||||
/// offline, and playback later stalls on a file with nothing in it. A server
|
||||
/// that answered 200 with no body — an error page, a transcode that produced
|
||||
/// nothing — used to land exactly there.
|
||||
///
|
||||
/// Reported as a network error so the existing retry budget applies and the
|
||||
/// `.part` file is kept for a resume.
|
||||
///
|
||||
/// TRACES: UR-019 | DR-168 | UT-168
|
||||
pub fn rejects_as_empty(downloaded: u64) -> Option<&'static str> {
|
||||
(downloaded == 0)
|
||||
.then_some("server sent an empty body; refusing to complete a zero-byte download")
|
||||
}
|
||||
|
||||
/// The partial-download sidecar for `target`.
|
||||
///
|
||||
/// **Appends** `.part` rather than replacing the extension. The worker used
|
||||
@@ -305,6 +332,23 @@ impl std::error::Error for DownloadError {}
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// A transfer that produced no bytes must never be marked complete.
|
||||
///
|
||||
/// Completing it publishes an empty file as playable offline; the media
|
||||
/// server then answers a request for it with a 416 and the item simply
|
||||
/// never starts, with nothing in the UI explaining why.
|
||||
///
|
||||
/// TRACES: UR-019 | DR-168 | UT-168
|
||||
#[test]
|
||||
fn test_a_zero_byte_transfer_is_rejected_rather_than_completed() {
|
||||
assert!(
|
||||
rejects_as_empty(0).is_some(),
|
||||
"a zero-byte download must not be completed"
|
||||
);
|
||||
assert!(rejects_as_empty(1).is_none());
|
||||
assert!(rejects_as_empty(4 * 1024 * 1024).is_none());
|
||||
}
|
||||
|
||||
/// The bitrate-download corruption: a transcode ignores `Range` and answers
|
||||
/// `200` with the whole stream. Appending that to the bytes already on disk
|
||||
/// duplicated them, so every retry grew the file past its real size and left
|
||||
|
||||
Reference in New Issue
Block a user