diagnostics
diagnostics copied to clipboard
Change SocketException to ServerNotAvailable + misc test fixes/logging
This started out as a bit of test logging. Then the unrelated GetProcessInfo test failed in CI so the change grew into a small product fix + more test improvements.
Product changes:
- Connecting to the IPC channel might fail when the runtime is starting up, shutting down, or is overloaded. If so the socket.Connect() call is going to throw a SocketException. In order to avoid having callers to DiagnosticClient APIs understand all the implrementation details and have to use catch clauses for a wide variety of different exceptions we now catch this exception and rethrow it wrapped as a ServerNotAvailableException.
Test changes:
- Add logging to the DuplicateMetricNames test to improve diagnostics when it hangs.
- Add retries to the GetProcessInfo requests that are running while the target process is starting up. There is no guarantee the runtime is listening to the socket at this point and connection requests may be refused. This may resolve one cause of flakiness noted in https://github.com/dotnet/diagnostics/issues/2760. The issue also shows a 'ReadPastEndOfStream' problem that is not addressed by this fix.
- Add verification that the ExePath returned by DebuggeeCompiler actually exists on disk. This catches some build/config issues in the test setup more eagerly.
- Add verification that the BuildProjectFramework and TargetConfiguration used by SdkPrebuiltDebuggeeCompiler are populated. This catches some specific test configuration errors more eagerly.
- Tweaked some out-of-date comments
PTAL @dotnet/dotnet-diag
@noahfalk still good to merge?
I just accepted Mitchell's suggestion but it reset the review gate. But if someone can accept again - yep ready to merge!