[dotnet-linker] Catch any exceptions from our custom steps and show them using our error reporting logic. (#9954)

* [dotnet-linker] Catch any exceptions from our custom steps and show them using our error reporting logic.

* Letting the linker handle the exceptions will not result in a particularly
  good experience, because the linker will crash.

* We can also show better information, since we have more knowledge about many
  of the exceptions we raise ourselves.

* [dotnet] Make ConfigurationAwareSubStep inherit from ExceptionalSubStep.

This required a minor modification to ExceptionalSubStep to allow for custom reporting.

* [dotnet] Implement ConfigurationAwareStep's exception handling like it's done in ExceptionalSubStep.

tools/dotnet-linker/SetupStep.cs

@@ -13,6 +13,8 @@ using Xamarin.Linker.Steps;
 namespace Xamarin {
 
 	public class SetupStep : ConfigurationAwareStep {
+		protected override string Name { get; } = "Setup";
+		protected override int ErrorCode { get; } = 2300;
 
 		List<IStep> _steps;
 		public List<IStep> Steps {
@@ -47,7 +49,7 @@ namespace Xamarin {
 			throw new InvalidOperationException ($"Could not insert {step} after {stepName} because {stepName} wasn't found.");
 		}
 
-		protected override void Process ()
+		protected override void TryProcess ()
 		{
 			// Don't use --custom-step to load each step, because this assembly
 			// is loaded into the current process once per --custom-step,

tools/dotnet-linker/Steps/CollectAssembliesStep.cs

@@ -2,9 +2,12 @@ using Mono.Cecil;
 
 namespace Xamarin.Linker {
 	public class CollectAssembliesStep : ConfigurationAwareStep {
-		protected override void ProcessAssembly (AssemblyDefinition assembly)
+		protected override string Name { get; } = "Collect Assemblies";
+		protected override int ErrorCode { get; } = 2330;
+
+		protected override void TryProcessAssembly (AssemblyDefinition assembly)
 		{
-			base.ProcessAssembly (assembly);
+			base.TryProcessAssembly (assembly);
 
 			Configuration.Assemblies.Add (assembly);
 		}

tools/dotnet-linker/Steps/CollectUnmarkedMembers.cs

@@ -10,13 +10,16 @@ namespace Xamarin.Linker {
 	public class CollectUnmarkedMembersSubStep : ConfigurationAwareSubStep {
 		Dictionary<TypeDefinition, List<TypeDefinition>> ProtocolImplementations => Configuration.DerivedLinkContext.ProtocolImplementations;
 
+		protected override string Name { get; } = "Collect Unmarked Members";
+		protected override int ErrorCode { get; } = 2230;
+
 		public override SubStepTargets Targets {
 			get {
 				return SubStepTargets.Type;
 			}
 		}
 
-		public override void ProcessType (TypeDefinition type)
+		protected override void Process (TypeDefinition type)
 		{
 			if (!Annotations.IsMarked (type))
 				LinkContext.AddLinkedAwayType (type);

tools/dotnet-linker/Steps/ConfigurationAwareStep.cs

@@ -1,5 +1,7 @@
 using System;
 using System.Collections.Generic;
+
+using Mono.Cecil;
 using Mono.Linker.Steps;
 
 using Xamarin.Bundler;
@@ -21,5 +23,69 @@ namespace Xamarin.Linker {
 			// We can't just throw an exception or exit here, since there might be only warnings in the list of exceptions.
 			ErrorHelper.Show (exceptions);
 		}
+
+		protected sealed override void Process ()
+		{
+			try {
+				TryProcess ();
+			} catch (Exception e) {
+				Report (Fail (e));
+			}
+		}
+
+		protected sealed override void ProcessAssembly (AssemblyDefinition assembly)
+		{
+			try {
+				TryProcessAssembly (assembly);
+			} catch (Exception e) {
+				Report (Fail (assembly, e));
+			}
+		}
+
+		protected sealed override void EndProcess ()
+		{
+			try {
+				TryEndProcess ();
+			} catch (Exception e) {
+				Report (FailEnd (e));
+			}
+		}
+
+		// state-aware versions to be subclassed
+		protected virtual void TryProcess ()
+		{
+		}
+
+		protected virtual void TryProcessAssembly (AssemblyDefinition assembly)
+		{
+		}
+
+		protected virtual void TryEndProcess ()
+		{
+		}
+
+		// failure overrides, with defaults
+
+		protected virtual Exception Fail (AssemblyDefinition assembly, Exception e)
+		{
+			/* Re-use MX_ExceptionalSubSteps here, it works just fine */
+			return ErrorHelper.CreateError (ErrorCode, e, Errors.MX_ExceptionalSubSteps, Name, assembly?.FullName);
+		}
+
+		protected virtual Exception Fail (Exception e)
+		{
+			return ErrorHelper.CreateError (ErrorCode | 1, e, Errors.MX_ConfigurationAwareStep, Name);
+		}
+
+		protected virtual Exception FailEnd (Exception e)
+		{
+			return ErrorHelper.CreateError (ErrorCode | 2, e, Errors.MX_ConfigurationAwareStep, Name);
+		}
+
+		// abstracts
+
+		protected abstract string Name { get; }
+
+		protected abstract int ErrorCode { get; }
 	}
 }

tools/dotnet-linker/Steps/ConfigurationAwareSubStep.cs

@@ -1,27 +1,11 @@
 using System;
 using System.Collections.Generic;
-using Mono.Linker;
-using Mono.Linker.Steps;
 
 using Xamarin.Bundler;
-using Xamarin.Tuner;
 
 namespace Xamarin.Linker {
-	public abstract class ConfigurationAwareSubStep : BaseSubStep {
-		public LinkerConfiguration Configuration { get; private set; }
-
-		public DerivedLinkContext LinkContext {
-			get { return Configuration.DerivedLinkContext; }
-		}
-
-		public override sealed void Initialize (LinkContext context)
-		{
-			base.Initialize (context);
-
-			Configuration = LinkerConfiguration.GetInstance (context);
-		}
-
-		protected void Report (Exception exception)
+	public abstract class ConfigurationAwareSubStep : ExceptionalSubStep {
+		protected override void Report (Exception exception)
 		{
 			ErrorHelper.Show (exception);
 		}
@@ -34,4 +18,3 @@ namespace Xamarin.Linker {
 		}
 	}
 }
-

tools/dotnet-linker/Steps/ExtractBindingLibrariesStep.cs

@@ -4,10 +4,11 @@ using System.Collections.Generic;
 namespace Xamarin.Linker {
 
 	public class ExtractBindingLibrariesStep : ConfigurationAwareStep {
-		protected override void EndProcess ()
-		{
-			base.EndProcess ();
+		protected override string Name { get; } = "Extract Binding Libraries";
+		protected override int ErrorCode { get; } = 2340;
 
+		protected override void TryEndProcess ()
+		{
 			// No attributes are currently linked away, which means we don't need to worry about linked away LinkWith attributes.
 			// Ref: https://github.com/mono/linker/issues/952 (still open as of this writing).
 			var exceptions = new List<Exception> ();

tools/dotnet-linker/Steps/GatherFrameworksStep.cs

@@ -8,12 +8,15 @@ using Xamarin.Linker;
 namespace Xamarin {
 
 	public class GatherFrameworksStep : ConfigurationAwareStep {
+		protected override string Name { get; } = "Gather Frameworks";
+		protected override int ErrorCode { get; } = 2310;
+
 		HashSet<string> Frameworks = new HashSet<string> ();
 		HashSet<string> WeakFrameworks = new HashSet<string> ();
 
-		protected override void ProcessAssembly (AssemblyDefinition assembly)
+		protected override void TryProcessAssembly (AssemblyDefinition assembly)
 		{
-			base.ProcessAssembly (assembly);
+			base.TryProcessAssembly (assembly);
 
 			if (Configuration.PlatformAssembly != assembly.Name.Name)
 				return;
@@ -21,10 +24,8 @@ namespace Xamarin {
 			global::Frameworks.Gather (Configuration.Application, assembly, Frameworks, WeakFrameworks);
 		}
 
-		protected override void EndProcess ()
+		protected override void TryEndProcess ()
 		{
-			base.EndProcess ();
-
 			// Remove duplicates. WeakFrameworks takes precedence
 			Frameworks.ExceptWith (WeakFrameworks);
 

tools/dotnet-linker/Steps/GenerateMainStep.cs

@@ -7,9 +7,12 @@ using Xamarin.Linker;
 namespace Xamarin {
 
 	public class GenerateMainStep : ConfigurationAwareStep {
-		protected override void EndProcess ()
+		protected override string Name { get; } = "Generate Main";
+		protected override int ErrorCode { get; } = 2320;
+
+		protected override void TryEndProcess ()
 		{
-			base.EndProcess ();
+			base.TryEndProcess ();
 
 			var registration_methods = new List<string> (Configuration.RegistrationMethods);
 			var items = new List<MSBuildItem> ();

tools/dotnet-linker/Steps/LoadNonSkippedAssembliesStep.cs

@@ -6,10 +6,12 @@ using Mono.Linker;
 namespace Xamarin.Linker {
 	// List all the assemblies we care about (i.e. the ones that have not been linked away)
 	public class LoadNonSkippedAssembliesStep : ConfigurationAwareStep {
+		protected override string Name { get; } = "Load Non Skipped Assemblies";
+		protected override int ErrorCode { get; } = 2350;
 
-		protected override void ProcessAssembly (AssemblyDefinition assembly)
+		protected override void TryProcessAssembly (AssemblyDefinition assembly)
 		{
-			base.ProcessAssembly (assembly);
+			base.TryProcessAssembly (assembly);
 
 			// Figure out if an assembly is linked away or not
 			if (Context.Annotations.HasAction (assembly)) {

tools/dotnet-linker/Steps/PreserveBlockCodeSubStep.cs

@@ -13,6 +13,9 @@ namespace Xamarin.Linker.Steps {
 		MethodDefinition ctor_string_def;
 		MethodReference ctor_string_ref;
 
+		protected override string Name { get; } = "Preserve Block Code";
+		protected override int ErrorCode { get; } = 2240;
+
 		public override SubStepTargets Targets {
 			get {
 				return SubStepTargets.Assembly |
@@ -53,18 +56,14 @@ namespace Xamarin.Linker.Steps {
 			return ctor_string_ref;
 		}
 
-		public override void ProcessAssembly (AssemblyDefinition assembly)
+		protected override void Process (AssemblyDefinition assembly)
 		{
 			// Clear out the method reference we have, so that we import the method definition again
 			ctor_string_ref = null;
-
-			base.ProcessAssembly (assembly);
 		}
 
-		public override void ProcessField (FieldDefinition field)
+		protected override void Process (FieldDefinition field)
 		{
-			base.ProcessField (field);
-
 			PreserveBlockField (field);
 		}
 

tools/dotnet-linker/Steps/RegistrarStep.cs

@@ -6,10 +6,11 @@ using Xamarin.Utils;
 
 namespace Xamarin.Linker {
 	public class RegistrarStep : ConfigurationAwareStep {
-		protected override void EndProcess ()
-		{
-			base.EndProcess ();
+		protected override string Name { get; } = "Registrar";
+		protected override int ErrorCode { get; } = 2360;
 
+		protected override void TryEndProcess ()
+		{
 			var app = Configuration.Application;
 
 			app.SelectRegistrar ();

tools/linker/ExceptionalSubStep.cs

@@ -47,7 +47,7 @@ namespace Xamarin.Linker {
 			try {
 				Process (assembly);
 			} catch (Exception e) {
-				throw Fail (assembly, e);
+				Report (Fail (assembly, e));
 			}
 		}
 
@@ -56,7 +56,7 @@ namespace Xamarin.Linker {
 			try {
 				Process (type);
 			} catch (Exception e) {
-				throw Fail (type, e);
+				Report (Fail (type, e));
 			}
 		}
 
@@ -65,7 +65,7 @@ namespace Xamarin.Linker {
 			try {
 				Process (field);
 			} catch (Exception e) {
-				throw Fail (field, e);
+				Report (Fail (field, e));
 			}
 		}
 
@@ -74,7 +74,7 @@ namespace Xamarin.Linker {
 			try {
 				Process (method);
 			} catch (Exception e) {
-				throw Fail (method, e);
+				Report (Fail (method, e));
 			}
 		}
 
@@ -83,7 +83,7 @@ namespace Xamarin.Linker {
 			try {
 				Process (property);
 			} catch (Exception e) {
-				throw Fail (property, e);
+				Report (Fail (property, e));
 			}
 		}
 
@@ -92,7 +92,7 @@ namespace Xamarin.Linker {
 			try {
 				Process (@event);
 			} catch (Exception e) {
-				throw Fail (@event, e);
+				Report (Fail (@event, e));
 			}
 		}
 
@@ -154,6 +154,11 @@ namespace Xamarin.Linker {
 			return ErrorHelper.CreateError (ErrorCode | 5, e, Errors.MX_ExceptionalSubSteps, Name, @event?.FullName);
 		}
 
+		protected virtual void Report (Exception e)
+		{
+			throw e;
+		}
+
 		// abstracts
 
 		protected abstract string Name { get; }

tools/mtouch/Errors.designer.cs

@@ -1523,6 +1523,12 @@ namespace Xamarin.Bundler {
             }
         }
         
+        internal static string MX_ConfigurationAwareStep {
+            get {
+                return ResourceManager.GetString("MX_ConfigurationAwareStep", resourceCulture);
+            }
+        }
+        
         internal static string MX3001 {
             get {
                 return ResourceManager.GetString("MX3001", resourceCulture);

tools/mtouch/Errors.resx

@@ -1295,7 +1295,26 @@
 	<!-- 220x: PreserveSmartEnumConversionsSubStep -->
 	<!-- 221x: RemoveBitcodeIncompatibleCodeStep -->
 	<!-- 222x: RemoveRejectedTypesStep -->
+	<!-- 223x: CollectUnmarkedMembersSubStep -->
+	<!-- 224x: PreserveBlockCodeSubStep -->
 	
+	<!-- 2300 -> 2399 is used by/reserved for ConfigurationAwareStep subclasses in the linker -->
+
+	<data name="MX_ConfigurationAwareStep" xml:space="preserve">
+		<value>The linker step '{0}' failed during processing.</value>
+		<comment>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</comment>
+	</data>
+
+	<!-- 230x: SetupStep -->
+	<!-- 231x: GatherFrameworksStep -->
+	<!-- 232x: GenerateMainStep -->
+	<!-- 233x: CollectAssembliesStep -->
+	<!-- 234x: ExtractBindingLibrariesStep -->
+	<!-- 235x: LoadNonSkippedAssembliesStep -->
+	<!-- 236x: RegistrarStep -->
+
 	<data name="MX3001" xml:space="preserve">
 		<value>Could not {0} the assembly '{1}'
 		</value>

tools/mtouch/xlf/Errors.cs.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>

tools/mtouch/xlf/Errors.de.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>

tools/mtouch/xlf/Errors.es.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>

tools/mtouch/xlf/Errors.fr.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>

tools/mtouch/xlf/Errors.it.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>

tools/mtouch/xlf/Errors.ja.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>

tools/mtouch/xlf/Errors.ko.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>

tools/mtouch/xlf/Errors.pl.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>

tools/mtouch/xlf/Errors.pt-BR.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>

tools/mtouch/xlf/Errors.ru.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>

tools/mtouch/xlf/Errors.tr.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>

tools/mtouch/xlf/Errors.zh-Hans.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>

tools/mtouch/xlf/Errors.zh-Hant.xlf

@@ -2768,6 +2768,13 @@
 		</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="MX_ConfigurationAwareStep">
+        <source>The linker step '{0}' failed during processing.</source>
+        <target state="new">The linker step '{0}' failed during processing.</target>
+        <note>This is a message when processing fails in the linker.
+			{0} - The name of the step that fails.
+		</note>
+      </trans-unit>
       <trans-unit id="MX_ExceptionalSubSteps">
         <source>The linker step '{0}' failed processing `{1}`.</source>
         <target state="new">The linker step '{0}' failed processing `{1}`.</target>