fix: improve error handling, logging and robustness across core modules

pisi/api.py

@@ -753,8 +753,8 @@ def configure_pending(packages=None):
 
     try:
         ctx.exec_usysconf()
-    except Exception as e:
-        raise e
+    except Exception:
+        raise
 
     # Clear legacy "needs configuration" flag
     order = generate_pending_order(packages)

pisi/atomicoperations.py

@@ -147,6 +147,14 @@ def __init__(self, package_fname, ignore_dep=None, ignore_file_conflicts=None):
             self.package.read()
         except zipfile.BadZipfile:
             raise zipfile.BadZipfile(self.package_fname)
+        except FileNotFoundError as e:
+            raise Error(
+                _("Package file not found: '%s'") % package_fname
+            ) from e
+        except OSError as e:
+            raise Error(
+                _("Cannot read package file '%s': %s") % (package_fname, e)
+            ) from e
         self.metadata = self.package.metadata
         self.files = self.package.files
         self.pkginfo = self.metadata.package

pisi/cli/check.py

@@ -154,8 +154,8 @@ def run(self):
             if ctx.ui.confirm(_("Reinstall the broken packages?")):
                 try:
                     pisi.api.install(broken_pkgs, reinstall=True)
-                except Exception as e:
-                    raise e
+                except Exception:
+                    raise
             else:
                 ctx.ui.info(_("Broken packages were not reinstalled."))
 

pisi/db/installdb.py

@@ -70,10 +70,30 @@ def init(self):
 
     def __generate_installed_pkgs(self):
         def split_name(dirname):
-            name, version, release = dirname.rsplit("-", 2)
+            parts = dirname.rsplit("-", 2)
+            if len(parts) != 3:
+                ctx.ui.warning(
+                    _("Skipping malformed package directory: '%s'") % dirname
+                )
+                return None
+            name, version, release = parts
             return name, version + "-" + release
 
-        return dict(list(map(split_name, os.listdir(ctx.config.packages_dir()))))
+        packages_dir = ctx.config.packages_dir()
+        try:
+            entries = os.listdir(packages_dir)
+        except OSError as e:
+            ctx.ui.warning(
+                _("Cannot read packages directory '%s': %s") % (packages_dir, e)
+            )
+            return {}
+
+        result = {}
+        for entry in entries:
+            pair = split_name(entry)
+            if pair is not None:
+                result[pair[0]] = pair[1]
+        return result
 
     def __get_marked_packages(self, _type):
         info_path = os.path.join(ctx.config.info_dir(), _type)
@@ -127,9 +147,16 @@ def list_installed_with_build_host(self, build_host):
         build_host_re = re.compile("<BuildHost>(.*?)</BuildHost>")
         found = []
         for name in self.list_installed():
-            xml = open(
-                os.path.join(self.package_path(name), ctx.const.metadata_xml)
-            ).read()
+            try:
+                with open(
+                    os.path.join(self.package_path(name), ctx.const.metadata_xml)
+                ) as f:
+                    xml = f.read()
+            except OSError as e:
+                ctx.ui.warning(
+                    _("Could not read metadata for package '%s': %s") % (name, e)
+                )
+                continue
             matched = build_host_re.search(xml)
             if matched:
                 if build_host != matched.groups()[0]:
@@ -157,18 +184,33 @@ def __get_distro_release(self, meta_doc):
 
     def get_version_and_distro_release(self, package):
         metadata_xml = os.path.join(self.package_path(package), ctx.const.metadata_xml)
-        meta_doc = iksemel.parse(metadata_xml)
+        try:
+            meta_doc = iksemel.parse(metadata_xml)
+        except Exception as e:
+            raise InstallDBError(
+                _("Failed to read metadata for package '%s': %s") % (package, e)
+            ) from e
         return self.__get_version(meta_doc) + self.__get_distro_release(meta_doc)
 
     def get_version(self, package):
         metadata_xml = os.path.join(self.package_path(package), ctx.const.metadata_xml)
-        meta_doc = iksemel.parse(metadata_xml)
+        try:
+            meta_doc = iksemel.parse(metadata_xml)
+        except Exception as e:
+            raise InstallDBError(
+                _("Failed to read metadata for package '%s': %s") % (package, e)
+            ) from e
         return self.__get_version(meta_doc)
 
     def get_files(self, package):
         files = pisi.files.Files()
         files_xml = os.path.join(self.package_path(package), ctx.const.files_xml)
-        files.read(files_xml)
+        try:
+            files.read(files_xml)
+        except Exception as e:
+            raise InstallDBError(
+                _("Failed to read file list for package '%s': %s") % (package, e)
+            ) from e
         return files
 
     def get_config_files(self, package):
