Skip to content

fix(migrations): catch CommandError-wrapped ResolutionError in rolling-deployment skip - #2117

Merged
nicoloboschi merged 1 commit into
vectorize-io:mainfrom
bionicbutterfly13:fix/migrations-catch-wrapped-resolution-error
Jun 11, 2026
Merged

fix(migrations): catch CommandError-wrapped ResolutionError in rolling-deployment skip#2117
nicoloboschi merged 1 commit into
vectorize-io:mainfrom
bionicbutterfly13:fix/migrations-catch-wrapped-resolution-error

Conversation

@bionicbutterfly13

Copy link
Copy Markdown
Contributor

What

The rolling-deployment "newer bank" guard in _run_migrations_internal catches ResolutionError, but command.upgrade() never raises that directly — alembic's ScriptDirectory._catch_revision_errors wraps it in alembic.util.exc.CommandError. So when an older replica starts against a database migrated by newer code, the designed warn-and-skip path never fires and startup dies with a raw traceback.

This PR catches the wrapped form too, cause-checked: CommandError is only swallowed when isinstance(e.__cause__, ResolutionError); any other CommandError propagates unchanged.

Fixes #2114

How it was hit

0.6.1 daemon starting against a bank migrated by 0.7.2 (head c1d2e3f4a5b6) — full traceback in #2114. The handler's log message ("Skipping migrations") never appeared; instead the daemon refused to start.

Tests

New tests/test_migrations_newer_revision.py (modeled on test_migrations_thread_safety.py — monkeypatched command.upgrade, no DB needed):

  • wrapped CommandError(cause=ResolutionError) → warn-and-skip (the regression)
  • bare ResolutionError → warn-and-skip (existing behavior preserved)
  • unrelated CommandError → still raises

Verification

  • uv run pytest tests/test_migrations_newer_revision.py tests/test_migrations_thread_safety.py → 4 passed
  • ruff check / ruff format --check clean on both touched files
  • Environment: macOS (Intel x86_64), Python 3.11.15

…g-deployment skip

command.upgrade() never raises ResolutionError directly — alembic's
ScriptDirectory._catch_revision_errors wraps it in CommandError, so the
newer-bank rolling-deployment handler never fired and startup died with
a raw traceback. Catch the wrapped form (cause-checked) and route it to
the same warn-and-skip path; unrelated CommandErrors still propagate.

Fixes vectorize-io#2114

@nicoloboschi nicoloboschi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified against installed alembic 1.17.1: _catch_revision_errors raises CommandError(...) from re, so __cause__ is genuinely the ResolutionError on the real command.upgrade_upgrade_revs path. Cause-check is correct, unrelated CommandErrors still propagate, and the new no-DB tests faithfully mirror alembic's wrapping. Minimal and well-scoped.

@nicoloboschi
nicoloboschi merged commit 0afa046 into vectorize-io:main Jun 11, 2026
75 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rolling-deployment newer-bank handler never fires: alembic wraps ResolutionError in CommandError

2 participants