From 656be17ec662a93ebce86e486ee90c96852ea669 Mon Sep 17 00:00:00 2001 From: Jakub Jares Date: Tue, 14 Jul 2026 13:58:25 +0200 Subject: [PATCH] Surface test host crashes during protocol negotiation When the test host crashes while negotiating the protocol version, for example because reflection based serialization is disabled and it cannot deserialize the version check message (#16274), the runner waited the whole connection timeout and then reported a generic timeout message that hides the real cause. The test host now writes the unrecoverable negotiation error to its standard error, because the protocol is not negotiated yet and it cannot send the error back over the channel, serializing the response would hit the same failure. CheckVersionWithTestHost waits for the negotiation response or for the test host to exit, whichever comes first, and when the host exits it throws with the error the host wrote to standard error instead of the timeout message. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../TestRequestSender.cs | 24 ++++++++++++++++-- .../EventHandlers/TestRequestHandler.cs | 14 +++++++++++ .../TestRequestSenderTests.cs | 18 +++++++++++++ .../EventHandlers/TestRequestHandlerTests.cs | 25 +++++++++++++++++++ 4 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.TestPlatform.CommunicationUtilities/TestRequestSender.cs b/src/Microsoft.TestPlatform.CommunicationUtilities/TestRequestSender.cs index 8361b07208..78d142ace3 100644 --- a/src/Microsoft.TestPlatform.CommunicationUtilities/TestRequestSender.cs +++ b/src/Microsoft.TestPlatform.CommunicationUtilities/TestRequestSender.cs @@ -281,9 +281,29 @@ public void CheckVersionWithTestHost() _channel.Send(data); - // Wait for negotiation response + // Wait for the negotiation response, or for the test host to exit. If the test host exits + // before it answers (for example it crashed while deserializing the version check message) + // we must stop waiting immediately instead of blocking for the whole connection timeout, and + // surface the error it wrote to its standard error rather than a generic timeout message. var timeout = EnvironmentHelper.GetConnectionTimeout(); - if (!protocolNegotiated.WaitOne(timeout * 1000)) + var waitResult = WaitHandle.WaitAny([protocolNegotiated, _clientExited.WaitHandle], timeout * 1000); + + // The test host process exited before it negotiated the protocol version. + if (waitResult == 1) + { + // Surface the error the test host wrote to its standard error (captured in + // OnClientProcessExit) so the user sees the real cause instead of a timeout. + var reason = CommonResources.TestHostProcessCrashed; + if (!string.IsNullOrWhiteSpace(_clientExitErrorMessage)) + { + reason = $"{reason} : {_clientExitErrorMessage}"; + } + + throw new TestPlatformException(reason); + } + + // Neither the negotiation response nor the test host exit happened within the timeout. + if (waitResult == WaitHandle.WaitTimeout) { throw new TestPlatformException(string.Format(CultureInfo.CurrentCulture, CommonResources.VersionCheckTimedout, timeout, EnvironmentHelper.VstestConnectionTimeout)); } diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/EventHandlers/TestRequestHandler.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/EventHandlers/TestRequestHandler.cs index 0c9fb56ed2..c0c437c43c 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/EventHandlers/TestRequestHandler.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/EventHandlers/TestRequestHandler.cs @@ -376,6 +376,13 @@ public void OnMessageReceived(object? sender, MessageReceivedEventArgs messageRe _messageProcessingUnrecoverableError = ex; EqtTrace.Error("Failed processing message {0}, aborting test run.", message.MessageType); EqtTrace.Error(ex); + + // The protocol version is not negotiated yet, so we cannot report this error back to + // the runner over the communication channel (serializing the response would hit the + // same failure). Write it to standard error instead, so the runner can capture it from + // the test host process output and show the real cause instead of a connection timeout. + WriteToStandardError($"Failed to process {message.MessageType} message in test host: {ex}"); + goto case MessageType.AbortTestRun; } break; @@ -594,6 +601,13 @@ public void OnMessageReceived(object? sender, MessageReceivedEventArgs messageRe } } + // Writes a message to the test host standard error stream. Virtual so tests can observe what the + // test host writes to standard error without depending on the process-wide Console.Error stream. + internal virtual void WriteToStandardError(string message) + { + ConsoleOutput.Instance.WriteLine(message, OutputLevel.Error); + } + private static ITestCaseEventsHandler? GetTestCaseEventsHandler(string? runSettings) { ITestCaseEventsHandler? testCaseEventsHandler = null; diff --git a/test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/TestRequestSenderTests.cs b/test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/TestRequestSenderTests.cs index f1df84a12d..ad3eedb975 100644 --- a/test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/TestRequestSenderTests.cs +++ b/test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/TestRequestSenderTests.cs @@ -286,6 +286,24 @@ public void CheckVersionWithTestHostShouldThrowIfProtocolNegotiationTimeouts() Assert.AreEqual(message, TimoutErrorMessage); } + [TestMethod] + public void CheckVersionWithTestHostShouldThrowWithTestHostErrorIfTestHostExitsDuringNegotiation() + { + // The test host connected but then exited during protocol negotiation, for example it crashed while + // deserializing the version check message when reflection-based serialization is disabled (issue #16274). + // OnClientProcessExit records the standard error and signals the exit. CheckVersionWithTestHost must stop + // waiting immediately and surface that error, instead of blocking for the whole connection timeout and + // reporting a generic timeout message. + SetupFakeCommunicationChannel(); + _testRequestSender.OnClientProcessExit("System.InvalidOperationException: Reflection-based serialization has been disabled for this application."); + + var message = Assert.ThrowsExactly(() => _testRequestSender.CheckVersionWithTestHost()).Message; + + Assert.Contains("Test host process crashed", message); + Assert.Contains("Reflection-based serialization has been disabled", message); + Assert.AreNotEqual(TimoutErrorMessage, message); + } + #endregion #region Discovery Protocol Tests diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/EventHandlers/TestRequestHandlerTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/EventHandlers/TestRequestHandlerTests.cs index 8c3a7910c7..05289f46f5 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/EventHandlers/TestRequestHandlerTests.cs +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/EventHandlers/TestRequestHandlerTests.cs @@ -169,6 +169,24 @@ public void ProcessRequestsVersionCheckShouldLogErrorIfDiagnosticsEnableFails() SendSessionEnd(); } + [TestMethod] + public void ProcessRequestsVersionCheckFailureShouldWriteErrorToStandardError() + { + // A VersionCheck payload that is not an int makes DeserializePayload throw, which simulates + // the unrecoverable serializer failure the test host hits when reflection-based serialization is + // disabled (issue #16274). The protocol version is not negotiated yet, so the error cannot be sent + // back over the channel and must instead be written to standard error for the runner to capture. + var message = new Message { MessageType = MessageType.VersionCheck, RawMessage = JsonDataSerializer.Instance.SerializePayload(MessageType.VersionCheck, "not-an-int", 1) }; + ProcessRequestsAsync(_mockTestHostManagerFactory.Object); + + SendMessageOnChannel(message); + + var handler = (TestableTestRequestHandler)_requestHandler; + var standardError = string.Join(Environment.NewLine, handler.StandardErrorMessages); + Assert.Contains(MessageType.VersionCheck, standardError); + SendSessionEnd(); + } + #endregion #region Discovery Protocol @@ -521,6 +539,8 @@ private void VerifyResponseMessageContains(string message) public class TestableTestRequestHandler : TestRequestHandler { + public List StandardErrorMessages { get; } = new(); + public TestableTestRequestHandler( TestHostConnectionInfo testHostConnectionInfo, ICommunicationEndpointFactory communicationEndpointFactory, @@ -536,6 +556,11 @@ public TestableTestRequestHandler( { } + internal override void WriteToStandardError(string message) + { + StandardErrorMessages.Add(message); + } + private static void OnLaunchAdapterProcessWithDebuggerAttachedAckReceived(Message message) { Assert.AreEqual(MessageType.LaunchAdapterProcessWithDebuggerAttachedCallback, message.MessageType);