src: centralize error logging into send_internal_server_error

Use only the exception messages as the main resource for error
messages.
The previous error code had string duplication in the form of:
	logging.error('msg here')
	raise Exception('msg here')

That approach also has the downside of having log duplication as
it had the local logging.err() and a global logging.exception()
inside send_internal_server_error capturing the exception message.
The actual code only requires raising an exception with a proper
error message.
Improve exception messages to give more error context.
Log every AssertionError as a backtrace.
Use the 'raise Exception from e' syntax to modify the a previously
raised exception 'e' into an exception with aditional context or
different type. This also prevents the message that warns about
newer exceptions being launch after an initial exception.

1 files changed, 21 insertions, 38 deletions

diff --git a/src/live/ogOperations.py b/src/live/ogOperations.py
index acb1a0e..d564ea9 100644
--- a/src/live/ogOperations.py
+++ b/src/live/ogOperations.py
@@ -51,9 +51,8 @@ class OgLiveOperations:
         try:
             proc = subprocess.call(["pkill", "-9", "browser"])
             proc = subprocess.Popen(["browser", "-qws", url])
-        except:
-            logging.exception('Cannot restart browser')
-            raise ValueError('Error: cannot restart browser')
+        except Exception as e:
+            raise RuntimeError('Cannot restart browser') from e
 
     def _refresh_payload_disk(self, cxt, part_setup, num_disk):
         part_setup['disk'] = str(num_disk)
@@ -120,8 +119,7 @@ class OgLiveOperations:
 
     def _write_md5_file(self, path):
         if not os.path.exists(path):
-            logging.error('Invalid path in _write_md5_file')
-            raise ValueError('Invalid image path when computing md5 checksum')
+            raise ValueError(f'Invalid image path {path} when computing md5 checksum')
         filename = path + ".full.sum"
         dig = self._compute_md5(path)
         try:
@@ -145,15 +143,13 @@ class OgLiveOperations:
         try:
             r = shutil.copy(src, dst)
             tip_write_csum(image_name)
-        except:
-            logging.exception('Error copying image to cache')
-            raise ValueError(f'Error: Cannot copy image {image_name} to cache')
+        except Exception as e:
+            raise RuntimeError(f'Error copying image {image_name} to cache. Reported: {e}') from e
 
     def _restore_image_unicast(self, repo, name, devpath, cache=False):
         if ogChangeRepo(repo, smb_user=self._smb_user, smb_pass=self._smb_pass) != 0:
             self._restartBrowser(self._url)
-            logging.error('ogChangeRepo could not change repository to %s', repo)
-            raise ValueError(f'Error: Cannot change repository to {repo}')
+            raise ValueError(f'Cannot change repository to {repo}')
         logging.debug(f'restore_image_unicast: name => {name}')
         if cache:
             image_path = f'/opt/opengnsys/cache/opt/opengnsys/images/{name}.img'
@@ -191,8 +187,7 @@ class OgLiveOperations:
         cmd_pc = shlex.split(f'partclone.restore -d0 -C -I -o {devpath}')
 
         if not os.path.exists(image_path):
-            logging.error('f{image_path} does not exist, exiting.')
-            raise ValueError(f'Error: Image not found at {image_path}')
+            raise RuntimeError(f'Image not found at {image_path} during image restore')
 
         with open('/tmp/command.log', 'wb', 0) as logfile:
             proc_lzop = subprocess.Popen(cmd_lzop,
@@ -248,9 +243,8 @@ class OgLiveOperations:
                                shell=True,
                                executable=OG_SHELL)
             (output, error) = ogRest.proc.communicate()
-        except:
-            logging.exception('Exception when running "shell run" subprocess')
-            raise ValueError('Error: Incorrect command value')
+        except Exception as e:
+            raise RuntimeError(f'Error when running "shell run" subprocess: {e}') from e
 
         if ogRest.proc.returncode != 0:
             logging.warn('Non zero exit code when running: %s', ' '.join(cmds))
@@ -298,9 +292,8 @@ class OgLiveOperations:
 
         try:
             inventory = get_hardware_inventory()
-        except ValueError as e:
-            logging.exception('Error occurred while running get_hardware_inventory')
-            raise e
+        except Exception as e:
+            raise RuntimeError(f'Error while running hardware inventory. {e}') from e
         finally:
             self._restartBrowser(self._url)
 
@@ -327,7 +320,6 @@ class OgLiveOperations:
         elif table_type == 'GPT':
             cxt.create_disklabel('gpt')
         else:
