graue/potsfyi

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.