Skip to content

Commit

Permalink
Make error handling more robust (#51)
Browse files Browse the repository at this point in the history
* Display generic HTTP exceptions as expandable alerts

* Simplify handling of non-HTTP errors

* Fix non-error target disappearing on error response

* Declare generic hx-target-error globally

* Add max width and height for error alerts

Both collapsed and expanded

* Fix missing error-key in session

---------

Co-authored-by: Hanne Moa <[email protected]>
  • Loading branch information
podliashanyk and hmpf authored Jan 24, 2024
1 parent 455f9b3 commit 71919ad
Show file tree
Hide file tree
Showing 13 changed files with 41 additions and 53 deletions.
19 changes: 1 addition & 18 deletions src/howitz/endpoints.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import os
import uuid

from flask import (
Blueprint,
Expand All @@ -24,7 +23,7 @@
from zinolib.ritz import NotConnectedError

from howitz.users.utils import authenticate_user
from .utils import login_check, serialize_exception
from .utils import login_check

main = Blueprint('main', __name__)

Expand Down Expand Up @@ -88,7 +87,6 @@ def get_current_events():
if session["not_connected_counter"] > 1: # This error is not intermittent - increase counter and handle
current_app.logger.exception('Recurrent NotConnectedError %s', notConnErr)
session["not_connected_counter"] += 1
show_error_popup(notConnErr, 'An error occurred, please check your connection status to Zino')
raise
else: # This error is intermittent - increase counter and retry
current_app.logger.exception('Intermittent NotConnectedError %s', notConnErr)
Expand Down Expand Up @@ -120,7 +118,6 @@ def poll_current_events():
if session["not_connected_counter"] > 1: # This error is not intermittent - increase counter and handle
current_app.logger.exception('Recurrent NotConnectedError %s', notConnErr)
session["not_connected_counter"] += 1
show_error_popup(notConnErr, 'An error occurred, please check your connection status to Zino')
raise
else: # This error is intermittent - increase counter and retry
current_app.logger.exception('Intermittent NotConnectedError %s', notConnErr)
Expand Down Expand Up @@ -209,7 +206,6 @@ def get_event_attributes(id, res_format=dict):
event = current_app.event_manager.create_event_from_id(int(id))
except RetryError as retryErr: # Intermittent error in Zino
current_app.logger.exception('RetryError when fetching event attributes %s', retryErr)
show_error_popup(retryErr, 'Could not fetch event attributes, please retry')
raise

event_dict = vars(event)
Expand All @@ -227,7 +223,6 @@ def get_event_details(id):
event_attr = vars(current_app.event_manager.create_event_from_id(int(id)))
except RetryError as retryErr: # Intermittent error in Zino
current_app.logger.exception('RetryError when fetching event details %s', retryErr)
show_error_popup(retryErr, 'Could not fetch event details, please retry')
raise

event_logs = current_app.event_manager.get_log_for_id(int(id))
Expand All @@ -238,16 +233,6 @@ def get_event_details(id):
return event_attr, event_logs, event_history, event_msgs


def show_error_popup(error, short_description):
alert_random_id = str(uuid.uuid4())

session["errors"][alert_random_id] = serialize_exception(error)
session.modified = True

return render_template('/components/popups/alerts/error/error-alert.html',
alert_id=alert_random_id, short_err_msg=short_description)


@main.route('/')
@main.route('/events')
@login_check()
Expand Down Expand Up @@ -342,7 +327,6 @@ def expand_event_row(event_id):
eventobj = current_app.event_manager.create_event_from_id(event_id)
except RetryError as retryErr: # Intermittent error in Zino
current_app.logger.exception('RetryError on row expand after retry, %s', retryErr)
show_error_popup(retryErr, 'Could not expand event, please retry')
raise
event = create_table_event(eventobj)

Expand Down Expand Up @@ -372,7 +356,6 @@ def collapse_event_row(event_id):
eventobj = current_app.event_manager.create_event_from_id(event_id)
except RetryError as retryErr: # Intermittent error in Zino
current_app.logger.exception('RetryError on row collapse %s', retryErr)
show_error_popup(retryErr, 'Could not collapse event, please retry')
raise
event = create_table_event(eventobj)

Expand Down
42 changes: 26 additions & 16 deletions src/howitz/error_handlers.py
Original file line number Diff line number Diff line change
@@ -1,26 +1,29 @@
import uuid

from flask import json, render_template, session, current_app
from flask import render_template, session, current_app, make_response
from werkzeug.exceptions import HTTPException

from howitz.utils import serialize_exception


# Fixme add non-generic error handling as well
def handle_generic_http_exception(e):
current_app.logger.exception("An unexpected HTTP exception has occurred %s", e)

"""Return JSON instead of HTML for HTTP errors."""
# start with the correct headers and status code from the error
response = e.get_response()
# replace the body with JSON
response.data = json.dumps({
"code": e.code,
"name": e.name,
"description": e.description,
})
response.content_type = "application/json"
return response
alert_random_id = str(uuid.uuid4())
short_err_msg = f"{e.code} {e.name}: {e.description}"

if not "errors" in session:
session["errors"] = dict()
session["errors"][str(alert_random_id)] = serialize_exception(e)
session.modified = True
current_app.logger.debug('ERRORS %s', session["errors"])

response = make_response(render_template('/components/popups/alerts/error/error-alert.html',
alert_id=alert_random_id, short_err_msg=short_err_msg))

response.headers['HX-Reswap'] = 'beforeend'

return response, e.code


def handle_generic_exception(e):
Expand All @@ -30,7 +33,10 @@ def handle_generic_exception(e):

# now you're handling non-HTTP exceptions only
alert_random_id = str(uuid.uuid4())
short_err_msg = 'An unexpected error has occurred'
try:
short_err_msg = e.args[0]
except IndexError:
short_err_msg = 'An unexpected error has occurred'

if not "errors" in session:
session["errors"] = dict()
Expand All @@ -39,8 +45,12 @@ def handle_generic_exception(e):
current_app.logger.debug('ERRORS %s', session["errors"])
current_app.logger.exception("An unexpected exception has occurred %s", e)

return render_template('/components/popups/alerts/error/error-alert.html',
alert_id=alert_random_id, short_err_msg=short_err_msg)
response = make_response(render_template('/components/popups/alerts/error/error-alert.html',
alert_id=alert_random_id, short_err_msg=short_err_msg))

response.headers['HX-Reswap'] = 'beforeend'

return response, 500


def handle_400(e):
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
<div id="error-alert-{{ alert_id }}"
class="fixed bottom-6 right-6 z-50 p-1 text-white bg-orange-800 rounded-lg shadow"
class="fixed bottom-0 right-0 z-50 p-1 m-6 text-white bg-orange-800 rounded-lg shadow max-h-screen max-w-screen overflow-hidden flex flex-col gap-1 justify-start flex-nowrap items-stretch"
role="alert"
>
<div id="error-msg-{{ alert_id }}-minimized"
class="flex justify-between items-center gap-6 w-full max-w-screen h-12 p-3 text-white bg-orange-800">
class="flex justify-between items-center gap-6 w-full max-w-screen h-12 p-3 text-white bg-orange-800 shrink-0">
<div class="flex flex-row no-wrap items-center">
<div class="inline-flex items-center justify-center flex-shrink-0 w-8 h-8 text-orange-950 bg-orange-950 rounded-lg">
{% include "/components/popups/alerts/error/error-icon.svg" %}
<span class="sr-only">Error icon</span>
</div>
<div id="short-error-msg-{{ alert_id }}"
class="ms-3 text-sm font-normal text-white">{{ short_err_msg }}</div>
class="ms-3 text-sm font-normal text-white overflow-y-scroll overflow-x-scroll max-h-9">{{ short_err_msg }}</div>
</div>

<div>
<div class="flex-shrink-0">
<button id="toggle-error-alert-{{ alert_id }}"
type="button"
hx-get="/alert/{{ alert_id }}/show-maximized-error"
Expand Down
1 change: 0 additions & 1 deletion src/howitz/templates/components/row/collapse-btn.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
hx-get="/events/{{ id }}/collapse_row"
hx-trigger="click"
hx-swap="multi:#event-accordion-row-{{ id }}:outerHTML,#event-details-row-{{ id }}:delete"
hx-target-error="body"
hx-indicator="#row-{{ id }}-indicator"
>

Expand Down
1 change: 0 additions & 1 deletion src/howitz/templates/components/row/event-row.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
hx-get="/events/{{ id }}/expand_row"
hx-trigger="dblclick"
hx-target="#event-accordion-row-{{ id }}"
hx-target-error="body"
hx-swap="outerHTML"
>
{% with padding='px-3 py-2' %}
Expand Down
1 change: 0 additions & 1 deletion src/howitz/templates/components/row/expand-btn.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
hx-get="/events/{{ id }}/expand_row"
hx-trigger="click"
hx-target="#event-accordion-row-{{ id }}"
hx-target-error="body"
hx-swap="outerHTML"
hx-indicator="#row-{{ id }}-indicator"
>
Expand Down
1 change: 0 additions & 1 deletion src/howitz/templates/components/row/expanded-row.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
hx-get="/events/{{ id }}/collapse_row"
hx-trigger="dblclick"
hx-swap="multi:#event-accordion-row-{{ id }}:outerHTML,#event-details-row-{{ id }}:delete"
hx-target-error="body"
>

{% with padding='px-3 py-2' %}
Expand Down
1 change: 0 additions & 1 deletion src/howitz/templates/components/table/events-table.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
hx-swap="innerHTML"
hx-target="#eventlist-list"
hx-trigger="load"
hx-target-error="body"
>
<thead class="text-xs text-center text-gray-700 uppercase bg-gray-50 dark:bg-gray-700 dark:text-gray-300">
<tr>
Expand Down
1 change: 1 addition & 0 deletions src/howitz/templates/layouts/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
<body
hx-ext="{% block extensions %}{% endblock %}"
class="h-full"
hx-target-error="body"
>
{% block content %} {% endblock content %}

Expand Down
6 changes: 0 additions & 6 deletions src/howitz/templates/responses/collapse-error-alert.html
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
{#<div class="text-white bg-orange-700">#}
{# <pre>#}
{# {{ err_stacktrace }}#}
{# </pre>#}
{#</div>#}

<button id="toggle-error-alert-{{ alert_id }}"
hx-swap-oob="outerHTML"
type="button"
Expand Down
8 changes: 7 additions & 1 deletion src/howitz/templates/responses/expand-error-alert.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
<div id="error-msg-{{ alert_id }}-maximized" class="rounded-md text-white p-0 m-0 bg-orange-700"><code><pre class="p-4 m-0">{{ err_description }}</pre></code></div>
<div id="error-msg-{{ alert_id }}-maximized"
class="rounded-md text-white p-0 m-3 bg-orange-700 overflow-y-scroll overflow-x-scroll max-h-[calc(100vh-9rem)] max-w-[calc(100vw-5rem)]">
<code>
<pre class="min-w-full p-4 m-0">{{ err_description }}</pre>
</code>
</div>


<button id="toggle-error-alert-{{ alert_id }}"
hx-swap-oob="outerHTML"
Expand Down
3 changes: 1 addition & 2 deletions src/howitz/templates/views/events.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

{% block page_title %} Events {% endblock %}

{% block extensions %} class-tools, multi-swap, disable-element, loading-states, response-targets {% endblock %}
{% block extensions %}class-tools, multi-swap, disable-element, loading-states, response-targets{% endblock %}


{% block content %}
Expand All @@ -18,7 +18,6 @@ <h1 class="sr-only">Events table</h1>
hx-trigger="load"
role="status"
class="htmx-indicator animate-pulse"
hx-target-error="body"
>

<span class="sr-only">Loading Table...</span>
Expand Down
2 changes: 1 addition & 1 deletion src/howitz/templates/views/login.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

{% block page_title %} Login {% endblock %}

{% block extensions %} class-tools, multi-swap, disable-element, loading-states, response-targets {% endblock %}
{% block extensions %}class-tools, multi-swap, disable-element, loading-states, response-targets{% endblock %}

{% block content %}

Expand Down

0 comments on commit 71919ad

Please sign in to comment.