fetchart: handle FilesystemError when setting album art#6516
fetchart: handle FilesystemError when setting album art#6516lawrence3699 wants to merge 1 commit intobeetbox:masterfrom
Conversation
When set_art encounters a permissions error or cross-device move failure (e.g. file locked by foobar2000 on Windows), the unhandled FilesystemError crashes the import or the fetchart CLI command. Wrap both _set_art call sites in try/except FilesystemError, matching the pattern already used in the cleanup method (line ~520). On failure, log a warning and continue instead of aborting. Fixes beetbox#6193.
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
There was a problem hiding this comment.
Pull request overview
PR make fetchart not crash when filesystem say “no” during set_art (Windows lock, cross-device move, etc). PR fit in beetsplug fetchart reliability: keep import + CLI running, just warn and move on.
Changes:
- Catch
util.FilesystemErroraround_set_artin import hook (assign_art) and CLI path (batch_fetch_art). - On CLI error, print “error setting art” instead of throwing.
- Add tests for both error paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
beetsplug/fetchart.py |
Wrap _set_art calls with try/except util.FilesystemError and log + continue/return. |
test/plugins/test_fetchart.py |
Add regression tests for FilesystemError in CLI and import assign path. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6516 +/- ##
==========================================
+ Coverage 70.40% 70.42% +0.02%
==========================================
Files 148 148
Lines 18806 18814 +8
Branches 3067 3067
==========================================
+ Hits 13240 13250 +10
+ Misses 4916 4913 -3
- Partials 650 651 +1
🚀 New features to boost your workflow:
|
snejus
left a comment
There was a problem hiding this comment.
Looks good. Just fix the formatting issues and add a note to the changelog please.
Description
Fixes #6193.
Problem
When
fetchartencounters aFilesystemErrorduringset_art(e.g. a file locked by another process on Windows, or a cross-device move permission error), the unhandled exception crashes the import session or thefetchartCLI command.The
cleanupmethod at line ~520 already catchesFilesystemErrorand logs it gracefully, but the two_set_artcall sites don't.Fix
Wrap both
_set_artcall sites intry/except util.FilesystemError:assign_art()(import pipeline): log the error and return early, skipping the prune step since no art was set.batch_fetch_art()(CLI command): log the error and show "error setting art" in the output instead of crashing.Tests
Added two tests covering both error paths:
test_batch_fetch_filesystem_error— verifies the CLI command reports the error message instead of crashing.test_assign_art_filesystem_error— verifies the import path returns gracefully on error.All existing fetchart tests pass.
To Do
ChangelogDocumentation