Fix dead retry-on-connection-failure logic in ArangoDatabase.connect() - #1331
Merged
Merged
Conversation
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.
6 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up from investigating yeti-docker#29 (empty Feeds page on fresh
install) — a more foundational bug I found underneath the task-registration
race, affecting
ArangoDatabase.connect()itself rather than just feedbootstrap.
connect()'s startup retry loop (core/database_arango.py) is meant totolerate ArangoDB being briefly unreachable: retry the initial
has_database()check up to 4 times over ~20s, then log clearly andsys.exit(1)rather than crash. It catchesrequests.exceptions.ConnectionError—but python-arango 8.1.x (the pinned version) doesn't let that exception
escape. Its own host resolver
(
arango/connection.py:178) catches the underlying transport errorinternally and re-raises the builtin
ConnectionError(
ConnectionAbortedError, specifically) instead. I verifiedrequests.exceptions.ConnectionErrorand the builtinConnectionErrorare unrelated sibling classes — both subclass
OSErrorindependently,neither is a subclass of the other (
issubclass(ConnectionAbortedError, requests.exceptions.ConnectionError)→False) — so the existing exceptclause could never match what python-arango actually raises.
Net effect: any Yeti process (webserver, celery worker, celery beat,
events consumer) started before ArangoDB is ready to accept connections
crashes immediately with an unhandled
ConnectionAbortedErrortracebackon the very first connection attempt, instead of retrying for ~20s and
exiting cleanly. Worse, only the
arangodbservice has arestartpolicy in the yeti-docker compose files —
api/tasks/events-tasks/tasks-beatdon't — so a crash here doesn't even self-heal via Docker'sown restart mechanism.
This is a real, independently-reproducible bug regardless of the
compose-level healthcheck mitigation already shipped
(yeti-platform/yeti-docker#30) — it's relevant to any deployment without
equivalent readiness gating (bare
docker run, Kubernetes without areadiness probe), and to transient ArangoDB restarts after initial
startup (the yeti-docker history has at least one commit restarting
arangodbon OOM).Fix
Catch both exception types — which one surfaces depends on where in the
request pipeline the failure originates, so there's no reason to pick
only one.
Test plan
test_connect_retries_and_exits_cleanly_on_unreachable_host(
tests/core_tests/database_arango.py): pointsconnect()at anunreachable host (mocking
time.sleepto keep the test fast) andasserts it raises
SystemExit(the cleanexit(1)path) ratherthan letting
ConnectionAbortedErrorescape unhandled.ConnectionAbortedErroron thepre-fix code, and passes with the fix.
tests/core_testssuite: 30/30 pass.tests/schemas(190/190) andtests/apiv2(198/200, same 2 knownpre-existing failures) pass unchanged.
ty check(core+yetictl and plugins jobs): 0 errors.ruff check/ruff format --check: clean.