Surface test host crashes during protocol negotiation#16285
Merged
Conversation
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 (microsoft#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>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves diagnosability when the testhost crashes during protocol negotiation (before the protocol is established), ensuring the runner surfaces the underlying error promptly instead of waiting out the full connection timeout and reporting a generic timeout.
Changes:
- Testhost (
TestRequestHandler) now writes unrecoverable version-check processing failures to standard error so the runner can capture and display the root cause. - Runner (
TestRequestSender.CheckVersionWithTestHost) now waits for either protocol negotiation or testhost exit, and throws immediately with captured standard error when the host exits during negotiation. - Adds unit tests covering both: writing negotiation failures to stderr (testhost side) and surfacing stderr on early exit (runner side).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/EventHandlers/TestRequestHandlerTests.cs | Adds a unit test and a testable override to observe standard-error output from version-check failures. |
| test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/TestRequestSenderTests.cs | Adds a unit test ensuring early testhost exit during negotiation throws with captured stderr (not timeout). |
| src/Microsoft.TestPlatform.CrossPlatEngine/EventHandlers/TestRequestHandler.cs | Writes unrecoverable version-check negotiation failures to standard error via a test-overridable method. |
| src/Microsoft.TestPlatform.CommunicationUtilities/TestRequestSender.cs | Waits on negotiation-or-exit and throws with captured stderr when the testhost exits during negotiation. |
nohwnd
enabled auto-merge (squash)
July 20, 2026 11:02
Evangelink
approved these changes
Jul 20, 2026
Evangelink
left a comment
Member
There was a problem hiding this comment.
Reviewed the negotiation, host-exit, timeout, and stderr-capture paths, including concurrent response/exit ordering and compatibility with older hosts. The implementation is sound.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #16274. The serializer bug itself is fixed in 18.9 and backported to 18.8.1 in #16281, this PR is about diagnosability, so the next time the test host dies during negotiation the user sees why instead of a 90 second timeout.
When the test host crashes while negotiating the protocol version (in #16274 it could not deserialize the version check message because reflection based serialization was disabled) two things hid the cause. The test host could not send the error back, because the protocol is not negotiated yet and serializing the response hits the same failure, so the error only reached the diag log. And
CheckVersionWithTestHostonly waited on the negotiation event, it ignored the test host exit, so it blocked for the whole connection timeout and then reported a generic "waiting for response timed out after 90 seconds" message.Now the test host writes the unrecoverable negotiation error to its standard error, which the runner already captures from the process output.
CheckVersionWithTestHostwaits for the negotiation response or for the test host to exit, whichever comes first, and when the host exits it throws with the captured standard error instead of the timeout message.Added unit tests for both sides: the test host writes a failed version check to standard error, and the runner surfaces the captured error and stops waiting when the host exits during negotiation.