Skip to content

Commit 978c081

Browse files
authored
DYN-10248: Fix crash when opening the Python Migration Assistant on an IronPython2 node (#17022)
1 parent 6113887 commit 978c081

2 files changed

Lines changed: 157 additions & 8 deletions

File tree

src/PythonMigrationViewExtension/MigrationAssistant/PythonMigrationAssistantViewModel.cs

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ internal class PythonMigrationAssistantViewModel : NotificationObject
2424
private readonly Version dynamoVersion;
2525
private PythonNode PythonNode;
2626

27+
private readonly Func<string, string> codeConverter;
2728
private IDiffViewViewModel currentViewModel;
2829
private SideBySideDiffModel diffModel;
2930

@@ -43,27 +44,47 @@ public IDiffViewViewModel CurrentViewModel
4344
set { this.currentViewModel = value; RaisePropertyChanged(nameof(this.CurrentViewModel)); }
4445
}
4546

46-
public PythonMigrationAssistantViewModel(PythonNode pythonNode, WorkspaceModel workspace, IPathManager pathManager, Version dynamoVersion)
47+
/// <summary>
48+
/// Initializes the view model, runs the 2-to-3 code migration, and prepares the diff model.
49+
/// If migration fails the view model is placed in an error state rather than propagating the exception.
50+
/// </summary>
51+
/// <param name="pythonNode">The Python node whose script will be migrated.</param>
52+
/// <param name="workspace">The workspace that contains the node.</param>
53+
/// <param name="pathManager">Provides backup and application paths.</param>
54+
/// <param name="dynamoVersion">The running Dynamo version, used to scope disclaimer-dismiss files.</param>
55+
/// <param name="codeConverter">
56+
/// Optional override for the migration function. Defaults to <see cref="ScriptMigrator.MigrateCode"/>.
57+
/// Intended for unit testing only.
58+
/// </param>
59+
public PythonMigrationAssistantViewModel(PythonNode pythonNode, WorkspaceModel workspace, IPathManager pathManager, Version dynamoVersion, Func<string, string> codeConverter = null)
4760
{
4861
PythonNode = pythonNode;
4962
OldCode = pythonNode.Script;
5063

5164
this.workspace = workspace;
5265
backupDirectory = pathManager.BackupDirectory;
5366
this.dynamoVersion = dynamoVersion;
67+
this.codeConverter = codeConverter;
5468

5569
try
5670
{
5771
MigrateCode();
5872
}
5973
catch (PythonException)
6074
{
61-
var sidebyside = new SideBySideDiffBuilder();
62-
diffModel = sidebyside.BuildDiffModel(OldCode, OldCode, false);
63-
64-
SetSideBySideViewModel();
65-
66-
CurrentViewModel.DiffState = State.Error;
75+
// Python script error during migration (e.g. syntax the 2to3 tool cannot parse).
76+
SetMigrationErrorState();
77+
return;
78+
}
79+
catch (Exception ex) when (
80+
ex is TypeLoadException ||
81+
ex is MissingMethodException ||
82+
ex is FileLoadException ||
83+
ex is BadImageFormatException)
84+
{
85+
// Python.Runtime assembly at runtime is incompatible (e.g. pythonnet 2.x instead of 3.x).
86+
// Types or members such as PyModule may be absent in older assemblies.
87+
SetMigrationErrorState();
6788
return;
6889
}
6990
SetSideBySideViewModel();
@@ -74,12 +95,21 @@ public PythonMigrationAssistantViewModel(PythonNode pythonNode, WorkspaceModel w
7495

7596
private void MigrateCode()
7697
{
77-
NewCode = ScriptMigrator.MigrateCode(OldCode);
98+
var converter = codeConverter ?? ScriptMigrator.MigrateCode;
99+
NewCode = converter(OldCode);
78100

79101
var sidebyside = new SideBySideDiffBuilder();
80102
diffModel = sidebyside.BuildDiffModel(OldCode, NewCode, false);
81103
}
82104

105+
private void SetMigrationErrorState()
106+
{
107+
var sidebyside = new SideBySideDiffBuilder();
108+
diffModel = sidebyside.BuildDiffModel(OldCode, OldCode, false);
109+
SetSideBySideViewModel();
110+
CurrentViewModel.DiffState = State.Error;
111+
}
112+
83113
/// <summary>
84114
/// Replaces the code in the Python node with the code changes made by the Migration Assistant.
85115
/// </summary>
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
using System;
2+
using System.IO;
3+
using Dynamo;
4+
using Dynamo.Graph.Workspaces;
5+
using Dynamo.PythonMigration.Differ;
6+
using Dynamo.PythonMigration.MigrationAssistant;
7+
using NUnit.Framework;
8+
using PythonNodeModels;
9+
10+
namespace DynamoPythonTests
11+
{
12+
[TestFixture]
13+
public class PythonMigrationAssistantViewModelTests : DynamoModelTestBase
14+
{
15+
/// <summary>
16+
/// Regression test: when Python.Runtime is an incompatible version (e.g. pythonnet 2.x loaded
17+
/// instead of 3.x), Py.CreateScope() throws TypeLoadException because PyModule does not exist
18+
/// in that assembly. The ViewModel must catch this and show an error state rather than crashing.
19+
/// </summary>
20+
[Test]
21+
public void WhenMigrationThrowsTypeLoadExceptionViewModelShowsErrorState()
22+
{
23+
// Arrange
24+
var pyNode = new PythonNode();
25+
var workspace = CurrentDynamoModel.CurrentWorkspace as WorkspaceModel;
26+
var pathManager = CurrentDynamoModel.PathManager;
27+
28+
Func<string, string> brokenMigrator = _ =>
29+
throw new TypeLoadException("Could not load type 'Python.Runtime.PyModule' from assembly 'Python.Runtime, Version=2.5.2.12086'");
30+
31+
// Act
32+
PythonMigrationAssistantViewModel viewModel = null;
33+
Assert.DoesNotThrow(() =>
34+
{
35+
viewModel = new PythonMigrationAssistantViewModel(
36+
pyNode, workspace, pathManager, new Version(3, 0), brokenMigrator);
37+
});
38+
39+
// Assert
40+
Assert.IsNotNull(viewModel);
41+
Assert.AreEqual(State.Error, viewModel.CurrentViewModel.DiffState);
42+
}
43+
44+
/// <summary>
45+
/// Verifies that a MissingMethodException (e.g. missing member in incompatible Python.Runtime)
46+
/// during migration is handled gracefully and shows an error state.
47+
/// </summary>
48+
[Test]
49+
public void WhenMigrationThrowsMissingMethodExceptionViewModelShowsErrorState()
50+
{
51+
var pyNode = new PythonNode();
52+
var workspace = CurrentDynamoModel.CurrentWorkspace as WorkspaceModel;
53+
var pathManager = CurrentDynamoModel.PathManager;
54+
55+
Func<string, string> brokenMigrator = _ =>
56+
throw new MissingMethodException("Python.Runtime.Py", "CreateScope");
57+
58+
PythonMigrationAssistantViewModel viewModel = null;
59+
Assert.DoesNotThrow(() =>
60+
{
61+
viewModel = new PythonMigrationAssistantViewModel(
62+
pyNode, workspace, pathManager, new Version(3, 0), brokenMigrator);
63+
});
64+
65+
Assert.IsNotNull(viewModel);
66+
Assert.AreEqual(State.Error, viewModel.CurrentViewModel.DiffState);
67+
}
68+
69+
/// <summary>
70+
/// Verifies that a FileLoadException (e.g. assembly version conflict) during migration
71+
/// is handled gracefully and shows an error state.
72+
/// </summary>
73+
[Test]
74+
public void WhenMigrationThrowsFileLoadExceptionViewModelShowsErrorState()
75+
{
76+
var pyNode = new PythonNode();
77+
var workspace = CurrentDynamoModel.CurrentWorkspace as WorkspaceModel;
78+
var pathManager = CurrentDynamoModel.PathManager;
79+
80+
Func<string, string> brokenMigrator = _ =>
81+
throw new FileLoadException("Could not load file or assembly 'Python.Runtime'");
82+
83+
PythonMigrationAssistantViewModel viewModel = null;
84+
Assert.DoesNotThrow(() =>
85+
{
86+
viewModel = new PythonMigrationAssistantViewModel(
87+
pyNode, workspace, pathManager, new Version(3, 0), brokenMigrator);
88+
});
89+
90+
Assert.IsNotNull(viewModel);
91+
Assert.AreEqual(State.Error, viewModel.CurrentViewModel.DiffState);
92+
}
93+
94+
/// <summary>
95+
/// Verifies that a BadImageFormatException (e.g. 32/64-bit or .NET target mismatch in
96+
/// Python.Runtime) during migration is handled gracefully and shows an error state.
97+
/// </summary>
98+
[Test]
99+
public void WhenMigrationThrowsBadImageFormatExceptionViewModelShowsErrorState()
100+
{
101+
var pyNode = new PythonNode();
102+
var workspace = CurrentDynamoModel.CurrentWorkspace as WorkspaceModel;
103+
var pathManager = CurrentDynamoModel.PathManager;
104+
105+
Func<string, string> brokenMigrator = _ =>
106+
throw new BadImageFormatException("The format of the file 'Python.Runtime' is invalid.");
107+
108+
PythonMigrationAssistantViewModel viewModel = null;
109+
Assert.DoesNotThrow(() =>
110+
{
111+
viewModel = new PythonMigrationAssistantViewModel(
112+
pyNode, workspace, pathManager, new Version(3, 0), brokenMigrator);
113+
});
114+
115+
Assert.IsNotNull(viewModel);
116+
Assert.AreEqual(State.Error, viewModel.CurrentViewModel.DiffState);
117+
}
118+
}
119+
}

0 commit comments

Comments
 (0)