DYN-10248: Fix crash when opening the Python Migration Assistant on an IronPython2 node#17022
DYN-10248: Fix crash when opening the Python Migration Assistant on an IronPython2 node#17022zeusongit merged 3 commits intoDynamoDS:masterfrom
Conversation
Add an optional Func<string,string> codeConverter to PythonMigrationAssistantViewModel and use it in MigrateCode to allow injecting a custom migrator (useful for testing). Catch TypeLoadException thrown when initializing the migration environment (e.g. incompatible Python.Runtime/pythonnet versions) and set the diff view to an Error state instead of crashing. Includes a unit test that verifies the view model shows Error when the migrator throws a TypeLoadException.
Extract SetMigrationErrorState to centralize error-state setup and use it for migration failures. Add XML docs to the constructor (including the test-only codeConverter param), replace duplicate error-handling code with a helper, and broaden the exception handling to catch TypeLoadException, MissingMethodException, FileLoadException, and BadImageFormatException. Add unit tests to verify MissingMethodException and FileLoadException are handled gracefully and import System.IO in the test file.
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-10248
There was a problem hiding this comment.
Pull request overview
Fixes a crash in Dynamo’s Python Migration Assistant when an incompatible Python.Runtime (pythonnet 2.x) is loaded at runtime (e.g., while IronPython2 is active), by handling TypeLoadException (and related assembly-mismatch exceptions) and ensuring the UI shows an error state instead of letting the exception terminate the app.
Changes:
- Catch
TypeLoadException(plus a few other assembly mismatch exceptions) during migration and set the diff viewer toState.Errorrather than crashing. - Add an optional injectable
Func<string, string>migration delegate to enable unit testing without relying on pythonnet runtime behavior. - Add regression tests validating the ViewModel ends in
State.Errorwhen migration throws.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/PythonMigrationViewExtension/MigrationAssistant/PythonMigrationAssistantViewModel.cs |
Adds injectable migration delegate and expands exception handling to prevent crash and show error state. |
test/Libraries/DynamoPythonTests/PythonMigrationAssistantViewModelTests.cs |
Adds regression tests ensuring migration exceptions are handled and reflected as State.Error. |
test/Libraries/DynamoPythonTests/PythonMigrationAssistantViewModelTests.cs
Show resolved
Hide resolved
|
|
Failing test is known flaky |
src/PythonMigrationViewExtension/MigrationAssistant/PythonMigrationAssistantViewModel.cs
Show resolved
Hide resolved
Is this expected? Does this always happen? If so, does it mean that IronPython2 cannot be migrated to PythonNet3? |
yes, ScriptMigrator.MigrateCode can't run the migration tool (it requires pythonnet 3.x APIs: PythonEngine.Initialize, Py.GIL, PyModule). The fix here just prevents the crash and shows an error state, but the user still can't migrate in that scenario. |
It also depends on when DynamoIronPython2.7 is installed and its Python.Runtime.dll wins the race at load time. If PythonNet3Engine's v3.x DLL gets into the AppDomain first, ScriptMigrator works fine regardless of which engine is selected on the node. |
jasonstratton
left a comment
There was a problem hiding this comment.
Aparajit already asked my one question about the API change
So, LGTM



Purpose
When the active Python engine is IronPython2, Python.Runtime version 2.5.2 (pythonnet 2.x) can be loaded at runtime instead of pythonnet 3.x. The Migration Assistant's ScriptMigrator.MigrateCode uses PyModule — a type introduced in pythonnet 3.x — as a local variable in a using statement. When the JIT compiles that method against the 2.x assembly, the CLR throws a TypeLoadException because PyModule does not exist. PythonMigrationAssistantViewModel only caught PythonException, so the TypeLoadException propagated uncaught and crashed the application.
Key changes:
Declarations
Check these if you believe they are true
Release Notes
Fixed a crash when clicking the Python Migration Assistant button on an IronPython2 node when pythonnet 2.x is loaded at runtime.
Reviewers
(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)
(FILL ME IN, optional) Any additional notes to reviewers or testers.
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of