Skip to content
This repository has been archived by the owner on Oct 20, 2018. It is now read-only.

Account for mutagen.File returning None for unreadable files #13

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

hedgepigdaniel
Copy link

It seems to be possible for a file to be detected as a music file by gmusicapi-wrapper but for mutagen to detect it as a non music file. This results in mutagen.File returning None which eventually is treated as a dictionary, causing an unhandled exception.

This change checks the return value of mutagen.File and handles None similarly to how exceptions are currently handled (filename is logged and added to the filtered files list).

@thebigmunch
Copy link
Owner

Thanks for the contribution.

Can you post a file (or a few) that mutagen.File returns None for? I'd like to get a better idea of what is going on to get that result and see if another library handles them.

The solution will likely be different than what you've proposed; I don't think I'd want to pass along a file whose type can't be detected properly. And, if I did, I'd want it to be an option defaulting to False.

@hedgepigdaniel
Copy link
Author

Sure. I did some further investigation and it came to my attention that the file... was literally empty. So to reproduce, just create an empty file and give it a supported extension.

Seems like mutagen does it's own file type detection, which may (as in this case) conflict with the filetype detection that gmusicapi-wrapper does (based on extension, in get_supported_filepaths). http://mutagen.readthedocs.io/en/latest/api/base.html#mutagen.File

@thebigmunch
Copy link
Owner

The checking in get_supported_filepaths is a quick out. The mutagen.File call here is for checking validity.

I can't reproduce getting None returned from mutagen.File with an empty file created using touch test.mp3. It properly throws a mutagen.mp3.HeaderNotFoundError which is a subclass of mutagen.MutagenError and is properly caught by the except clause. So, please post a file that you know to cause this or possibly more details that might be pertinent.

@thebigmunch
Copy link
Owner

Just from quickly testing the other supported extensions, it looks like Ogg and MP4 don't throw an exception. This seems like something that should happen with mutagen.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants