Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow pkgs to partially load if some of the assemblies are good #12763

Merged
merged 7 commits into from
Apr 11, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions src/DynamoCore/Models/DynamoModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1420,17 +1420,26 @@ internal void LoadNodeLibrary(Assembly assem, bool suppressZeroTouchLibraryLoad
// I think this is consistent with the current behavior - IE today - if a nodeModel exists in an assembly, the rest of the assembly
// is not imported as ZT - the same will be true if the assembly contains a NodeViewCustomization.

// TODO(mjk) draw up matrix of current behaviors which nodeLib flag can control.
if (!NodeModelAssemblyLoader.ContainsNodeModelSubType(assem)
&& !(NodeModelAssemblyLoader.ContainsNodeViewCustomizationType(assem)))
bool nodeModelOrNodeView;
try
{
nodeModelOrNodeView = !NodeModelAssemblyLoader.ContainsNodeModelSubType(assem)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These checks (ex ContainsNodeModelSubType) actually iterate over all the types in the assembly...so in the case of RhythmRevit.dll....it would throw a ReflectionTypeLoadException exception

pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
&& !(NodeModelAssemblyLoader.ContainsNodeViewCustomizationType(assem));
}
catch (Exception ex) {
throw new Exceptions.LibraryLoadFailedException(assem.Location, ex.Message);
Copy link
Contributor Author

@pinzart90 pinzart90 Apr 1, 2022

Choose a reason for hiding this comment

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

any other type of Exception would disrupt the package loading operation.
Exceptions are managed in the same way inside LibraryServices.LoadNodeLibrary, i.e LibraryLoadFailedException usually means you can continue, anything else is fatal to the loading op

}

// TODO(mjk) draw up matrix of current behaviors which nodeLib flag can control.
if (nodeModelOrNodeView)
pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
{
if (suppressZeroTouchLibraryLoad)
{
LibraryServices.LoadNodeLibrary(assem.Location, false);
}
else
{
LibraryServices.ImportLibrary(assem.Location);
LibraryServices.ImportLibrary(assem.Location, false);
pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
}

return;
Expand Down
45 changes: 33 additions & 12 deletions src/DynamoPackages/PackageLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,11 @@ private void TryLoadPackageIntoLibrary(Package package)
if (package.LoadState.State == PackageLoadState.StateTypes.Unloaded) return;

List<Assembly> loadedNodeLibs = new List<Assembly>();
List<Assembly> failedNodeLibs = new List<Assembly>();
try
{
// load node libraries
bool fatalError = false;
// Try to load node libraries from all assemblies
foreach (var assem in package.EnumerateAndLoadAssembliesInBinDirectory())
{
if (assem.IsNodeLibrary)
Expand All @@ -258,13 +260,29 @@ private void TryLoadPackageIntoLibrary(Package package)
OnRequestLoadNodeLibrary(assem.Assembly);
loadedNodeLibs.Add(assem.Assembly);
}
catch (LibraryLoadFailedException ex)
catch (Exception ex)
{
Log(ex.GetType() + ": " + ex.Message);
failedNodeLibs.Add(assem.Assembly);
if (ex is LibraryLoadFailedException)
{
// Managed exception
// We can still try to load other parts of the package
Log(ex.GetType() + ": " + ex.Message);
}
else
{
// Everything else is considered fatal to the package loading operation.
fatalError = true;
pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

if (fatalError)
{
throw new Exception("Failed to load the following assemblies : " + string.Join(", ", failedNodeLibs.Select(x => Path.GetFileName(x.Location))));
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be new LibraryLoadFailedException?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well..it could be..but I don't think it matters...since we do not catch that in any special way in this try catch

}

// load custom nodes
var packageInfo = new Graph.Workspaces.PackageInfo(package.Name, new Version(package.VersionName));
var customNodes = OnRequestLoadCustomNodeDirectory(package.CustomNodeDirectory, packageInfo);
Expand Down Expand Up @@ -292,22 +310,25 @@ private void TryLoadPackageIntoLibrary(Package package)
PythonServices.PythonEngineManager.Instance.
LoadPythonEngine(package.LoadedAssemblies.Select(x => x.Assembly));
}
catch (CustomNodePackageLoadException e)
catch (Exception e)
pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
{
Package originalPackage =
localPackages.FirstOrDefault(x => x.CustomNodeDirectory == e.InstalledPath);
OnConflictingPackageLoaded(originalPackage, package);
// Try to load any valid node libraries even if the package is in error state
PackagesLoaded?.Invoke(loadedNodeLibs);

package.LoadState.SetAsError(e.Message);
}
catch (Exception e)
{
if (e is DynamoServices.AssemblyBlockedException)

if (e is CustomNodePackageLoadException ce)
{
Package originalPackage =
localPackages.FirstOrDefault(x => x.CustomNodeDirectory == ce.InstalledPath);
OnConflictingPackageLoaded(originalPackage, package);
}
else if (e is DynamoServices.AssemblyBlockedException)
pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
{
var failureMessage = string.Format(Properties.Resources.PackageLoadFailureForBlockedAssembly, e.Message);
DynamoServices.LoadLibraryEvents.OnLoadLibraryFailure(failureMessage, Properties.Resources.LibraryLoadFailureMessageBoxTitle);
}
package.LoadState.SetAsError(e.Message);

Log("Exception when attempting to load package " + package.Name + " from " + package.RootDirectory);
Log(e.GetType() + ": " + e.Message);
}
Expand Down