Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ef4004566
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if *reconnect_attempts < MAX_RECONNECT_ATTEMPTS { | ||
| *reconnect_attempts += 1; | ||
| return Some((resume_offset, true)); |
There was a problem hiding this comment.
Reset the retry budget after successful progress
The retry counter only increases and is never reset after a resumed response successfully transfers data. Consequently, a long track that experiences three recoverable disconnects at widely separated offsets will fail immediately on its fourth disconnect, even though every prior reconnection succeeded; the limit should apply to consecutive failures, with the counter reset once the resumed request makes progress.
Useful? React with 👍 / 👎.
| if state.end_offset != start_offset | ||
| || matches!((state.total_size, total_size), (Some(old), Some(new)) if old != new) | ||
| { |
There was a problem hiding this comment.
Bind resumed ranges to the original representation
When the URL's resource changes between the interrupted response and the retry, this check accepts the resumed body solely because its offset and optional total size match. A replacement object with the same length is therefore appended to the buffered prefix of the old object, silently producing mixed audio; retain a response validator such as a strong ETag and validate it or use If-Range before preserving the buffer.
Useful? React with 👍 / 👎.
| fn parse_content_range_start(response: &reqwest::Response) -> Option<u64> { | ||
| let value = response.headers().get(CONTENT_RANGE)?.to_str().ok()?; | ||
| let (unit, range) = value.trim().split_once(' ')?; | ||
| if !unit.eq_ignore_ascii_case("bytes") { | ||
| return None; | ||
| } | ||
| range.split_once('-')?.0.parse().ok() |
There was a problem hiding this comment.
Verify that resumed partial responses reach their advertised end
The new range validation retains only the response's starting offset. If a resumed 206 uses chunked transfer encoding and terminates cleanly before the end declared by Content-Range, response.chunk() returns EOF rather than a transport error, so the loop marks the shorter wire_offset complete and playback silently ends early instead of retrying or reporting truncation; parse the range end and compare it with the final offset.
Useful? React with 👍 / 👎.
变更
1.0.0-alpha.5验证
cargo test --all-targetscargo clippy --lib --bins --test cli --test truncated_http --test online_playback -- -D warningscargo build --releaserustfmt --edition 2024 --check rust/src/stream.rsgit diff --check