@@ -271,7 +313,18 @@ def pkg_dir(self, pkg, version, release):
     def get_package(self, package):
         metadata = pisi.metadata.MetaData()
         metadata_xml = os.path.join(self.package_path(package), ctx.const.metadata_xml)
-        metadata.read(metadata_xml)
+        if not os.path.exists(metadata_xml):
+            raise InstallDBError(
+                _("Metadata file missing for package '%s'. "
+                  "Package may be corrupted. Try reinstalling it.") % package
+            )
+        try:
+            metadata.read(metadata_xml)
+        except Exception as e:
+            raise InstallDBError(
+                _("Corrupted metadata for package '%s': %s. "
+                  "Try reinstalling the package.") % (package, e)
+            ) from e
         return metadata.package
 
     def get_package_by_pkgconfig(self, pkgconfig):
@@ -342,10 +395,9 @@ def list_auto_installed(self):
 
     def __write_marked_packages(self, _type, packages):
         info_file = os.path.join(ctx.config.info_dir(), _type)
-        config = open(info_file, "w")
-        for pkg in set(packages):
-            config.write("%s\n" % pkg)
-        config.close()
+        with open(info_file, "w") as config:
+            for pkg in set(packages):
+                config.write("%s\n" % pkg)
 
     def __clear_marked_packages(self, _type, package):
         if package == "*":

pisi/db/lazydb.py

@@ -70,7 +70,7 @@ def cache_valid(self):
         try:
             f = self.__cache_version_file()
             ver = open(f).read().strip()
-        except IOError:
+        except OSError:
             return False
         return ver == LazyDB.cache_version
 
@@ -87,9 +87,12 @@ def cache_load(self):
                     open(self.__cache_file(), "rb"), encoding="utf-8"
                 )
                 return True
-            except (pickle.UnpicklingError, EOFError):
+            except (pickle.UnpicklingError, EOFError, AttributeError, ImportError, TypeError, Exception):
                 if os.access(ctx.config.cache_root_dir(), os.W_OK):
-                    os.unlink(self.__cache_file())
+                    try:
+                        os.unlink(self.__cache_file())
+                    except OSError:
+                        pass
                 return False
         return False
 

pisi/db/packagedb.py

@@ -309,7 +309,7 @@ def list_newest(self, repo, since=None):
                         0:6
                     ]
                 )
-            except:
+            except (ValueError, IndexError):
                 failed = True
             if failed:
                 try:
@@ -318,7 +318,7 @@ def list_newest(self, repo, since=None):
                             self.get_package(pkg).history[-1].date, "%Y-%m-%d"
                         )[0:6]
                     )
-                except:
+                except (ValueError, IndexError):
                     enter_date = datetime.datetime(
                         *time.strptime(
                             self.get_package(pkg).history[-1].date, "%Y-%d-%m"

pisi/db/repodb.py

@@ -113,9 +113,8 @@ def get_order(self):
 
     def _update(self, doc):
         repos_file_path = os.path.join(ctx.config.info_dir(), ctx.const.repos)
-        repo_file = open(repos_file_path, "w")
-        repo_file.write("%s\n" % doc.toPrettyString())
-        repo_file.close()
+        with open(repos_file_path, "w") as repo_file:
+            repo_file.write("%s\n" % doc.toPrettyString())
         self._doc = None
         self.repos = self._get_repos()
 
@@ -193,9 +192,10 @@ def get_repo_doc(self, repo_name):
         except Exception as e:
             raise RepoError(
                 _(
-                    "Error parsing repository index information. Index file does not exist or is malformed."
-                )
-            )
+                    "Error parsing repository index information for '%s'. "
+                    "Index file does not exist or is malformed."
+                ) % index_path
+            ) from e
 
     def get_repo(self, repo):
         return Repo(pisi.uri.URI(self.get_repo_url(repo)))
@@ -223,11 +223,14 @@ def add_repo(self, name, repo_info, at=None):
 
         try:
             os.makedirs(repo_path)
-        except Exception as e:
-            pass
+        except OSError as e:
+            import errno
+            if e.errno != errno.EEXIST:
+                ctx.ui.warning(_("Failed to create repository directory '%s': %s") % (repo_path, e))
 
         urifile_path = pisi.util.join_path(ctx.config.index_dir(), name, "uri")
-        open(urifile_path, "w").write(repo_info.indexuri.get_uri())
+        with open(urifile_path, "w") as urifile:
+            urifile.write(repo_info.indexuri.get_uri())
         self.repoorder.add(name, repo_info.indexuri.get_uri())
 
     def remove_repo(self, name):

