pdbprovider: improve error handling for partially broken dbs

Author: flesniak

prodj/data/dataprovider.py

@@ -6,12 +6,7 @@
 from .datastore import DataStore
 from .dbclient import DBClient
 from .pdbprovider import PDBProvider
-
-class TemporaryQueryError(Exception):
-  pass
-
-class FatalQueryError(Exception):
-  pass
+from .exceptions import TemporaryQueryError, FatalQueryError
 
 class DataProvider(Thread):
   def __init__(self, prodj):

prodj/data/dbclient.py

@@ -5,6 +5,7 @@
 
 from prodj.network import packets
 from prodj.data import dataprovider
+from prodj.data.exceptions import FatalQueryError, TemporaryQueryError
 from prodj.pdblib.usbanlz import AnlzTag
 
 metadata_type = {
@@ -249,7 +250,7 @@ def receive_dbmessage(self, sock):
       except (StreamError, RangeError, TypeError) as e:
         logging.debug("Received %d bytes but parsing failed, trying to receive more", len(data))
         parse_errors += 1
-    raise dataprovider.TemporaryQueryError("Failed to receive dbmessage after {} tries and {} timeouts".format(parse_errors, receive_timeouts))
+    raise TemporaryQueryError("Failed to receive dbmessage after {} tries and {} timeouts".format(parse_errors, receive_timeouts))
 
   def query_list(self, player_number, slot, sort_mode, id_list, request_type):
     sock = self.getSocket(player_number)
@@ -340,7 +341,7 @@ def query_list(self, player_number, slot, sort_mode, id_list, request_type):
         else:
           break
     if parse_errors >= self.parse_error_count or receive_timeouts >= self.receive_timeout_count:
-      raise dataprovider.FatalQueryError("Failed to receive {} render reply after {} timeouts, {} parse errors".format(request_type, receive_timeouts, parse_errors))
+      raise FatalQueryError("Failed to receive {} render reply after {} timeouts, {} parse errors".format(request_type, receive_timeouts, parse_errors))
 
     # basically, parse_metadata returns a single dict whereas parse_list returns a list of dicts
     if request_type in ["metadata_request", "mount_info_request", "track_info_request"]:
@@ -395,7 +396,7 @@ def get_server_port(self, player_number):
     if player_number not in self.remote_ports:
       client = self.prodj.cl.getClient(player_number)
       if client is None:
-        raise dataprovider.TemporaryQueryError("failed to get remote port, player {} unknown".format(player_number))
+        raise TemporaryQueryError("failed to get remote port, player {} unknown".format(player_number))
       sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
       sock.connect((client.ip_addr, packets.DBServerQueryPort))
       sock.send(packets.DBServerQuery.build({}))
@@ -425,7 +426,7 @@ def send_setup_packet(self, sock, player_number):
     sock.send(packets.DBMessage.build(query))
     data = sockrcv(sock, 48)
     if len(data) == 0:
-      raise dataprovider.TemporaryQueryError("Failed to connect to player {}".format(player_number))
+      raise TemporaryQueryError("Failed to connect to player {}".format(player_number))
     reply = packets.DBMessage.parse(data)
     logging.info("connected to player {}".format(reply["args"][1]["value"]))
 
@@ -479,19 +480,19 @@ def socksnd(self, sock, data):
     except BrokenPipeError as e:
       player_number = next((n for n, d in self.socks.items() if d[0] == sock), None)
       if player_number is None:
-        raise dataprovider.FatalQueryError("socksnd failed with unknown sock")
+        raise FatalQueryError("socksnd failed with unknown sock")
       else:
         self.closeSocket(player_number)
-        raise dataprovider.TemporaryQueryError("Connection to player {} lost".format(player_number))
+        raise TemporaryQueryError("Connection to player {} lost".format(player_number))
 
   def ensure_request_possible(self, request, player_number):
     client = self.prodj.cl.getClient(player_number)
     if client is None:
-      raise dataprovider.TemporaryQueryError("player {} not found in clientlist".format(player_number))
+      raise TemporaryQueryError("player {} not found in clientlist".format(player_number))
     critical_requests = ["metadata_request", "artwork_request", "preview_waveform_request", "beatgrid_request", "waveform_request"]
     critical_play_states = ["no_track", "loading_track", "cannot_play_track", "emergency"]
     if request in critical_requests and client.play_state in critical_play_states:
-      raise dataprovider.TemporaryQueryError("DataProvider: delaying %s request due to play state: %s".format(request, client.play_state))
+      raise TemporaryQueryError("DataProvider: delaying %s request due to play state: %s".format(request, client.play_state))
 
   def handle_request(self, request, params):
     self.ensure_request_possible(request, params[0])
@@ -544,10 +545,10 @@ def handle_request(self, request, params):
       try: # pre-parse beatgrid data (like metadata) for easier access
         return packets.Beatgrid.parse(reply)["beats"]
       except (RangeError, FieldError) as e:
-        raise dataprovider.FatalQueryError("failed to parse beatgrid data: {}".format(e))
+        raise FatalQueryError("failed to parse beatgrid data: {}".format(e))
     elif request == "mount_info":
       return self.query_list(*params[:2], None, [params[2]], "mount_info_request")
     elif request == "track_info":
       return self.query_list(*params[:2], None, [params[2]], "track_info_request")
     else:
-      raise dataprovider.FatalQueryError("invalid request type {}".format(request))
+      raise FatalQueryError("invalid request type {}".format(request))

prodj/data/exceptions.py

@@ -0,0 +1,5 @@
+class TemporaryQueryError(Exception):
+  pass
+
+class FatalQueryError(Exception):
+  pass

prodj/data/pdbprovider.py

@@ -1,8 +1,8 @@
 import logging
 import os
 
-from . import dataprovider
-from .datastore import DataStore
+from prodj.data.exceptions import FatalQueryError
+from prodj.data.datastore import DataStore
 from prodj.pdblib.pdbdatabase import PDBDatabase
 from prodj.pdblib.usbanlzdatabase import UsbAnlzDatabase
 from prodj.network.rpcreceiver import ReceiveTimeout
@@ -16,6 +16,15 @@ def __init__(self, reason):
   def __str__(self):
     return self.reason
 
+def wrap_get_name_from_db(call, id):
+  if id == 0:
+    return ""
+  try:
+    return call(id).name
+  except KeyError as e:
+    logging.warning(f'Broken database: {e}')
+    return "?"
+
 class PDBProvider:
   def __init__(self, prodj):
     self.prodj = prodj
@@ -39,7 +48,7 @@ def delete_pdb(self, filename):
   def download_pdb(self, player_number, slot):
     player = self.prodj.cl.getClient(player_number)
     if player is None:
-      raise dataprovider.FatalQueryError("player {} not found in clientlist".format(player_number))
+      raise FatalQueryError("player {} not found in clientlist".format(player_number))
     filename = "databases/player-{}-{}.pdb".format(player_number, slot)
     self.delete_pdb(filename)
     try:
@@ -49,7 +58,7 @@ def download_pdb(self, player_number, slot):
         logging.debug("default pdb path not found on player %d, trying MacOS path", player_number)
         self.prodj.nfs.enqueue_download(player.ip_addr, slot, "/.PIONEER/rekordbox/export.pdb", filename, sync=True)
     except (RuntimeError, ReceiveTimeout) as e:
-      raise dataprovider.FatalQueryError("database download from player {} failed: {}".format(player_number, e))
+      raise FatalQueryError("database download from player {} failed: {}".format(player_number, e))
     return filename
 
   def download_and_parse_pdb(self, player_number, slot):
@@ -58,27 +67,27 @@ def download_and_parse_pdb(self, player_number, slot):
     try:
       db.load_file(filename)
     except RuntimeError as e:
-      raise dataprovider.FatalQueryError("PDBProvider: failed to parse \"{}\": {}".format(filename, e))
+      raise FatalQueryError("PDBProvider: failed to parse \"{}\": {}".format(filename, e))
     return db
 
   def get_db(self, player_number, slot):
     if (player_number, slot) not in self.dbs:
       try:
         db = self.download_and_parse_pdb(player_number, slot)
-      except dataprovider.FatalQueryError as e:
+      except FatalQueryError as e:
         db = InvalidPDBDatabase(str(e))
       finally:
         self.dbs[player_number, slot] = db
     else:
       db = self.dbs[player_number, slot]
     if isinstance(db, InvalidPDBDatabase):
-      raise dataprovider.FatalQueryError(f'PDB database not available: {db}')
+      raise FatalQueryError(f'PDB database not available: {db}')
     return db
 
   def download_and_parse_usbanlz(self, player_number, slot, anlz_path):
     player = self.prodj.cl.getClient(player_number)
     if player is None:
-      raise dataprovider.FatalQueryError("player {} not found in clientlist".format(player_number))
+      raise FatalQueryError("player {} not found in clientlist".format(player_number))
     dat = self.prodj.nfs.enqueue_buffer_download(player.ip_addr, slot, anlz_path)
     ext = self.prodj.nfs.enqueue_buffer_download(player.ip_addr, slot, anlz_path.replace("DAT", "EXT"))
     db = UsbAnlzDatabase()
@@ -99,16 +108,15 @@ def get_anlz(self, player_number, slot, track_id):
   def get_metadata(self, player_number, slot, track_id):
     db = self.get_db(player_number, slot)
     track = db.get_track(track_id)
-    artist = db.get_artist(track.artist_id).name if track.artist_id > 0 else ""
-    album = db.get_album(track.album_id).name if track.album_id > 0 else ""
-    key = db.get_key(track.key_id).name if track.key_id > 0 else ""
-    genre = db.get_genre(track.genre_id).name if track.genre_id > 0 else ""
-    color_name = colors[track.color_id] if track.color_id > 0 else ""
-    if track.color_id > 0:
-      color = db.get_color(track.color_id)
-      color_text = color.name
-    else:
-      color_text = ""
+    artist = wrap_get_name_from_db(db.get_artist, track.artist_id)
+    album = wrap_get_name_from_db(db.get_album, track.album_id)
+    key = wrap_get_name_from_db(db.get_key, track.key_id)
+    genre = wrap_get_name_from_db(db.get_genre, track.genre_id)
+    color_text = wrap_get_name_from_db(db.get_color, track.color_id)
+
+    color_name = ""
+    if track.color_id in range(1, len(colors)):
+      color_name = colors[track.color_id]
 
     metadata = {
       "track_id": track.id,
@@ -135,7 +143,7 @@ def get_metadata(self, player_number, slot, track_id):
   def get_artwork(self, player_number, slot, artwork_id):
     player = self.prodj.cl.getClient(player_number)
     if player is None:
-      raise dataprovider.FatalQueryError("player {} not found in clientlist".format(player_number))
+      raise FatalQueryError("player {} not found in clientlist".format(player_number))
     db = self.get_db(player_number, slot)
     try:
       artwork = db.get_artwork(artwork_id)
@@ -222,30 +230,26 @@ def convert_and_sort_track_list(self, db, track_list, sort_mode):
     else:
       col2_name = sort_mode
     for track in track_list:
-      try:
-        if col2_name in ["title", "artist"]:
-          col2_item = db.get_artist(track.artist_id).name if track.artist_id > 0 else ""
-        elif col2_name == "album":
-          col2_item = db.get_album(track.album_id).name if track.album_id > 0 else ""
-        elif col2_name == "genre":
-          col2_item = db.get_genre(track.genre_id).name if track.genre_id > 0 else ""
-        elif col2_name == "label":
-          col2_item = db.get_label(track.label_id).name if track.label_id > 0 else ""
-        elif col2_name == "original_artist":
-          col2_item = db.get_artist(track.original_artist_id).name if track.original_artist_id > 0 else ""
-        elif col2_name == "remixer":
-          col2_item = db.get_artist(track.remixer_id).name if track.remixer_id > 0 else ""
-        elif col2_name == "key":
-          col2_item = db.get_key(track.key_id).name if track.key_id > 0 else ""
-        elif col2_name == "bpm":
-          col2_item = track.bpm_100/100
-        elif col2_name in ["rating", "comment", "duration", "bitrate", "play_count"]: # 1:1 mappings
-          col2_item = track[col2_name]
-        else:
-          raise dataprovider.FatalQueryError("unknown sort mode {}".format(sort_mode))
-      except KeyError as e:
-        logging.warning(f'Broken database: {e}')
-        col2_item = "?"
+      if col2_name in ["title", "artist"]:
+        col2_item = wrap_get_name_from_db(db.get_artist, track.artist_id)
+      elif col2_name == "album":
+        col2_item = wrap_get_name_from_db(db.get_album, track.album_id)
+      elif col2_name == "genre":
+        col2_item = wrap_get_name_from_db(db.get_genre, track.genre_id)
+      elif col2_name == "label":
+        col2_item = wrap_get_name_from_db(db.get_label, track.label_id)
+      elif col2_name == "original_artist":
+        col2_item = wrap_get_name_from_db(db.get_artist, track.original_artist_id)
+      elif col2_name == "remixer":
+        col2_item = wrap_get_name_from_db(db.get_artist, track.remixer_id)
+      elif col2_name == "key":
+        col2_item = wrap_get_name_from_db(db.get_key, track.key_id)
+      elif col2_name == "bpm":
+        col2_item = track.bpm_100/100
+      elif col2_name in ["rating", "comment", "duration", "bitrate", "play_count"]: # 1:1 mappings
+        col2_item = track[col2_name]
+      else:
+        raise FatalQueryError("unknown sort mode {}".format(sort_mode))
       converted += [{
         "title": track.title,
         col2_name: col2_item,
@@ -399,4 +403,4 @@ def handle_request(self, request, params):
     elif request == "mount_info":
       return self.get_mount_info(*params)
     else:
-      raise dataprovider.FatalQueryError("invalid request type {}".format(request))
+      raise FatalQueryError("invalid request type {}".format(request))