Remaining fixes from types experiment (#839)

CHANGELOG.md

@@ -1,6 +1,22 @@
 
 # Changelog
 
+## Version 4.0.5 - Unreleased
+
+* Musicbrainz fill-in feature should be less crash-prone
+* Ability to force Twitch chat bot to post rather than reply
+* Twitch bot permissions should be more reliable
+* Quite a few small bug fixes all over that could potentially lead to crashes
+* Dependency fixes as usual
+* Some log cleanup
+* Minor perf enhancements here and there
+* Experimental: duration/duration_hhmmss
+* keep menu item running longer so users do not think the app is actually
+  shut down
+* document the windows media support
+* change source code line length to 100
+* change how plugins are defined
+
 ## Version 4.0.4 - 2023-05-07
 
 * Experimental feature: Given an option to use Musicbrainz to fill in missing

nowplaying/imagecache.py

@@ -145,7 +145,7 @@ def random_image_fetch(self, artist, imagetype):
             try:
                 image = self.cache[data['cachekey']]
             except KeyError as error:
-                logging.error('random: %s', error)
+                logging.error('random: cannot fetch key %s', error)
                 self.erase_cachekey(data['cachekey'])
             if image:
                 break
@@ -345,7 +345,6 @@ def erase_url(self, url):
         with sqlite3.connect(self.databasefile, timeout=30) as connection:
             connection.row_factory = sqlite3.Row
             cursor = connection.cursor()
-            logging.debug('Delete %s for reasons', url)
             try:
                 cursor.execute('DELETE FROM artistsha WHERE url=?;', (url, ))
             except sqlite3.OperationalError:

nowplaying/metadata.py

@@ -16,6 +16,8 @@
 
 import nowplaying.config
 import nowplaying.hostmeta
+import nowplaying.musicbrainz
+import nowplaying.utils
 import nowplaying.vendor.audio_metadata
 from nowplaying.vendor.audio_metadata.formats.mp4_tags import MP4FreeformDecoders
 
@@ -24,7 +26,7 @@ class MetadataProcessors:  # pylint: disable=too-few-public-methods
     ''' Run through a bunch of different metadata processors '''
 
     def __init__(self, config=None):
-        self.metadata = None
+        self.metadata = {}
         self.imagecache = None
         if config:
             self.config = config
@@ -33,7 +35,10 @@ def __init__(self, config=None):
 
     async def getmoremetadata(self, metadata=None, imagecache=None, skipplugins=False):
         ''' take metadata and process it '''
-        self.metadata = metadata
+        if metadata:
+            self.metadata = metadata
+        else:
+            self.metadata = {}
         self.imagecache = imagecache
 
         if 'artistfanarturls' not in self.metadata:
@@ -72,7 +77,7 @@ async def getmoremetadata(self, metadata=None, imagecache=None, skipplugins=Fals
         return self.metadata
 
     def _fix_duration(self):
-        if not self.metadata.get('duration'):
+        if not self.metadata or not self.metadata.get('duration'):
             return
 
         try:
@@ -85,18 +90,20 @@ def _fix_duration(self):
         self.metadata['duration'] = duration
 
     def _strip_identifiers(self):
+        if not self.metadata:
+            return
 
         if self.config.cparser.value('settings/stripextras',
                                      type=bool) and self.metadata.get('title'):
             self.metadata['title'] = nowplaying.utils.titlestripper_advanced(
                 title=self.metadata['title'], title_regex_list=self.config.getregexlist())
 
     def _uniqlists(self):
+        if not self.metadata:
+            return
 
         if self.metadata.get('artistwebsites'):
-            newlist = [
-                url_normalize.url_normalize(url) for url in self.metadata.get('artistwebsites')
-            ]
+            newlist = [url_normalize.url_normalize(url) for url in self.metadata['artistwebsites']]
             self.metadata['artistwebsites'] = newlist
 
         lists = ['artistwebsites', 'isrc', 'musicbrainzartistid']
@@ -119,6 +126,9 @@ def _uniqlists(self):
 
     def _process_hostmeta(self):
         ''' add the host metadata so other subsystems can use it '''
+        if self.metadata is None:
+            self.metadata = {}
+
         if self.config.cparser.value('weboutput/httpenabled', type=bool):
             self.metadata['httpport'] = self.config.cparser.value('weboutput/httpport', type=int)
         hostmeta = nowplaying.hostmeta.gethostmeta()
@@ -130,7 +140,7 @@ def _process_audio_metadata(self):
 
     def _process_tinytag(self):
         ''' given a chunk of metadata, try to fill in more '''
-        if not self.metadata.get('filename'):
+        if not self.metadata or not self.metadata.get('filename'):
             return
 
         try:
@@ -163,14 +173,18 @@ def _process_tinytag(self):
     def _process_image2png(self):
         # always convert to png
 
-        if 'coverimageraw' not in self.metadata or not self.metadata['coverimageraw']:
+        if not self.metadata or 'coverimageraw' not in self.metadata or not self.metadata[
+                'coverimageraw']:
             return
 
         self.metadata['coverimageraw'] = nowplaying.utils.image2png(self.metadata['coverimageraw'])
         self.metadata['coverimagetype'] = 'png'
         self.metadata['coverurl'] = 'cover.png'
 
     def _musicbrainz(self):
+        if not self.metadata:
+            return None
+
         musicbrainz = nowplaying.musicbrainz.MusicBrainzHelper(config=self.config)
         metalist = musicbrainz.providerinfo()
 
@@ -204,7 +218,7 @@ def _mb_fallback(self):
         ''' at least see if album can be found '''
 
         # user does not want fallback support
-        if not self.config.cparser.value('musicbrainz/fallback', type=bool):
+        if not self.metadata or not self.config.cparser.value('musicbrainz/fallback', type=bool):
             return
 
         # either missing key data or has already been processed
@@ -251,24 +265,27 @@ async def _process_plugins(self):
             with concurrent.futures.ThreadPoolExecutor(max_workers=3,
                                                        thread_name_prefix='artistextras') as pool:
                 for plugin in self.config.plugins['artistextras']:
-                    metalist = self.config.pluginobjs['artistextras'][plugin].providerinfo()
-                    loop = asyncio.get_running_loop()
-                    tasks.append(
-                        loop.run_in_executor(
-                            pool, self.config.pluginobjs['artistextras'][plugin].download,
-                            self.metadata, self.imagecache))
+                    try:
+                        metalist = self.config.pluginobjs['artistextras'][plugin].providerinfo()
+                        loop = asyncio.get_running_loop()
+                        tasks.append(
+                            loop.run_in_executor(
+                                pool, self.config.pluginobjs['artistextras'][plugin].download,
+                                self.metadata, self.imagecache))
 
-            for task in tasks:
-                try:
-                    if addmeta := await task:
-                        self.metadata = recognition_replacement(config=self.config,
-                                                                metadata=self.metadata,
-                                                                addmeta=addmeta)
+                    except Exception as error:  # pylint: disable=broad-except
+                        logging.error('%s threw exception %s', plugin, error, exc_info=True)
 
-                except Exception as error:  # pylint: disable=broad-except
-                    logging.error('%s threw exception %s', plugin, error, exc_info=True)
+            for task in tasks:
+                if addmeta := await task:
+                    self.metadata = recognition_replacement(config=self.config,
+                                                            metadata=self.metadata,
+                                                            addmeta=addmeta)
 
     def _generate_short_bio(self):
+        if not self.metadata:
+            return
+
         message = self.metadata['artistlongbio']
         message = message.replace('\n', ' ')
         message = message.replace('\r', ' ')
@@ -286,11 +303,15 @@ class AudioMetadataRunner:  # pylint: disable=too-few-public-methods
     ''' run through audio_metadata '''
 
     def __init__(self, config=None):
-        self.metadata = None
+        self.metadata = {}
         self.config = config
 
     def process(self, metadata):
         ''' process it '''
+
+        if not metadata:
+            return metadata
+
         if not metadata.get('filename'):
             return metadata
 
@@ -345,6 +366,10 @@ def _itunes(tempdata, freeform):
                                                 addmeta=tempdata)
 
     def _process_audio_metadata_id3_usertext(self, usertextlist):
+
+        if not self.metadata:
+            self.metadata = {}
+
         for usertext in usertextlist:
             if usertext.description == 'Acoustid Id':
                 self.metadata['acoustidid'] = usertext.text[0]
@@ -357,7 +382,10 @@ def _process_audio_metadata_id3_usertext(self, usertextlist):
             elif usertext.description == 'originalyear':
                 self.metadata['date'] = usertext.text[0]
 
-    def _process_audio_metadata_othertags(self, tags):
+    def _process_audio_metadata_othertags(self, tags):  # pylint: disable=too-many-branches
+        if not self.metadata:
+            self.metadata = {}
+
         if 'discnumber' in tags and 'disc' not in self.metadata:
             text = tags['discnumber'][0].replace('[', '').replace(']', '')
             try:
@@ -388,6 +416,8 @@ def _process_audio_metadata_othertags(self, tags):
             self._process_audio_metadata_id3_usertext(tags.usertext)
 
     def _process_audio_metadata_remaps(self, tags):
+        if not self.metadata:
+            self.metadata = {}
 
         # single:
 
@@ -420,6 +450,9 @@ def _process_audio_metadata_remaps(self, tags):
                     self.metadata[dest] = [str(tags[src])]
 
     def _process_audio_metadata(self):  # pylint: disable=too-many-branches
+        if not self.metadata or not self.metadata.get('filename'):
+            return
+
         try:
             base = nowplaying.vendor.audio_metadata.load(self.metadata['filename'])
         except Exception as error:  # pylint: disable=broad-except
@@ -468,6 +501,9 @@ def recognition_replacement(config=None, metadata=None, addmeta=None):
     if not addmeta:
         return metadata
 
+    if not metadata:
+        metadata = {}
+
     for meta in addmeta:
         if meta in ['artist', 'title', 'artistwebsites']:
             if config.cparser.value(f'recognition/replace{meta}', type=bool) and addmeta.get(meta):

nowplaying/musicbrainz.py

@@ -90,6 +90,7 @@ def lastditcheffort(self, metadata):
             return None
 
         self._setemail()
+        mydict = {}
 
         addmeta = {
             'artist': metadata.get('artist'),
@@ -144,6 +145,7 @@ def isrc(self, isrclist):
             return None
 
         self._setemail()
+        mbdata = {}
 
         for isrc in isrclist:
             try:
@@ -210,7 +212,7 @@ def releaselookup_noartist(recordingid):
                 try:
                     mbdata = musicbrainzngs.browse_releases(recording=recordingid,
                                                             includes=['labels', 'artist-credits'])
-                except:  # pylint: disable=bare-except
+                except Exception as error:  # pylint: disable=broad-except
                     logging.error('MusicBrainz threw an error: %s', error)
                     return None
             return mbdata

nowplaying/trackrequests.py

@@ -189,16 +189,18 @@ def normalize(crazystring):
         ''' user input needs to be normalized for best case matches '''
         if not crazystring:
             return ''
-        return normality.normalize(crazystring).replace(' ', '')
+        if text := normality.normalize(crazystring):
+            return text.replace(' ', '')
+        return ''
 
     async def add_to_db(self, data):
         ''' add an entry to the db '''
         if not self.databasefile.exists():
             logging.error('%s does not exist, refusing to add.', self.databasefile)
             return
 
-        data['normalizedartist'] = self.normalize(data.get('artist'))
-        data['normalizedtitle'] = self.normalize(data.get('title'))
+        data['normalizedartist'] = self.normalize(data.get('artist', ''))
+        data['normalizedtitle'] = self.normalize(data.get('title', ''))
 
         if data.get('reqid'):
             reqid = data['reqid']
@@ -305,6 +307,7 @@ async def _find_good_request(self, setting):
         plugin = self.config.cparser.value('settings/input')
         tryagain = True
         counter = 10
+        metadata = None
         while tryagain and counter > 0:
             counter -= 1
             roulette = await self.config.pluginobjs['inputs'][f'nowplaying.inputs.{plugin}'
@@ -313,7 +316,12 @@ async def _find_good_request(self, setting):
                 config=self.config).getmoremetadata(metadata={'filename': roulette},
                                                     skipplugins=True)
 
-            if not artistdupes or metadata.get('artist') not in artistdupes:
+            if not metadata:
+                logging.error('Did not get any metadata from %s', roulette)
+                continue
+
+            if not artistdupes or (metadata.get('artist')
+                                   and metadata['artist'] not in artistdupes):
                 tryagain = False
                 await asyncio.sleep(.5)
             if tryagain:

nowplaying/uihelp.py

@@ -10,6 +10,10 @@ class UIHelp:
     ''' utility functions for GUI code'''
 
     def __init__(self, config, qtui):
+        if not config:
+            raise AssertionError('config cannot be empty')
+        if not qtui:
+            raise AssertionError('qtui cannot be empty')
         self.config = config
         self.qtui = qtui
 

nowplaying/upgrade.py

@@ -211,11 +211,18 @@ def preload(self):
         if shafile.exists():
             with open(shafile, encoding='utf-8') as fhin:
                 self.oldshas = json.loads(fhin.read())
+        else:
+            logging.error('%s file is missing.', shafile)
 
     def check_preload(self, filename, userhash):
         ''' check if the given file matches a known hash '''
         found = None
         hexdigest = None
+
+        if not self.oldshas:
+            logging.error('updateshas.json file was not loaded.')
+            return None
+
         if filename in self.oldshas:
             for version, hexdigest in self.oldshas[filename].items():
                 if userhash == hexdigest:
@@ -250,8 +257,7 @@ def setup_templates(self):
                              self.usertemplatedir)
                 continue
 
-            destpath = str(userpath).replace('.txt', '.new')
-            destpath = pathlib.Path(destpath.replace('.htm', '.new'))
+            destpath = userpath.with_suffix('.new')
             if destpath.exists():
                 userhash = checksum(destpath)
                 if apphash == userhash:

nowplaying/upgradeutils.py

@@ -51,7 +51,7 @@ def __init__(self, version):
     def _calculate(self):
         olddict = copy.copy(self.chunk)
         for key, value in olddict.items():
-            if value and value.isdigit():
+            if isinstance(value, str) and value.isdigit():
                 self.chunk[key] = int(value)
 
         if self.chunk.get('rc') or self.chunk.get('commitnum'):
@@ -103,7 +103,7 @@ def __init__(self, testmode=False):
         if not testmode:
             self.get_versions()
 
-    def get_versions(self, testdata=None):
+    def get_versions(self, testdata=None):  # pylint: disable=too-many-branches
         ''' ask github about current versions '''
         try:
             if not testdata:
@@ -124,6 +124,10 @@ def get_versions(self, testdata=None):
             else:
                 jsonreldata = testdata
 
+            if not jsonreldata:
+                logging.error('No data from Github. Aborting.')
+                return
+
             for rel in jsonreldata:
                 if not isinstance(rel, dict):
                     logging.error(rel)