Skip to content

Improve diagnostic logging across MTP crash and IPC paths#8584

Merged
Evangelink merged 3 commits into
mainfrom
dev/amauryleve/mtp-diagnostic-logging
May 26, 2026
Merged

Improve diagnostic logging across MTP crash and IPC paths#8584
Evangelink merged 3 commits into
mainfrom
dev/amauryleve/mtp-diagnostic-logging

Conversation

@Evangelink
Copy link
Copy Markdown
Member

Summary

Surgical improvements to internal logging across high-risk crash and diagnostic paths in Microsoft.Testing.Platform. The goal is to make it noticeably easier to diagnose problems and crashes from logs alone, without changing any public API.

Changes

File Improvement
Helpers/UnhandledExceptionHandler.cs Pattern-match e.ExceptionObject is Exception, route the typed exception through Log(LogLevel.Critical, msg, ex, Formatter) (structured sinks now get the object, not just a stringified message), and pass it into FailFast(msg, ex). Console output kept full-fidelity for backward-compat with acceptance tests.
Hosts/ServerTestHost.cs Same exception-routing fix for OnCurrentDomainUnhandledException / OnTaskSchedulerUnobservedTaskException.
IPC/NamedPipeClient.cs On IPC EOF the client called _environment.Exit(GenericFailure) with zero diagnostic output. Now writes a clear Console.Error.WriteLineAsync diagnostic naming the pipe and the most likely cause before exiting. Wrapped in try/catch so logging failures cannot shadow the original issue. [Embedded] type cannot take an ILogger without breaking MTP_MSBUILD_TASKS source-embedded compilation, so stderr is the pragmatic compromise.
IPC/NamedPipeServer.cs Debug log on graceful client disconnect (was a silent return). Error log with pipe name, WasConnected and LoopTaskStatus before throwing on Dispose / DisposeAsync timeout.
Configurations/JsonConfigurationProvider.cs Added null-safe LogDebugAsync / LogErrorAsync helpers. Logs: previously swallowed Path.GetFullPath failure; missing default config file (was silent return); JsonConfigurationFileParser.Parse failures with file path before rethrowing.
Configurations/ConfigurationManager.cs Trace-level full-config dump wrapped in try/catch so I/O failures cannot break configuration loading.
Hosts/TestHostControllersTestHost.cs Debug log when PID lookup InvalidOperationException is swallowed. Warning log on non-graceful exit including OS exit code, IPC-reported exit code (or <not received>), whether TestHostCompletedRequest was received, PID, and CancellationRequested.
Requests/TestHostTestFrameworkInvoker.cs Session UID logged at Debug. Session warnings / failures previously only flowed to the output device; now also logged at Warning / Error with phase (CreateTestSession / CloseTestSession) and session UID.

Deliberately out of scope

  • Microsoft.Testing.Extensions.HotReload has zero logging today — sizable instrumentation effort, deferred.
  • ServerMode/JsonRpc/TcpMessageHandler.cs / MessageHandlerFactory.cs — connection loss, malformed headers and deserialization failures not logged. Substantial server-mode surface change, deferred.
  • NamedPipeClient cannot accept an ILogger without breaking the [Embedded] / MTP_MSBUILD_TASKS source-embedded constraint.

Validation

  • .\build.cmd -projects Microsoft.Testing.Platform.csproj — green for net8.0, net9.0, netstandard2.0.
  • dotnet test Microsoft.Testing.Platform.UnitTests --filter "FullyQualifiedName~Configuration|FullyQualifiedName~UnhandledException|FullyQualifiedName~NamedPipe|FullyQualifiedName~ServerTestHost|FullyQualifiedName~TestHostControllers|FullyQualifiedName~TestHostTestFrameworkInvoker" — 75/75 pass.
  • dotnet test Microsoft.Testing.Extensions.UnitTests — 334 pass, 4 skipped (platform-specific).
  • Existing acceptance tests (UnhandledExceptionPolicyTests.cs) only assert message prefixes; verified the new structure still matches.

