From 29d508f9f685f85de6ba517bf135da564568731e Mon Sep 17 00:00:00 2001 From: Thomas Chopitea Date: Thu, 30 Jul 2026 09:44:45 +0000 Subject: [PATCH] Fix dead retry-on-connection-failure logic in ArangoDatabase.connect() connect()'s startup retry loop is meant to tolerate ArangoDB being briefly unreachable: retry up to 4 times over ~20s, then log clearly and exit(1) rather than crash. It caught requests.exceptions.ConnectionError, but python-arango 8.1.x's own host resolver (connection.py) catches the underlying transport error internally and re-raises the builtin ConnectionError instead -- verified requests.exceptions.ConnectionError and the builtin ConnectionError are unrelated sibling classes (both subclass OSError independently), so the except clause could never match what python-arango actually raises. Net effect: any process started before ArangoDB is ready to accept connections crashes immediately with an unhandled ConnectionAbortedError traceback on the very first connection attempt, instead of retrying and exiting cleanly. Only the arangodb service has a restart policy in the yeti-docker compose files (api/tasks/events-tasks/tasks-beat don't), so a crash here doesn't self-heal via Docker either. Fix: catch both exception types, since which one surfaces depends on where in the request pipeline the failure originates. Added a regression test that points connect() at an unreachable host (mocking time.sleep to keep it fast) and asserts it raises SystemExit (the clean exit(1) path) rather than letting ConnectionAbortedError escape unhandled -- verified it fails with an uncaught ConnectionAbortedError on the pre-fix code and passes with the fix. --- core/database_arango.py | 7 ++++++- tests/core_tests/database_arango.py | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 tests/core_tests/database_arango.py diff --git a/core/database_arango.py b/core/database_arango.py index cb8efc867..ceaa28718 100644 --- a/core/database_arango.py +++ b/core/database_arango.py @@ -195,7 +195,12 @@ def connect( try: yeti_db = sys_db.has_database(database) break - except requests.exceptions.ConnectionError as e: + # python-arango's own host-resolver raises the builtin + # ConnectionError when it exhausts its internal retries, rather + # than propagating requests' ConnectionError -- catch both, since + # which one surfaces depends on where in the request the failure + # originates. + except (ConnectionError, requests.exceptions.ConnectionError) as e: logging.error("Connection error: {0:s}".format(str(e))) logging.error("Retrying in 5 seconds...") time.sleep(5) diff --git a/tests/core_tests/database_arango.py b/tests/core_tests/database_arango.py new file mode 100644 index 000000000..807e5ab84 --- /dev/null +++ b/tests/core_tests/database_arango.py @@ -0,0 +1,18 @@ +import unittest +from unittest import mock + +from core import database_arango + + +class ConnectRetryTest(unittest.TestCase): + def test_connect_retries_and_exits_cleanly_on_unreachable_host(self) -> None: + """connect() must retry (not crash with an unhandled exception) when + ArangoDB is unreachable, and exit cleanly once it exhausts its + retries. python-arango's host resolver wraps a failed connection + attempt in the builtin ConnectionError rather than + requests.exceptions.ConnectionError, so the retry loop's except + clause needs to catch both.""" + db = database_arango.ArangoDatabase() + with mock.patch("core.database_arango.time.sleep"): + with self.assertRaises(SystemExit): + db.connect(host="127.0.0.1", port=1, username="root", password="")