library: include path and reason in ReadError/WriteError messages#6645
library: include path and reason in ReadError/WriteError messages#6645eyupcanakman wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
PR fix bug where ReadError.__str__ / WriteError.__str__ print <super: ...> instead of real message. Now error string show file path plus underlying reason, so user can debug read/write fail.
Changes:
- Fix
ReadErrorandWriteErrorstring formatting to call parent__str__()and include path + reason. - Add regression tests to make sure error message no longer contains
super:and does include underlying reason. - Add changelog entry for bug fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
beets/library/exceptions.py |
Make ReadError / WriteError use super().__str__() so message includes path + reason. |
test/test_library.py |
Add assertions/tests that error strings no longer show <super: ...> and include reason text. |
docs/changelog.rst |
Document fix in Unreleased bug fixes section. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6645 +/- ##
=======================================
Coverage 72.44% 72.45%
=======================================
Files 160 160
Lines 20690 20690
Branches 3272 3272
=======================================
+ Hits 14989 14991 +2
+ Misses 4976 4974 -2
Partials 725 725
🚀 New features to boost your workflow:
|
eyupcanakman
force-pushed
the
fix/file-error-str-super-6560
branch
from
24a8e61 to
5826ce5
Compare
ReadError and WriteError stringified the super() proxy in their f-string, so file read and write failures lost the path and reason. Use super().__str__() to return the FileOperationError message.
eyupcanakman
force-pushed
the
fix/file-error-str-super-6560
branch
from
5826ce5 to
64d6fc1
Compare
|
|
||
| def __str__(self): | ||
| return f"error reading {super()}" | ||
| return f"error reading {super().__str__()}" |
There was a problem hiding this comment.
Hm I thought enclosing something within {} in a f-string calls __str__ method automatically. If not, I think calling str(super()) is a bit cleaner.
There was a problem hiding this comment.
Right for a normal object, but super() returns a proxy and the f-string (or str()) resolves __str__ on the proxy itself, so {super()} and str(super()) both print <super: ...>. Happy to switch it to FileOperationError.__str__(self) if that reads cleaner, that one forwards to the parent correctly.
There was a problem hiding this comment.
I see. It's fine - just add a comment to explain what's going on here, thanks!
Description
ReadError.__str__andWriteError.__str__putsuper()straight into an f-string, so a failed read or write printederror reading <super: <class 'ReadError'>, <ReadError object>>and the actual path and reason were lost. Callingsuper().__str__()returns theFileOperationErrormessage with the path and underlying reason. The reportedbeet importcase now logs the file path and the failure reason.Fixes #6560.
To Do
Documentation