fix(MusicService): extend cache-corruption recovery to malformed container parse errors#812
Open
adam-adrian wants to merge 3 commits into
Open
Conversation
cached media as cache corruption
Reported in the field: "Source error / Code: 3001 / ParserException:
Invalid integer size: 87 {contentIsMalformed=true, dataType=1}".
Error code 3001 is ERROR_CODE_PARSING_CONTAINER_MALFORMED, not an IO
error, so isCacheCorruptionError() returned early and never purged the
cache. The error fell through to isRetryableRemoteParserFailure(), which
only re-prepares (and is gated by !isFullyCachedMedia) without removing
the corrupted cached segment, so re-prepare re-read the same bad bytes
and the user kept hitting 3001 on revisit.
Expand isCacheCorruptionError() to also match
ERROR_CODE_PARSING_CONTAINER_MALFORMED / _UNSUPPORTED and detect
ParserException / "Invalid integer size" / "Skipping atom with length" /
contentIsMalformed in the cause chain. To avoid hijacking genuinely
malformed *remote* streams, the parse-error branch only counts as cache
corruption when the content is actually cached; an isContentCached hint
(hasAnyCachedData, including partial spans) is passed from onPlayerError.
Remote-only parse failures still fall through to the existing
re-resolve-URL retry path.
…e recovery Remove PlaybackException.ERROR_CODE_PARSING_CONTAINER_UNSUPPORTED (3002) from isCacheCorruptionError(). An unsupported container represents a structural codec limitation, not file/cache corruption. Purging and re-downloading would not help in this case, so we narrow the check to strictly ERROR_CODE_PARSING_CONTAINER_MALFORMED (3001).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
This is a follow-up PR that extends the newly merged cache-corruption recovery path in
MusicServiceto handle container-parsing errors (such as the 3001ParserExceptionexperienced in the field) when they occur on cached media.Previously,
isCacheCorruptionError()only listened forERROR_CODE_IO_UNSPECIFIEDandERROR_CODE_IO_READ_POSITION_OUT_OF_RANGE. This meant that a corrupted or truncated cached MP4 file throwing a3001container-parse error (ERROR_CODE_PARSING_CONTAINER_MALFORMED) would bypass the recovery block, fall back to the remote parser retry path (which simply retries without purging the cache), and cause an infinite playback failure/retry loop for the user on revisit.This PR solves that by:
isCacheCorruptionError()on both IO and container-parse errors.hasAnyCachedDatasignal (which checks if any cached spans, including partial ones, exist) to ensure we only treat parse failures as cache corruption when there are cached bytes on disk.Linked Work
Change Type
Affected Surfaces
:app:innertube:betterlyrics:kugou:lrclib:lastfm:simpmusic:paxsenix:unison:canvas:shazamkit:spotifycoreserverfastlane.githubMusicService.ktplayback error-handling path onlyScreenshots / Recordings
Behavior Notes
3001), we check if there are cached spans (fully or partially) for the media ID.hasAnyCachedData == true) andisCacheCorruptionErrormatches aParserExceptionor typical corrupt atom messages ("Invalid integer size", "Skipping atom with length", "contentIsMalformed=true"), we mark it as cache corruption.isRetryableRemoteParserFailure()to re-resolve the URL.!isLocalMedia).Architecture Checklist
Loading,Success,Empty, andErrorwhere this PR introduces or changes screen state.runBlockingis introduced in app execution paths.Compose / Material Checklist
collectAsStateWithLifecycle().@Immutableor@Stable.keyvalues and explicitcontentType.derivedStateOfwhere appropriate.stringResource()and duplicated visible strings on the same screen are avoided.WindowInsetscorrectly when this PR changes screen layout.Concurrency / Performance Checklist
viewModelScopeor an existing lifecycle-owned application/service scope.Dispatchers.IOorDispatchers.Default.Data / Persistence Checklist
app/schemas.Playback / Integration Checklist
universal,arm64,armeabi,x86, andx86_64variants.Localization / Assets Checklist
Privacy / Security Checklist
Verification
Reviewer Focus
moe.rukamori.archivetune.playback.MusicService.kt:isCacheCorruptionError(): Check the added conditions for container-parse errors and howisContentCached(fromhasAnyCachedData) protects remote-only streams from having their cache cleared pointlessly.hasAnyCachedData: Check the lightweight query ofgetCachedSpanson bothdownloadCacheandplayerCacheto verify partial downloads are accounted for.Release Notes
Fixed an issue where playback would get stuck in a failure loop with error code 3001 (ParserException) when encountering a corrupted or truncated song file in the cache. ArchiveTune now automatically clears the bad segment and retries.