Repository navigation
fix(state_sync): preserve the original exception when migration rollback fails - #6130
Open
zhaoruizhe81 wants to merge 1 commit into
Open
zhaoruizhe81 wants to merge 1 commit into
zhaoruizhe81 wants to merge 1 commit into
Conversation
…ack fails On a fresh state DB the exception handler in StateMigrator.migrate() called self.rollback() unguarded. update_versions() has already written a non-zero schema_version by then, so rollback() finds no *_backup tables and raises "There are no prior migrations to roll back to." from inside the handler, replacing the original exception before 'raise SQLMeshError(...) from e' is reached. The CLI only prints str(ex), so the real cause was invisible without SQLMESH_DEBUG=1. - Wrap the handler's rollback() in its own try/except and log failures with logger.exception so rollback can no longer mask the error. - Guard the handler's second on_migration_end() call the same way, so analytics failures cannot re-raise from inside the handler. - Include the original error in the raised message and keep it as __cause__. Fixes SQLMesh#6096 Signed-off-by: zhaoruizhe <zhaoruizhe81@gmail.com>
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.
Description
What
In
StateMigrator.migrate()(sqlmesh/core/state_sync/db/migrator.py), the exception handler calledself.rollback()unguarded. On a fresh state DB,update_versions()has already written a non-zeroschema_versionwhen a later statement fails, sorollback()finds no*_backuptables and raisesSQLMeshError("There are no prior migrations to roll back to.")from inside the handler — replacing the original exception beforeraise SQLMeshError("SQLMesh migration failed.") from eis ever reached. The original error only survives in__context__, which the CLI does not print (_default_exception_handlershowsstr(ex)only).Changes:
self.rollback()call in its own try/except and log the failure withlogger.exception, so a failed rollback can no longer mask the original error.analytics.collector.on_migration_end(...)call the same way — when analytics is the original failure source it would otherwise raise again from inside the handler and bypass the wrapping below.SQLMeshError(f"SQLMesh migration failed: {e}"), keepingeas__cause__, so the cause is visible withoutSQLMESH_DEBUG=1.The rollback-semantics changes suggested in the issue (treating "no backup tables" as non-fatal on a fresh state, and when
update_versions()commits) are intentionally left out — they change behavior and deserve a separate discussion.Why
Fixes #6096. On a fresh state DB — the path every new deployment and fresh CI database takes — any bootstrap-time error was turned into an undiagnosable rollback message. In the reported case a one-line
PermissionErroron an unwritableSQLMESH_HOMEcost an instrumented debug run to find.Test Plan
test_migration_failure_after_update_versionsintests/core/state_sync/test_state_sync.py: patcheson_migration_endto fail right afterupdate_versions()and asserts the raised error carries the original exception as__cause__(with the message containing it).test_first_migration_failureonly covers failures beforeupdate_versions(), whererollback()takes the cleanschema_version == 0path and the bug cannot trigger.main(commitd69c262):pytest tests/core/state_sync/test_state_sync.py::test_migration_failure_after_update_versionsfails withsqlmesh.utils.errors.SQLMeshError: There are no prior migrations to roll back to.raised frommigrator.py:123(self.rollback()) →migrator.py:152.2 passedtogether withtest_first_migration_failure).on_migration_endraisingRuntimeError("BOOM original error"), thenContext(...).migrate()):CAUGHT: SQLMeshError: There are no prior migrations to roll back to./__cause__: NoneType: NoneCAUGHT: SQLMeshError: SQLMesh migration failed: BOOM original error/__cause__: RuntimeError: BOOM original errorpytest tests/core/state_sync -q: 103 passed.pre-commit run --all-files(themake stylehooks):ruffPassed,ruff-formatPassed;mypyreports no errors in the two touched files. (Note: my local venv does not have the fulldevextra installed — pyspark/dbt/dateparser stubs are missing — so full-tree mypy shows pre-existing import-stub errors unrelated to this change; I did not run the fullmake fast-testsuite locally.)Checklist
make styleand fixed any issuesmake fast-test)git commit -s) per the DCO