From 7d9bfd28be7cc9f3aab9e051426b96e63ff05300 Mon Sep 17 00:00:00 2001
From: Chris Hines <chris.hines@monash.edu>
Date: Fri, 26 Feb 2021 14:17:11 +1100
Subject: [PATCH] cleanup some code around detecting errors

---
 tes/apiendpoints.py | 128 +++++++++++++++++++-------------------------
 1 file changed, 56 insertions(+), 72 deletions(-)

diff --git a/tes/apiendpoints.py b/tes/apiendpoints.py
index 94663cc..6878d2f 100644
--- a/tes/apiendpoints.py
+++ b/tes/apiendpoints.py
@@ -187,6 +187,54 @@ class ContactUs(Resource):
         f.close()
         return
 
+def wrap_execute(sshsess, host, bastion=None, user=None, cmd=None):
+    """
+    This function is supposed to interpret all the possible exceptions from Ssh.execute and generate
+    approrparite HTTP errors (both status codes and messages)
+
+    There are a range of situations:
+    it might raise an SshExecException (non-zero return code)
+    it might return valid data or not
+    it might return stderr or not
+    the data on stderr might be valid json or not.
+    """
+    try:
+        if bastion is None:
+            res = Ssh.execute(sshsess, host=host, user=user, cmd=cmd)
+        else:
+            res = Ssh.execute(sshsess, host=host, bastion=bastion, user=user, cmd=cmd)
+        if not (res['stderr'] == '' or res['stderr'] is None or res['stderr'] == b''):
+            logger.error(res['stderr'])
+            #flask_restful.abort(400, message=res['stderr'].decode())
+            return apiabort(400, message=res['stderr'].decode())
+
+        try:
+            data = json.loads(res['stdout'].decode())
+            return data
+        except json.decoder.JSONDecodeError:
+            return apiabort(500, message="{} failed to execute on {}".format(cmd,host))
+        except Exception as e:
+            import traceback
+            logger.error(e)
+            logger.error(traceback.format_exc())
+            return apiabort(400, message=e)
+
+    except SshCtrlException as e:
+        if ("{}".format(e) != ''):
+            return apiabort(400,message="We're having difficultly contacting {}. We failed with the message: {}".format(host,e))
+        else:
+            # This is the most common error, if the user disconnected from the network and the api server cleaned up all the agents and control sockets
+            return apiabort(401,message="It looks like your login to {} timed out. Plese log in again".format(host))
+    # This exception is raised if the remove command had a non-zero return code
+    except SshExecException as e:
+        return apiabort(400,message="{}".format(e))
+    # Any other exceptions. This should never be reached.
+    except Exception as e:
+        import traceback
+        logger.error('JobStat.get: Exception {}'.format(e))
+        logger.error(traceback.format_exc())
+        return apiabort(500,message="SSH failed in an unexpected way")
+
 class JobStat(Resource):
     """
     endpoints to return info on jobs on the backend compute Resource
@@ -212,43 +260,7 @@ class JobStat(Resource):
             import traceback
             return apiabort(400, message="Missing required parameter {}\n{}".format(e,traceback.format_exc()))
 
-        try:
-            res = Ssh.execute(sshsess, host=host, user=user, cmd=cmd)
-            if not (res['stderr'] == '' or res['stderr'] is None or res['stderr'] == b''):
-                logger.error(res['stderr'])
-                #flask_restful.abort(400, message=res['stderr'].decode())
-                return apiabort(400, message=res['stderr'].decode())
-            try:
-                jobs = json.loads(res['stdout'].decode())
-                return jobs
-            except json.decoder.JSONDecodeError:
-                return apiabort(500, message="{} failed to execute on {}".format(cmd,host))
-
-            except Exception as e:
-                import traceback
-                logger.error(e)
-                logger.error(traceback.format_exc())
-                #flask_restful.abort(400, message=e)
-                return apiabort(400, message=e)
-        except SshAgentException as e:
-            logger.error(e)
-            #flask_restful.abort(401, message="Identity error {}".format(e))
-            return apiabort(401, message="Identity error {}".format(e))
-        except SshCtrlException as e:
-            #flask_restful.abort(400,message="We're having difficultly contacting {}. We failed with the message: {}".format(host,e))
-            if ("{}".format(e) is not ''):
-                return apiabort(400,message="We're having difficultly contacting {}. We failed with the message: {}".format(host,e))
-            else:
-                return apiabort(401,message="It looks like your login to {} timed out. Plese log in again".format(host))
-            #return apiabort(400,message="We're having difficultly contacting {}. We failed with the message: {}".format(host,e))
-        except SshExecException as e:
-            return apiabort(400,message="{}".format(e))
-        except Exception as e:
-            import traceback
-            logger.error('JobStat.get: Exception {}'.format(e))
-            logger.error(traceback.format_exc())
-            #flask_restful.abort(500,message="SSH failed in an unexpected way")
-            return apiabort(500,message="SSH failed in an unexpected way")
+        rv = wrap_execute(sshsess, host=host, user=user, cmd=cmd)
 
 class MkDir(Resource):
     def post(self):
@@ -488,48 +500,20 @@ class AppInstance(Resource):
         command is passed as a query string"""
         import logging
         logger=logging.getLogger()
+        sshsess = SSHSession.get_sshsession()
         try:
-            sshsess = SSHSession.get_sshsession()
             paramscmd = json.loads(request.args.get('cmd')).format(jobid=jobid)
-            import logging
-            logger = logging.getLogger()
-            #cmd = 'ssh -o StrictHostKeyChecking=no -o CheckHostIP=no {batchhost} '.format(batchhost=batchhost) + paramscmd
-            try:
-                res = Ssh.execute(sshsess, host=batchhost, bastion=loginhost, user=username, cmd=paramscmd)
-            except:
-                message = "The server couldn't execute to {} to get the necessary info".format(batchhost)
-                #flask_restful.abort(400, message=message)
-                import traceback
-                logger.error(traceback.format_exc())
-                return apiabort(400, message=message)
-            try:
-                data = json.loads(res['stdout'].decode())
-                if 'error' in data:
-                    return data, 400
-                return data
-            except json.decoder.JSONDecodeError as e:
-                logger.error(res['stderr']+res['stdout'])
-                message="I'm having trouble using ssh to find out about that application"
-                #flask_restful.abort(400, message=message)
-                return apiabort(400, message=message)
-                #raise AppParamsException(res['stderr']+res['stdout'])
-            if len(res['stderr']) > 0:
-                logger.error(res['stderr']+res['stdout'])
-                #flask_restful.abort(400, message="The command {} on {} didn't work".format(paramscmd,batchhost))
-                return apiabort(400, message="The command {} on {} didn't work".format(paramscmd,batchhost))
-                #raise AppParamsException(res['stderr'])
-            if 'error' in data:
-                raise AppParamsException(data['error'])
-            if not (res['stderr'] == '' or res['stderr'] is None or res['stderr'] == b''):
-                #flask_restful.abort(400, message=res['stderr'].decode())
-                return apiabort(400, message=res['stderr'].decode())
-            return data
+        except json.decoder.JSONDecodeError:
+            return apiabort(400, message="No command was sent to the API server")
+        try:
+            rv = wrap_execute(sshsess, host=batchhost, bastion=loginhost, user=username, cmd=paramscmd)
+            return rv
         except Exception as e:
             import traceback
             logger.error(e)
             logger.error(traceback.format_exc())
             #flask_restful.abort(500,message="AppUrl failed in some unexpected way")
-            return apiabort(500,message="AppUrl failed in some unexpected way")
+            return apiabort(500,message="AppInstance failed in some unexpected way")
 
 class CreateTunnel(Resource):
     @staticmethod
-- 
GitLab