aggregate_metadata shouldn't catch all exceptions
Closed this issue · 0 comments
aggregate_metadata
in manage.py has a bare except
clause, catching all manner of errors that mutagen.File
might issue. We shouldn't catch all exceptions — that's bad. For one thing pressing Ctrl-C to try to abort the database update issues a KeyboardInterrupt
exception, which this catches. You end up having to press Ctrl-C repeatedly till you get lucky.
Unfortunately, Mutagen doesn't make it easy to do this right. Its exceptions are not documented anywhere, and looking at the source code, they don't all inherit from a shared base class, either: the ID3 errors inherit from ValueError and NotImplementedError while FLAC errors inherit from IOError. Fun. Since the errors are badly behaved, the best option might be to use a patched Mutagen that behaves better and look at trying to get the patch accepted upstream. Or short-term we could catch ValueError, NotImplementedError and IOError and report the actual class of the error received. Hopefully that would cover all these.