-            logging.info(f'Unsupported partition layout: {table_type}, only MSDOS and GPT are supported')
             raise ValueError(f'Unsupported partition scheme {table_type}, only MSDOS and GPT are supported')
 
         logging.info(f'Setting up partition layout to {table_type}')
@@ -422,8 +414,7 @@ class OgLiveOperations:
 
         if ogChangeRepo(repo, smb_user=self._smb_user, smb_pass=self._smb_pass) != 0:
             self._restartBrowser(self._url)
-            logging.error('ogChangeRepo could not change repository to %s', repo)
-            raise ValueError(f'Error: Cannot change repository to {repo}')
+            raise RuntimeError(f'Cannot change image repository to {repo}')
 
         if ogRest.terminated:
             return
@@ -439,30 +430,25 @@ class OgLiveOperations:
 
             if pa is None:
                 self._restartBrowser(self._url)
-                logging.error(f'Target partition /dev/{diskname} not found')
-                raise ValueError(f'Target partition /dev/{diskname} not found')
+                raise RuntimeError(f'Target partition /dev/{diskname} not found')
 
             padev = cxt.partition_to_string(pa, fdisk.FDISK_FIELD_DEVICE)
             fstype = cxt.partition_to_string(pa, fdisk.FDISK_FIELD_FSTYPE)
             if not fstype:
-                logging.error(f'No filesystem detected in {padev}. Aborting image creation.')
-                raise ValueError('Target partition has no filesystem present')
+                raise RuntimeError(f'No filesystem detected in {padev}. Aborting image creation')
 
             if change_access(user=self._smb_user, pwd=self._smb_pass) == -1:
-                logging.error('remount of /opt/opengnsys/images has failed')
-                raise AssertionError('remount of /opt/opengnsys/images has failed')
+                raise RuntimeError('remount of /opt/opengnsys/images has failed')
 
             if os.access(f'/opt/opengnsys/images', os.R_OK | os.W_OK) == False:
-                logging.error('Cannot access /opt/opengnsys/images in read and write mode, check permissions')
-                raise ValueError('Cannot access image folder')
+                raise RuntimeError('Cannot access /opt/opengnsys/images in read and write mode, check permissions')
 
             if os.access(f'{image_path}', os.R_OK) == True:
                 logging.info(f'image file {image_path} already exists, updating.')
 
             ogCopyEfiBootLoader(disk, partition)
             if ogReduceFs(disk, partition) == -1:
-                logging.error(f'Failed to shrink {fstype} filesystem in {padev}')
-                raise ValueError('Failed to shrink filesystem')
+                raise ValueError(f'Failed to shrink {fstype} filesystem in {padev}')
 
             cmd1 = shlex.split(f'partclone.{fstype} -I -C --clone -s {padev} -O -')
             cmd2 = shlex.split(f'lzop -1 -fo {image_path}')
@@ -482,7 +468,7 @@ class OgLiveOperations:
             try:
                 retdata = p2.communicate()
             except OSError as e:
-                logging.exception('Unexpected error when running partclone and lzop commands')
+                raise OSError(f'Unexpected error when running partclone and lzop commands: {e}') from e
             finally:
                 logfile.close()
                 p2.terminate()
@@ -494,14 +480,12 @@ class OgLiveOperations:
             ogExtendFs(disk, partition)
 
             if os.access(f'{image_path}', os.R_OK) == False:
-                logging.error(f'Cannot access partclone image file {image_path}')
-                raise ValueError('Cannot access image file')
+                raise RuntimeError(f'Cannot access partclone image file {image_path}')
 
             image_info = ogGetImageInfo(image_path)
         except:
             self._restartBrowser(self._url)
-            logging.exception('Exception when running "image create" subprocess')
-            raise ValueError(f'Error: Cannot create image for {fstype} filesystem in device {padev}')
+            raise RuntimeError(f'Error: Cannot create image for {fstype} filesystem in device {padev}')
 
         try:
             st = os.stat(image_path)
@@ -519,8 +503,7 @@ class OgLiveOperations:
         image_info.mtime = mtime
 
         if self._write_md5_file(f'/opt/opengnsys/images/{name}.img') == -1:
-            logging.error(f'cannot write {name}.full.sum file')
-            raise ValueError(f'Error: Cannot write {name}.full.sum file')
+            raise ValueError(f'Cannot write {name}.full.sum file')
 
         self._restartBrowser(self._url)