pisi/fetcher.py

@@ -204,7 +204,7 @@ def fetch(self):
                             blocknum += 1
                             fetch_handler.update(blocknum, bs, size)
                     success = True
-            except URLError as e:
+            except (URLError, OSError) as e:
                 attempt += 1
                 if attempt == self._get_retry_attempts() + 1:
                     raise FetchError(
@@ -215,6 +215,9 @@ def fetch(self):
                     _('\nFailed to fetch file, retrying %d out of %d "%s": %s')
                     % (attempt, self._get_retry_attempts(), self.url.get_uri(), e)
                 )
+                ctx.ui.debug(
+                    _('Error type: %s') % type(e).__name__
+                )
                 pass
 
         if os.stat(self.partial_file).st_size == 0:
@@ -226,6 +229,7 @@ def fetch(self):
             )
 
         shutil.move(self.partial_file, self.archive_file)
+        ctx.ui.info(_('Downloaded: %s') % self.archive_file)
 
         return self.archive_file
 
@@ -316,4 +320,4 @@ def _test_range_support(self):
 def fetch_url(url, destdir, progress=None, destfile=None):
     fetch = Fetcher(url, destdir, destfile)
     fetch.progress = progress
-    fetch.fetch()
+    return fetch.fetch()

pisi/index.py

@@ -250,23 +250,21 @@ def add_groups(path):
 def add_components(path):
     ctx.ui.info(_("Adding components.xml to index"))
     components_xml = component.Components()
-    components_xml.read(path)
-    # try:
+    try:
+        components_xml.read(path)
+    except Exception as e:
+        raise Error(_("Component file '%s' is corrupt or unreadable: %s") % (path, e)) from e
     return components_xml.components
-    # except:
-    #    raise Error(_('Component in %s is corrupt') % path)
-    # ctx.ui.error(str(Error(*errs)))
 
 
 def add_distro(path):
-    ctx.ui.info("Adding distribution.xml to index")
+    ctx.ui.info(_("Adding distribution.xml to index"))
     distro = component.Distribution()
-    # try:
-    distro.read(path)
+    try:
+        distro.read(path)
+    except Exception as e:
+        raise Error(_("Distribution file '%s' is corrupt or unreadable: %s") % (path, e)) from e
     return distro
-    # except:
-    #    raise Error(_('Distribution in %s is corrupt') % path)
-    # ctx.ui.error(str(Error(*errs)))
 
 
 def add_spec(params):

pisi/operations/build.py

@@ -1780,7 +1780,7 @@ def build(pspec):
         pb.build()
     except ActionScriptException as e:
         ctx.ui.error(_("Action script error caught."))
-        raise e
+        raise
     finally:
         if ctx.ui.errors or ctx.ui.warnings:
             ctx.ui.warning(

pisi/operations/install.py

@@ -159,8 +159,8 @@ def install_pkg_names(packages, reinstall=False):
             if install_op.pkginfo.name in automatic:
                 install_op.automatic = True
             install_op.install(False)
-    except Exception as e:
-        raise e
+    except Exception:
+        raise
     finally:
         ctx.exec_usysconf()
 
@@ -335,9 +335,8 @@ def get_package(self, key, repo=None):
     try:
         for x in order:
             atomicoperations.install_single_file(dfn[x], reinstall)
-    except Exception as e:
-        raise e
-        return False
+    except Exception:
+        raise
     finally:
         ctx.exec_usysconf()
 

pisi/operations/remove.py

@@ -131,9 +131,9 @@ def remove(
             if installdb.has_package(package):
                 atomicoperations.remove_single(package)
             else:
-                ctx.ui.info(_("Package %s is not installed. Cannot remove.") % package)
-    except Exception as e:
-        raise e
+                ctx.ui.info(_('Package %s is not installed. Cannot remove.') % package)
+    except Exception:
+        raise
     finally:
         ctx.exec_usysconf()
 

pisi/operations/upgrade.py

@@ -248,8 +248,8 @@ def upgrade(packages = [], repo = None):
             if install_op.pkginfo.name in automatic:
                 install_op.automatic = True
             install_op.install(True)
-    except Exception as e:
-        raise e
+    except Exception:
+        raise
     finally:
         ctx.exec_usysconf()
 

pisi/pxml/xmlfile.py

@@ -48,7 +48,7 @@ def parsexml(self, xml):
             self.doc = iks.parseString(xml.decode() if type(xml) == bytes else xml)
             return self.doc
         except Exception as e:
-            raise Error(_("String '%s' has invalid XML") % (xml))
+            raise Error(_('String \'%s\' has invalid XML') % (xml)) from e
 
     def readxml(
         self,