forslund / mopidy_skill

A Mycroft skill for controlling the mopidy media player
GNU General Public License v3.0
15 stars 14 forks source link

GMusic album parsing can crash skill initialization #26

Open ProjectMoon opened 4 years ago

ProjectMoon commented 4 years ago

Mopidy 3 is returning some entries that look like this:

    {
      "__model__": "Ref",
      "uri": "gmusic:album:<id>",
      "name": "Unknown Album",
      "type": "directory"
    }

The list comprehension assumes the name is Artist - Album. When it encounters an entry like this, the list comprehension crashes and ultimately aborts the entire skill initialization process.

This specifically affects the get_gmusic_artists method in mopidy_post.py.

It could be changed to something like this:

def parse_album(gmusic_name):
    try:
        return e.split(' - ')[1]
    except:
        return e

def get_gmusic_albums(self):
    p = self.browse('gmusic:album')
    p = {e['name']: e for e in p if e['type'] == 'directory'}
    return {parse_album(e): p[e] for e in p}

Of course, then you wind up getting duplicates (I have multiple unique Unknown Albums for some reason). A better temporary solution would probably be to either ignore these failures, or squash them all into one album as far as the skill is concerned.

forslund commented 4 years ago

I need to update my Mopidy it seems. Still running on 2.1.0

I'd be happy to accept a PR for this fix if you feel like it. If not I'll try to tackle it in the coming days. I want to get this in good enough shape to submit to the mycroft-skills repo, so I'm very glad for your bug reports.

ProjectMoon commented 4 years ago

I can prepare a PR yes. Currently I just patched it to ignore the invalid records. There's another unrelated issue I'm investigating too, but it might not be an issue with this code