Handle unsupported ID3 frames in MP3 Quick Cleanse (remove TENC, add safeSetFrame)#31
Merged
ChrisAdamsdevelopment merged 1 commit intoMay 18, 2026
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideMakes MP3 Quick Cleanse metadata writing resilient to unsupported ID3 frames by introducing a safe frame setter, routing supported frames through it, and removing the known-unsupported TENC frame write while preserving existing error handling behavior. Sequence diagram for safeSetFrame usage in writeMP3MetadatasequenceDiagram
participant QC as QuickCleanse
participant writeMP3Metadata
participant safeSetFrame
participant writer
participant console
QC->>writeMP3Metadata: writeMP3Metadata(file, metadata)
writeMP3Metadata->>safeSetFrame: safeSetFrame(TIT2, title)
safeSetFrame->>writer: setFrame(TIT2, title)
alt frame_supported
writer-->>safeSetFrame: success
safeSetFrame-->>writeMP3Metadata: true
else frame_unsupported
writer-->>safeSetFrame: throw error
safeSetFrame->>console: warn([quick-cleanse] skipped unsupported ID3 frame)
safeSetFrame-->>writeMP3Metadata: false
end
writeMP3Metadata->>safeSetFrame: safeSetFrame(TPE1, [artist])
writeMP3Metadata->>safeSetFrame: safeSetFrame(TCON, [genre])
writeMP3Metadata->>writer: addTag()
writer-->>writeMP3Metadata: Blob
writeMP3Metadata-->>QC: Blob
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
safeSetFramehelper currently swallows all errors fromwriter.setFrame; consider narrowing the catch to only the known unsupported-frame error shape (or at least checking an error code/message) so genuine write failures don’t get silently downgraded to warnings. - If
safeSetFrameis intended to be reused elsewhere, it may be worth extracting it to a shared utility (or at least outsidewriteMP3Metadata) and adding some context (e.g., file name) to the warning to make troubleshooting skipped frames easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `safeSetFrame` helper currently swallows all errors from `writer.setFrame`; consider narrowing the catch to only the known unsupported-frame error shape (or at least checking an error code/message) so genuine write failures don’t get silently downgraded to warnings.
- If `safeSetFrame` is intended to be reused elsewhere, it may be worth extracting it to a shared utility (or at least outside `writeMP3Metadata`) and adding some context (e.g., file name) to the warning to make troubleshooting skipped frames easier.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Motivation
writeMP3Metadataattempted to write an unsupportedTENCID3 frame and thus aborted the whole metadata rewrite.Description
writeMP3Metadatainsrc/utils/metadata.jsto add a localsafeSetFrame(frameId, value)helper that wrapswriter.setFrameintry/catchand logs a warning when a frame is unsupported.TENCframe entirely so the known unsupported frame is no longer written.safeSetFrame:TIT2(title),TPE1(artist), andTCON(genre).addTag,getBlob, unreadable input) so the function still returns aBlobon success or throws a useful error on real failures, and no other files were changed.Testing
npm run build; the build failed due to pre-existing TypeScript/dependency/React typing issues unrelated to this metadata change, so no full app build could be validated here.TENC, and whenTIT2,TPE1, orTCONare present they are written if supported while unsupported frames are skipped with a console warning (no failure).Codex Task
Summary by Sourcery
Improve resilience of MP3 metadata writing in Quick Cleanse by safely handling unsupported ID3 frames and stopping use of a known unsupported frame.
Bug Fixes:
Enhancements: