Merge ~hyask/autopkgtest-cloud:skia/browse_exception_handler into autopkgtest-cloud:master

[Skia]

[Tim Andersson (andersson123)]

LGTM! But failed CI, fix that and I'll approve :)

[Tim Andersson (andersson123)]

Sorry, just realised I do have a qualm with this:

Please amend the commit message to accurately reflect the changes. This commit doesn't just add a generic exception handler ;)

[Tim Andersson (andersson123)]

review: Needs Fixing

[Skia (hyask)]

Rebased and split into two different proper commits.

[Tim Andersson (andersson123)]

The first commit message doesn't mention the new HTML at all, and it's not clear the exception handler is user facing in this instance.

The second commit message needs a bit of work too IMO, "properly" isn't a good word to use as it's undefined. A bit more detail please.

[Skia (hyask)]

I'm not sure if spending that much time on commit messages was really worth it, but here you go :-)

[Tim Andersson (andersson123)]

Of course it was worth it. We need good, detailed & descriptive high quality commit messages for our git archaeology. We are the QA team after all.

Anyway. LGTM now.

review: Approve

[Tim Andersson (andersson123)]

I also just want to add that I'm not being nitpicky or pedantic with wanting more detailed commit messages. It's a standard I've been held to in my MP's so I'm just passing it on.

[Skia (hyask)]

Sure, np :-)

That reminds me that Brian wanted to find a warcry for the QA team. That would have been appropriate in that situation XD

QA MF!

[Brian Murray (brian-murray)]

I think I was really looking for a team haka.

diff --git a/charms/focal/autopkgtest-web/webcontrol/browse.cgi b/charms/focal/autopkgtest-web/webcontrol/browse.cgi
index 2918276..f71b406 100755
--- a/charms/focal/autopkgtest-web/webcontrol/browse.cgi
+++ b/charms/focal/autopkgtest-web/webcontrol/browse.cgi
@@ -12,6 +12,7 @@ from wsgiref.handlers import CGIHandler
 
 import flask
 from helpers.admin import select_abnormally_long_jobs
+from helpers.exceptions import RunningJSONNotFound
 from helpers.utils import get_all_releases, get_supported_releases
 from werkzeug.middleware.proxy_fix import ProxyFix
 
@@ -62,10 +63,9 @@ def get_running_jobs():
     try:
         with open(RUNNING_CACHE) as f:
             # package -> runhash -> release -> arch -> (params, duration, logtail)
-            running_info = json.load(f)
-    except FileNotFoundError:
-        running_info = {}
-    return running_info
+            return json.load(f)
+    except FileNotFoundError as e:
+        raise RunningJSONNotFound(e) from e
 
 
 def render(template, code=200, **kwargs):
@@ -90,7 +90,7 @@ def render(template, code=200, **kwargs):
             template,
             base_url=flask.url_for("index_root"),
             static_url=flask.url_for("static", filename=""),
-            **kwargs
+            **kwargs,
         ),
         code,
     )
@@ -531,6 +531,25 @@ def statistics():
     )
 
 
+def invalid(inv_exception, code=400):
+    """Return message and HTTP error code for an invalid request"""
+    return render("browse-error.html", error=inv_exception, code=code)
+
+
[e-mail](Exception)
+def all_exception_handler(error):
+    # If the exception doesn't have the exit_code method, it's not an expected
+    # exception defined in helpers/exceptions.py
+    try:
+        return invalid(error, error.exit_code())
+    except Exception as e:
+        return render(
+            "browse-error.html",
+            error=f"Error {e} during handling of {error}",
+            code=500,
+        )
+
+
 if __name__ == "__main__":
     app.config["DEBUG"] = True
     init_config()
diff --git a/charms/focal/autopkgtest-web/webcontrol/helpers/exceptions.py b/charms/focal/autopkgtest-web/webcontrol/helpers/exceptions.py
index becb8c7..e94164e 100644
--- a/charms/focal/autopkgtest-web/webcontrol/helpers/exceptions.py
+++ b/charms/focal/autopkgtest-web/webcontrol/helpers/exceptions.py
@@ -10,6 +10,11 @@ EXAMPLE_URL = (
 )
 
 
+class RunningJSONNotFound(FileNotFoundError):
+    def exit_code(self):
+        return 500
+
+
 class WebControlException(Exception):
     def __init__(self, message, exit_code):
         super().__init__(message)
diff --git a/charms/focal/autopkgtest-web/webcontrol/templates/browse-error.html b/charms/focal/autopkgtest-web/webcontrol/templates/browse-error.html
index af23654..12b6cf7 100644
--- a/charms/focal/autopkgtest-web/webcontrol/templates/browse-error.html
+++ b/charms/focal/autopkgtest-web/webcontrol/templates/browse-error.html
@@ -1,5 +1,12 @@
 {% extends "browse-layout.html" %}
 {% block content %}
-<h1>Error:</h1>
-<p>{{error}}</p>
+<div style="background: #f33; padding: 0.5em 1.5em;">
+  <h1>Error:</h1>
+  <p>{{ error }}</p>
+</div>
+<p>
+A server error has occured, please contact a member of the Ubuntu QA team. You
+can contact them via the ubuntu-quality mailing list, or via the #ubuntu-quality
+IRC channel on irc.libera.chat (highlight 'qa-help' for more reactivity).
+</p>
 {% endblock %}