Audit LogErrorFromException #2238
Description
An occasional error that we'll see in the build logs is:
xamarin-android/build-tools/scripts/TestApks.targets(249,5): error : Root element is missing.
This message is borderline infuriating, because TestApks.targets(249,5) is a <RenameTestCases/> task execution. How is there a missing root element?!
Turns Out™ that this is MSBuild being helpful: <RenameTestCases/>'s Execute() method has a catch block which only calls Log.LogErrorFromException():
Log.LogErrorFromException() helpfully provides the filename and line information for where the task was invoked -- hence TestApks.targets(249,5) -- but the output isn't clear that there's an intervening task here. For months (years?) I've been laboring under the assumption that this Root element is missing message is being generated from MSBuild.
That is not the case. The error is reported from our task.
The Real™ problem is that the "UI" reporting of the error is inscrutable.
In short, Log.LogErrorFromException() should be considered a code smell: if it's the only thing present, then when it logs an error, the way that the error is reported -- which includes the MSBuild .targets callsite location, but no mention that this is being reported from the task that executed -- is extremely misleading and unhelpful.
We should audit our codebase for all uses of Log.LogErrorFromException() and ensure that when we do use it, we provide useful "error context" as well, so that when an error occurs, more than just Exception.Message ("Root element is missing") and the .targets call site is provided.