OctopusDeploy / Halibut

| Public | A secure communication stack for .NET using JSON-RPC over SSL.
Other
12 stars 44 forks source link

Including `ConnectionInitializationFailedExceptions` as connection errors #609

Closed sburmanoctopus closed 6 months ago

sburmanoctopus commented 6 months ago

[sc-70984]

Background

In Tentacle, the ClientScriptExecutionCanBeCancelledWhenRetriesAreEnabled.DuringStartScript_ForListeningTentacle_ThatIsRetryingTheRpc_AndConnecting_ScriptExecutionCanBeCancelled test was flaky.

On closer inspection, it was found that when it failed, it was forever trying to send a Cancel message to the Tentacle when it should not be.

It looks as though what was happening was that an exception was being thrown during "prepare exchange", and this exception was being considered as ConnectionState.Unknown.

The reason we care about the connection state is because if we cancel, and encounter an exception while connecting, then we can walk away. The Tentacle never received any instructions.

But if we encounter an exception after we have connected, then the Tentacle may have received instructions, and we should send a "Cancel" message to the Tentacle.

Results

Before

If we managed to get a successful connection, or reused an existing connection, but failed during the MessageExchangeProtocol.PrepareExchangeAsClientAsync (i.e. before we ever sent any request information), it would not be considered "connecting" in the exception.

After

We now consider an exception here as "connecting".

How to review this PR

Quality :heavy_check_mark:

Pre-requisites