@joebigelow / wix-1 / commits / 563857ee

Simplify and standardize exception handling in extension manager

Rob Mensching committed Dec 27, 2018 at 08:21 UTC 563857ee4097aead50ff5711c5dc759c0fcc4c5d
1 file changed +35 -45
src/WixToolset.Core/ExtensibilityServices/ExtensionManager.cs
+35 -45
@@ -44,33 +44,44 @@ namespace WixToolset.Core.ExtensibilityServices
44
45 public void Load(string extensionPath)
46 {
47 - Assembly assembly;
48 -
49 - // Absolute path to an assembly which means only "load from" will work even though we'd prefer to
50 - // use Assembly.Load (see the documentation for Assembly.LoadFrom why).
51 - if (Path.IsPathRooted(extensionPath))
52 - {
53 - assembly = ExtensionManager.ExtensionLoadFrom(extensionPath);
54 - }
55 - else if (ExtensionManager.TryExtensionLoad(extensionPath, out assembly))
47 + try
48 {
57 - // Loaded the assembly by name from the probing path.
49 + Assembly assembly;
50 +
51 + // Absolute path to an assembly which means only "load from" will work even though we'd prefer to
52 + // use Assembly.Load (see the documentation for Assembly.LoadFrom why).
53 + if (Path.IsPathRooted(extensionPath))
54 + {
55 + assembly = Assembly.LoadFrom(extensionPath);
56 + }
57 + else if (ExtensionManager.TryExtensionLoad(extensionPath, out assembly))
58 + {
59 + // Loaded the assembly by name from the probing path.
60 + }
61 + else if (ExtensionManager.TryExtensionLoad(Path.GetFileNameWithoutExtension(extensionPath), out assembly))
62 + {
63 + // Loaded the assembly by filename alone along the probing path.
64 + }
65 + else // relative path to an assembly
66 + {
67 + // We want to use Assembly.Load when we can because it has some benefits over Assembly.LoadFrom
68 + // (see the documentation for Assembly.LoadFrom). However, it may fail when the path is a relative
69 + // path, so we should try Assembly.LoadFrom one last time. We could have detected a directory
70 + // separator character and used Assembly.LoadFrom directly, but dealing with path canonicalization
71 + // issues is something we don't want to deal with if we don't have to.
72 + assembly = Assembly.LoadFrom(extensionPath);
73 + }
74 +
75 + this.Add(assembly);
76 }
59 - else if (ExtensionManager.TryExtensionLoad(Path.GetFileNameWithoutExtension(extensionPath), out assembly))
77 + catch (ReflectionTypeLoadException rtle)
78 {
61 - // Loaded the assembly by filename alone along the probing path.
79 + throw new WixException(ErrorMessages.InvalidExtension(extensionPath, String.Join(Environment.NewLine, rtle.LoaderExceptions.Select(le => le.ToString()))));
80 }
63 - else // relative path to an assembly
81 + catch (Exception e)
82 {
65 - // We want to use Assembly.Load when we can because it has some benefits over Assembly.LoadFrom
66 - // (see the documentation for Assembly.LoadFrom). However, it may fail when the path is a relative
67 - // path, so we should try Assembly.LoadFrom one last time. We could have detected a directory
68 - // separator character and used Assembly.LoadFrom directly, but dealing with path canonicalization
69 - // issues is something we don't want to deal with if we don't have to.
70 - assembly = ExtensionManager.ExtensionLoadFrom(extensionPath);
83 + throw new WixException(ErrorMessages.InvalidExtension(extensionPath, e.Message), e);
84 }
72 -
73 - this.Add(assembly);
85 }
86
87 public IEnumerable<T> Create<T>() where T : class
@@ -93,18 +104,6 @@ namespace WixToolset.Core.ExtensibilityServices
104 return extensions.Cast<T>().ToList();
105 }
106
96 - private static Assembly ExtensionLoadFrom(string assemblyName)
97 - {
98 - try
99 - {
100 - return Assembly.LoadFrom(assemblyName);
101 - }
102 - catch (Exception e)
103 - {
104 - throw new WixException(ErrorMessages.InvalidExtension(assemblyName, e.Message), e);
105 - }
106 - }
107 -
107 private static bool TryExtensionLoad(string assemblyName, out Assembly assembly)
108 {
109 try
@@ -112,19 +111,10 @@ namespace WixToolset.Core.ExtensibilityServices
111 assembly = Assembly.Load(assemblyName);
112 return true;
113 }
115 - catch (IOException innerE)
116 - {
117 - if (innerE is FileLoadException || innerE is FileNotFoundException)
118 - {
119 - assembly = null;
120 - return false;
121 - }
122 -
123 - throw new WixException(ErrorMessages.InvalidExtension(assemblyName, innerE.Message), innerE);
124 - }
125 - catch (Exception e)
114 + catch (IOException e) when (e is FileLoadException || e is FileNotFoundException)
115 {
127 - throw new WixException(ErrorMessages.InvalidExtension(assemblyName, e.Message), e);
116 + assembly = null;
117 + return false;
118 }
119 }
120 }