From b5077f0aa3212f4e1bb4bfc419c8bb97d1afdc01 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Thu, 6 Jun 2024 11:01:32 +0200 Subject: [PATCH 01/10] Make sure stacktrace is included when re-raising NotConnectedError --- src/howitz/endpoints.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/howitz/endpoints.py b/src/howitz/endpoints.py index 057675c1..bc0f4031 100644 --- a/src/howitz/endpoints.py +++ b/src/howitz/endpoints.py @@ -89,7 +89,7 @@ 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 - raise + raise notConnErr else: # This error is intermittent - increase counter and retry current_app.logger.exception('Intermittent NotConnectedError %s', notConnErr) session["not_connected_counter"] += 1 @@ -120,7 +120,7 @@ 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 - raise + raise notConnErr else: # This error is intermittent - increase counter and retry current_app.logger.exception('Intermittent NotConnectedError %s', notConnErr) session["not_connected_counter"] += 1 From 161880969ad0fce44c62c5aa5ebb90bf56390ba1 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Thu, 6 Jun 2024 11:05:04 +0200 Subject: [PATCH 02/10] Move connection logic to a separate helper --- src/howitz/endpoints.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/howitz/endpoints.py b/src/howitz/endpoints.py index bc0f4031..a58256c6 100644 --- a/src/howitz/endpoints.py +++ b/src/howitz/endpoints.py @@ -45,14 +45,7 @@ def auth_handler(username, password): user = authenticate_user(current_app.database, username, password) current_app.logger.debug('User %s', user) - if not current_app.event_manager.is_connected: - current_app.event_manager = Zino1EventManager.configure(current_app.zino_config) - current_app.event_manager.connect() - current_app.logger.info('Connected to Zino %s', current_app.event_manager.is_connected) - - if not current_app.event_manager.is_authenticated: - current_app.event_manager.authenticate(username=user.username, password=user.token) - current_app.logger.info('Authenticated in Zino %s', current_app.event_manager.is_authenticated) + connect_to_zino(user.username, user.password, user.token) if current_app.event_manager.is_authenticated: # is zino authenticated current_app.logger.debug('User is Zino authenticated %s', current_app.event_manager.is_authenticated) @@ -82,6 +75,17 @@ def logout_handler(): current_app.logger.info("Logged out successfully.") +def connect_to_zino(username, password, token): + if not current_app.event_manager.is_connected: + current_app.event_manager = Zino1EventManager.configure(current_app.zino_config) + current_app.event_manager.connect() + current_app.logger.info('Connected to Zino %s', current_app.event_manager.is_connected) + + if not current_app.event_manager.is_authenticated: + current_app.event_manager.authenticate(username=username, password=token) + current_app.logger.info('Authenticated in Zino %s', current_app.event_manager.is_authenticated) + + def get_current_events(): try: current_app.event_manager.get_events() From 6d1e5e5c7ba2cedbc328cd8bf7d46fda3603ca51 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Thu, 6 Jun 2024 11:12:16 +0200 Subject: [PATCH 03/10] Add lost connection handler --- src/howitz/error_handlers.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/howitz/error_handlers.py b/src/howitz/error_handlers.py index 64ea4188..1a9d5902 100644 --- a/src/howitz/error_handlers.py +++ b/src/howitz/error_handlers.py @@ -1,8 +1,10 @@ import uuid from flask import render_template, session, current_app, make_response, request +from flask_login import current_user from werkzeug.exceptions import HTTPException +from howitz.endpoints import connect_to_zino from howitz.utils import serialize_exception @@ -75,3 +77,18 @@ def handle_403(e): response.headers['HX-Trigger'] = 'htmx:responseError' return response, 403 + + +def handle_lost_connection(e): + if isinstance(e, BrokenPipeError): + current_app.logger.exception(msg=f"{e.code} {e.name}: {e.description}") + else: + current_app.logger.exception("Lost connection to Zino server %s", e) + + if current_app.event_manager.is_connected: + current_app.event_manager.disconnect() + + connect_to_zino(current_user.username, current_user.password, current_user.token) + + # FIXME + return "", 200 From 3f4a47ec6f82a1e9f57f6dcc5d9c05eb35fcb411 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Thu, 6 Jun 2024 11:15:19 +0200 Subject: [PATCH 04/10] Register lost connection handler in the app --- src/howitz/__init__.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/howitz/__init__.py b/src/howitz/__init__.py index 3557c64c..8978e691 100644 --- a/src/howitz/__init__.py +++ b/src/howitz/__init__.py @@ -12,10 +12,10 @@ from howitz.config.utils import load_config from howitz.config.zino1 import make_zino1_config from howitz.config.howitz import make_howitz_config -from howitz.error_handlers import handle_generic_exception, handle_generic_http_exception, handle_400, handle_404, handle_403 +from howitz.error_handlers import handle_generic_exception, handle_generic_http_exception, handle_400, handle_404, handle_403, handle_lost_connection from howitz.users.db import UserDB from howitz.users.commands import user_cli -from zinolib.controllers.zino1 import Zino1EventManager +from zinolib.controllers.zino1 import Zino1EventManager, LostConnectionError, NotConnectedError __all__ = ["create_app"] @@ -29,7 +29,10 @@ def create_app(test_config=None): app.register_error_handler(HTTPException, handle_generic_http_exception) app.register_error_handler(BadRequest, handle_400) app.register_error_handler(NotFound, handle_404) - app.register_error_handler(Forbidden, handle_403), + app.register_error_handler(Forbidden, handle_403) + app.register_error_handler(LostConnectionError, handle_lost_connection) + app.register_error_handler(BrokenPipeError, handle_lost_connection) + app.register_error_handler(NotConnectedError, handle_lost_connection) # load config app = load_config(app, test_config) From 40c12c54564bd2d42224ada7b603b5a8133d45b5 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Fri, 7 Jun 2024 08:57:40 +0200 Subject: [PATCH 05/10] Drop specific handling of NotConnectedError All errors related to connection loss are now handled in a separate handler --- src/howitz/endpoints.py | 32 +++----------------------------- 1 file changed, 3 insertions(+), 29 deletions(-) diff --git a/src/howitz/endpoints.py b/src/howitz/endpoints.py index a58256c6..8ba3967b 100644 --- a/src/howitz/endpoints.py +++ b/src/howitz/endpoints.py @@ -20,7 +20,7 @@ from zinolib.controllers.zino1 import Zino1EventManager, RetryError, EventClosedError from zinolib.event_types import Event, AdmState, PortState, BFDState, ReachabilityState, LogEntry, HistoryEntry from zinolib.compat import StrEnum -from zinolib.ritz import NotConnectedError, AuthenticationError +from zinolib.ritz import AuthenticationError from howitz.users.utils import authenticate_user @@ -55,7 +55,6 @@ def auth_handler(username, password): session["selected_events"] = [] session["expanded_events"] = {} session["errors"] = {} - session["not_connected_counter"] = 0 return user raise AuthenticationError('Unexpected error on Zino authentication') @@ -71,7 +70,6 @@ def logout_handler(): session.pop('expanded_events', {}) session.pop('selected_events', []) session.pop('errors', {}) - session.pop('not_connected_counter', 0) current_app.logger.info("Logged out successfully.") @@ -87,19 +85,7 @@ def connect_to_zino(username, password, token): def get_current_events(): - try: - current_app.event_manager.get_events() - except NotConnectedError as notConnErr: - 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 - raise notConnErr - else: # This error is intermittent - increase counter and retry - current_app.logger.exception('Intermittent NotConnectedError %s', notConnErr) - session["not_connected_counter"] += 1 - current_app.event_manager.get_events() - pass - + current_app.event_manager.get_events() events = current_app.event_manager.events events_sorted = {k: events[k] for k in sorted(events, @@ -118,19 +104,7 @@ def get_current_events(): def poll_current_events(): - try: - current_app.event_manager.get_events() - except NotConnectedError as notConnErr: - 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 - raise notConnErr - else: # This error is intermittent - increase counter and retry - current_app.logger.exception('Intermittent NotConnectedError %s', notConnErr) - session["not_connected_counter"] += 1 - current_app.event_manager.get_events() - pass - + current_app.event_manager.get_events() events = current_app.event_manager.events events_sorted = {k: events[k] for k in sorted(events, From 84936c0244ceab0330698f391dc0d15004bbfc05 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Fri, 7 Jun 2024 09:30:30 +0200 Subject: [PATCH 06/10] Improve logging of errors related to lost connection --- src/howitz/error_handlers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/howitz/error_handlers.py b/src/howitz/error_handlers.py index 1a9d5902..943eaa0a 100644 --- a/src/howitz/error_handlers.py +++ b/src/howitz/error_handlers.py @@ -81,9 +81,9 @@ def handle_403(e): def handle_lost_connection(e): if isinstance(e, BrokenPipeError): - current_app.logger.exception(msg=f"{e.code} {e.name}: {e.description}") + current_app.logger.exception("Lost connection to Zino server: %s", e) else: - current_app.logger.exception("Lost connection to Zino server %s", e) + current_app.logger.exception("Lost connection to Zino server: %s", e.args[0]) if current_app.event_manager.is_connected: current_app.event_manager.disconnect() From 9aeee410b7f60e2dfd8356ca7c14189c3c78bf49 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Fri, 7 Jun 2024 09:31:47 +0200 Subject: [PATCH 07/10] Handle different scenarios when losing connection to Zino server --- src/howitz/error_handlers.py | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/howitz/error_handlers.py b/src/howitz/error_handlers.py index 943eaa0a..a54257e2 100644 --- a/src/howitz/error_handlers.py +++ b/src/howitz/error_handlers.py @@ -85,10 +85,28 @@ def handle_lost_connection(e): else: current_app.logger.exception("Lost connection to Zino server: %s", e.args[0]) - if current_app.event_manager.is_connected: - current_app.event_manager.disconnect() - - connect_to_zino(current_user.username, current_user.password, current_user.token) - - # FIXME - return "", 200 + if current_user.is_authenticated: # Re-connect to Zino with existing credentials and inform user that there was an error via alert pop-up + if current_app.event_manager.is_connected: + current_app.event_manager.disconnect() + + connect_to_zino(current_user.username, current_user.password, current_user.token) + + alert_random_id = str(uuid.uuid4()) + try: + short_err_msg = e.args[0] + except IndexError: + short_err_msg = 'Temporarily lost connection to Zino server' + + if not "errors" in session: + session["errors"] = dict() + session["errors"][str(alert_random_id)] = serialize_exception(e) + session.modified = True + + 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, 503 + else: # Redirect to /login for complete re-authentication + res = make_response() + res.headers['HX-Redirect'] = '/login' + return res From 9985ebf55f82c473d26f88666bc651b47729132d Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Fri, 7 Jun 2024 09:35:30 +0200 Subject: [PATCH 08/10] Bump zinolib requirement --- pyproject.toml | 2 +- requirements-frozen.txt | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index bd582ef4..d965c909 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ dependencies = [ "flask_assets", "flask-login", "jinja2", - "zinolib>=1.1.0", + "zinolib>=1.1.1", "werkzeug<3", # needed by flask-login "pydantic", ] diff --git a/requirements-frozen.txt b/requirements-frozen.txt index 2034f602..39e22c40 100644 --- a/requirements-frozen.txt +++ b/requirements-frozen.txt @@ -2,7 +2,7 @@ # This file is autogenerated by pip-compile with Python 3.10 # by the following command: # -# pip-compile --resolver=backtracking pyproject.toml +# pip-compile --output-file=requirements-frozen.txt constraints.txt pyproject.toml # annotated-types==0.6.0 # via pydantic @@ -45,8 +45,9 @@ webassets==2.0 # via flask-assets werkzeug==2.3.8 # via + # -r constraints.txt # flask # flask-login # howitz (pyproject.toml) -zinolib==1.0.1 +zinolib==1.1.1 # via howitz (pyproject.toml) From 7606e6a9f3d888c91fe294298773ec49a8256180 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Fri, 7 Jun 2024 10:51:48 +0200 Subject: [PATCH 09/10] Drop unused password parameter --- src/howitz/endpoints.py | 4 ++-- src/howitz/error_handlers.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/howitz/endpoints.py b/src/howitz/endpoints.py index e10b0079..75a9b0f3 100644 --- a/src/howitz/endpoints.py +++ b/src/howitz/endpoints.py @@ -45,7 +45,7 @@ def auth_handler(username, password): user = authenticate_user(current_app.database, username, password) current_app.logger.debug('User %s', user) - connect_to_zino(user.username, user.password, user.token) + connect_to_zino(user.username, user.token) if current_app.event_manager.is_authenticated: # is zino authenticated current_app.logger.debug('User is Zino authenticated %s', current_app.event_manager.is_authenticated) @@ -75,7 +75,7 @@ def logout_handler(): current_app.logger.info("Logged out successfully.") -def connect_to_zino(username, password, token): +def connect_to_zino(username, token): if not current_app.event_manager.is_connected: current_app.event_manager = Zino1EventManager.configure(current_app.zino_config) current_app.event_manager.connect() diff --git a/src/howitz/error_handlers.py b/src/howitz/error_handlers.py index a54257e2..0a401144 100644 --- a/src/howitz/error_handlers.py +++ b/src/howitz/error_handlers.py @@ -89,7 +89,7 @@ def handle_lost_connection(e): if current_app.event_manager.is_connected: current_app.event_manager.disconnect() - connect_to_zino(current_user.username, current_user.password, current_user.token) + connect_to_zino(current_user.username, current_user.token) alert_random_id = str(uuid.uuid4()) try: From a67309b6d8037e7be097749ffacd27efac15898a Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Fri, 7 Jun 2024 13:05:06 +0200 Subject: [PATCH 10/10] Log with level error when losing connection to Zino --- src/howitz/error_handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/howitz/error_handlers.py b/src/howitz/error_handlers.py index 0a401144..49b43776 100644 --- a/src/howitz/error_handlers.py +++ b/src/howitz/error_handlers.py @@ -83,7 +83,7 @@ def handle_lost_connection(e): if isinstance(e, BrokenPipeError): current_app.logger.exception("Lost connection to Zino server: %s", e) else: - current_app.logger.exception("Lost connection to Zino server: %s", e.args[0]) + current_app.logger.error("Lost connection to Zino server: %s", e.args[0]) if current_user.is_authenticated: # Re-connect to Zino with existing credentials and inform user that there was an error via alert pop-up if current_app.event_manager.is_connected: