Create clear error messages for dotnet-trace

[mikelle-rogers]

When attempting to access Process.MainModule.Filename, if architectures are different between the app and dotnet-trace or if the app is running with admin privileges, a Win32Exception is thrown. Fixes #2781

[noahfalk]

Glad you tracked it down : ) I'm worried the error message might worry people more than help them and suggested an alternative.

[noahfalk]

I think different process architecture was the error in this particular case, but Win32Exception isn't specific to only this error condition. For example I assume you would get the same Exception if you run a process as admin and then run dotnet-trace as a non-admin user even if both processes used the same architecture.

I'd guess most users don't have any strong expectation about what the default trace file name should be. If they ran the tool a few times and the first time it outputs "MyApp_010122_034522.nettrace" and then a second time it outputs "Process1234_010122_034522.nettrace" I expect many users wouldn't notice or they would think about it for a few seconds, shrug, and then move on. How do you feel about not printing any error, setting a default process name that includes the process id, and then continuing on? I'd propose doing this not only for the Win32Exception but for all failures in the process.MainModule.FileName property. For example:

processMainModuleFileName = $"Process{processId}";
for(int attempts = 0; true; attempts++)
{
    try
    {
        processMainModuleFileName = process.MainModule.FileName;
        break;
    }
    catch {}
    Thread.Sleep(200);
}

[josalem]

I concur with Noah on this. If we can't differentiate the architecture mismatch exception from a permission mismatch (both are Win32Exception it would seem), then we should just silently continue. The architecture mismatch shouldn't be a catastrophic failure, while the permissions mismatch should result in another exception further down in the logic.

[mikelle-rogers]

The reason that I only caught the Win32Exception is because I did not want to introduce another issue where something actually is wrong, but I caught the exception and did not print anything out.

Here is the explanation for the reason to have the loop: // Reading the process MainModule filename can fail if the target process closes or isn't fully setup. Retry a few times to attempt to address the issue

Besides the explanation in the comment, is there any reason to have this loop? Is is just to get the MainModule filename? If so, I agree with your suggestion Noah. However, there could be an additional/deeper reason for ensuring MainModule works, which if we caught all exceptions, would hide the deeper reason.

[noahfalk]

LGTM : )