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);