Notes

  • No public API changes. All edits are internal to existing types.
  • No .resx changes — MTP log messages are not localized.

Surgical improvements to internal logging across high-risk crash/diagnostic paths in Microsoft.Testing.Platform. No public API changes.

- UnhandledExceptionHandler/ServerTestHost: pattern-match e.ExceptionObject as Exception and pass the typed exception via structured logging instead of stringifying it into the message; route exception into FailFast.

- NamedPipeClient: write a stderr diagnostic naming the pipe before silent _environment.Exit on IPC EOF (best-effort, wrapped in try/catch).

- NamedPipeServer: log Debug on graceful client disconnect; log Error with pipe name, WasConnected and LoopTaskStatus before throwing on Dispose/DisposeAsync timeout.

- JsonConfigurationProvider: add null-safe LogDebugAsync/LogErrorAsync helpers; log swallowed Path.GetFullPath failure, missing default config file, and wrap parser invocation with Error log including file path.

- ConfigurationManager: wrap Trace-level full-config dump in try/catch so I/O failures cannot break configuration loading.

- TestHostControllersTestHost: log Debug when PID lookup InvalidOperationException is swallowed; log Warning on non-graceful exit with OS exit code, IPC-reported exit code, TestHostCompletedRequest flag, PID and CancellationRequested.

- TestHostTestFrameworkInvoker: log Debug session UID; log Warning/Error from session results with phase (CreateTestSession/CloseTestSession) and session UID.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 25, 2026 21:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves internal diagnostics across Microsoft.Testing.Platform crash, IPC, configuration, and test-host session paths without changing public API.

Changes:

  • Routes typed exceptions into logger exception parameters for unhandled exception paths.
  • Adds diagnostic logging for IPC disconnects, non-graceful test-host exits, config parse/load failures, and session warnings/failures.
  • Makes trace-level configuration file dumping best-effort so diagnostic I/O failures do not break configuration loading.
Show a summary per file
File Description
src/Platform/Microsoft.Testing.Platform/Requests/TestHostTestFrameworkInvoker.cs Logs session UID and mirrors session warning/failure results to diagnostic logs.
src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs Logs graceful client disconnects and dispose timeout diagnostics.
src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeClient.cs Emits a stderr diagnostic before exiting on IPC EOF.
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostControllersTestHost.cs Adds debug/warning details for PID lookup failures and non-graceful test-host exits.
src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs Passes unhandled exceptions through logger exception parameters while preserving output-device text.
src/Platform/Microsoft.Testing.Platform/Helpers/UnhandledExceptionHandler.cs Separates console text from structured log text and passes typed exceptions to logging/FailFast.
src/Platform/Microsoft.Testing.Platform/Configurations/JsonConfigurationProvider.cs Adds null-safe debug/error logging for path resolution, missing default config, and parse failures.
src/Platform/Microsoft.Testing.Platform/Configurations/ConfigurationManager.cs Wraps trace config dump in best-effort logging.

Copilot's findings

  • Files reviewed: 8/8 changed files
  • Comments generated: 0

Comment thread src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeClient.cs Fixed
- TestHostControllersTestHost: drop redundant _testHostPID.HasValue ternary; the value is guaranteed by the Unreachable guard a few lines above.

- NamedPipeClient: replace bare catch with typed/filtered catch for the expected non-fatal exceptions from Console.Error.WriteLineAsync.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses PR feedback: each diagnostic field on its own line is easier to read in log files than a single comma-separated line.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 26, 2026 10:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new

@Evangelink Evangelink enabled auto-merge (squash) May 26, 2026 12:08
@Evangelink Evangelink merged commit cf0c485 into main May 26, 2026
47 checks passed
@Evangelink Evangelink deleted the dev/amauryleve/mtp-diagnostic-logging branch May 26, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants