Merge "Unified error handling"

software/software/api/app.py

@@ -1,5 +1,5 @@
 """
-Copyright (c) 2023 Wind River Systems, Inc.
+Copyright (c) 2023-2024 Wind River Systems, Inc.
 
 SPDX-License-Identifier: Apache-2.0
 
@@ -8,6 +8,7 @@ SPDX-License-Identifier: Apache-2.0
 import pecan
 
 from software.config import CONF
+from software.utils import ExceptionHook
 
 
 def get_pecan_config():
@@ -39,15 +40,14 @@ def setup_app(pecan_config=None):
         pecan_config = get_pecan_config()
     pecan.configuration.set_config(dict(pecan_config), overwrite=True)
 
-    # todo(abailey): Add in the hooks
-    hooks = []
+    hook_list = [ExceptionHook()]
 
     # todo(abailey): It seems like the call to pecan.configuration above
     # mean that the following lines are redundnant?
     app = pecan.make_app(
         pecan_config.app.root,
         debug=pecan_config.app.debug,
-        hooks=hooks,
+        hooks=hook_list,
         force_canonical=pecan_config.app.force_canonical,
         guess_content_type_from_ext=pecan_config.app.guess_content_type_from_ext
     )

software/software/authapi/app.py

@@ -1,5 +1,5 @@
 """
-Copyright (c) 2023 Wind River Systems, Inc.
+Copyright (c) 2023-2024 Wind River Systems, Inc.
 
 SPDX-License-Identifier: Apache-2.0
 
@@ -12,6 +12,7 @@ from software.authapi import acl
 from software.authapi import config
 from software.authapi import hooks
 from software.authapi import policy
+from software.utils import ExceptionHook
 
 auth_opts = [
     cfg.StrOpt('auth_strategy',
@@ -34,6 +35,7 @@ def setup_app(pecan_config=None, extra_hooks=None):
 
     app_hooks = [hooks.ConfigHook(),
                  hooks.ContextHook(pecan_config.app.acl_public_routes),
+                 ExceptionHook(),
                  ]
     if extra_hooks:
         app_hooks.extend(extra_hooks)

software/software/exceptions.py

@@ -1,5 +1,5 @@
 """
-Copyright (c) 2023 Wind River Systems, Inc.
+Copyright (c) 2023-2024 Wind River Systems, Inc.
 
 SPDX-License-Identifier: Apache-2.0
 
@@ -115,3 +115,28 @@ class DeployAlreadyExist(SoftwareError):
 class ReleaseVersionDoNotExist(SoftwareError):
     """Release Version Do Not Exist"""
     pass
+
+
+class SoftwareServiceError(Exception):
+    """
+    This is a service error, such as file system issue or configuration
+    issue, which is expected at design time for a valid reason.
+    This exception type will provide detail information to the user.
+    see ExceptionHook for detail
+    """
+    def __init__(self, info="", warn="", error=""):
+        self._info = info
+        self._warn = warn
+        self._error = error
+
+    @property
+    def info(self):
+        return self._info if self._info is not None else ""
+
+    @property
+    def warning(self):
+        return self._warn if self._warn is not None else ""
+
+    @property
+    def error(self):
+        return self._error if self._error is not None else ""

software/software/software_controller.py

@@ -2698,52 +2698,6 @@ class PatchController(PatchService):
         return deploy_host_list
 
 
-# The wsgiref.simple_server module has an error handler that catches
-# and prints any exceptions that occur during the API handling to stderr.
-# This means the patching sys.excepthook handler that logs uncaught
-# exceptions is never called, and those exceptions are lost.
-#
-# To get around this, we're subclassing the simple_server.ServerHandler
-# in order to replace the handle_error method with a custom one that
-# logs the exception instead, and will set a global flag to shutdown
-# the server and reset.
-#
-class MyServerHandler(simple_server.ServerHandler):
-    def handle_error(self):
-        LOG.exception('An uncaught exception has occurred:')
-        if not self.headers_sent:
-            self.result = self.error_output(self.environ, self.start_response)
-            self.finish_response()
-        global keep_running
-        keep_running = False
-
-
-def get_handler_cls():
-    cls = simple_server.WSGIRequestHandler
-
-    # old-style class doesn't support super
-    class MyHandler(cls, object):
-        def address_string(self):
-            # In the future, we could provide a config option to allow reverse DNS lookup
-            return self.client_address[0]
-
-        # Overload the handle function to use our own MyServerHandler
-        def handle(self):
-            """Handle a single HTTP request"""
-
-            self.raw_requestline = self.rfile.readline()
-            if not self.parse_request():  # An error code has been sent, just exit
-                return
-
-            handler = MyServerHandler(
-                self.rfile, self.wfile, self.get_stderr(), self.get_environ()
-            )
-            handler.request_handler = self  # pylint: disable=attribute-defined-outside-init
-            handler.run(self.server.get_app())
-
-    return MyHandler
-
-
 class PatchControllerApiThread(threading.Thread):
     def __init__(self):
         threading.Thread.__init__(self)
@@ -2767,8 +2721,7 @@ class PatchControllerApiThread(threading.Thread):
             self.wsgi = simple_server.make_server(
                 host, port,
                 app.VersionSelectorApplication(),
-                server_class=server_class,
-                handler_class=get_handler_cls())
+                server_class=server_class)
 
             self.wsgi.socket.settimeout(api_socket_timeout)
             global keep_running
@@ -2821,8 +2774,7 @@ class PatchControllerAuthApiThread(threading.Thread):
             self.wsgi = simple_server.make_server(
                 host, port,
                 auth_app.VersionSelectorApplication(),
-                server_class=server_class,
-                handler_class=get_handler_cls())
+                server_class=server_class)
 
             # self.wsgi.serve_forever()
             self.wsgi.socket.settimeout(api_socket_timeout)

software/software/utils.py

@@ -1,29 +1,61 @@
 """
-Copyright (c) 2023 Wind River Systems, Inc.
+Copyright (c) 2023-2024 Wind River Systems, Inc.
 
 SPDX-License-Identifier: Apache-2.0
 
 """
+import hashlib
+from pecan import hooks
 import json
 import logging
-import re
-import shutil
 from netaddr import IPAddress
 import os
 from oslo_config import cfg as oslo_cfg
 from packaging import version
+import re
+import shutil
 import socket
 from socket import if_nametoindex as if_nametoindex_func
+import traceback
+import webob
 
 import software.constants as constants
-
 from software.exceptions import StateValidationFailure
+from software.exceptions import SoftwareServiceError
 
 
 LOG = logging.getLogger('main_logger')
 CONF = oslo_cfg.CONF
 
 
+class ExceptionHook(hooks.PecanHook):
+    def _get_stacktrace_signature(self, trace):
+        trace = re.sub(', line \\d+', '', trace)
+        # only taking 4 bytes from the hash to identify different error paths
+        signature = hashlib.shake_128(trace.encode('utf-8')).hexdigest(4)
+        return signature
+
+    def on_error(self, state, e):
+        trace = traceback.format_exc()
+        signature = self._get_stacktrace_signature(trace)
+        status = 500
+
+        if isinstance(e, SoftwareServiceError):
+            LOG.warning("An issue is detected. Signature [%s]" % signature)
+            LOG.exception(e)
+
+            data = dict(info=e.info, warning=e.warning, error=e.error)
+            data['error'] = data['error'] + " Error signature [%s]" % signature
+        else:
+            err_msg = "Internal error occurred. Error signature [%s]" % signature
+            LOG.error(err_msg)
+            LOG.exception(e)
+            # Unexpected exceptions, exception message is not sent to the user.
+            # Instead state as internal error
+            data = dict(info="", warning="", error=err_msg)
+        return webob.Response(json.dumps(data), status=status)
+
+
 def if_nametoindex(name):
     try:
         return if_nametoindex_func(name)