Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<TestPlatformException>(() => _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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,24 @@ public void ProcessRequestsVersionCheckShouldLogErrorIfDiagnosticsEnableFails()
SendSessionEnd();
}

[TestMethod]
public void ProcessRequestsVersionCheckFailureShouldWriteErrorToStandardError()
{
// A VersionCheck payload that is not an int makes DeserializePayload<int> 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);
Comment thread
nohwnd marked this conversation as resolved.
SendSessionEnd();
}

#endregion

#region Discovery Protocol
Expand Down Expand Up @@ -521,6 +539,8 @@ private void VerifyResponseMessageContains(string message)

public class TestableTestRequestHandler : TestRequestHandler
{
public List<string> StandardErrorMessages { get; } = new();

public TestableTestRequestHandler(
TestHostConnectionInfo testHostConnectionInfo,
ICommunicationEndpointFactory communicationEndpointFactory,
Expand All @@ -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);
Expand Down