Add a test that closes a connection with Toxiproxy

Follow-up / replacement to #1519 * Begin addressing `CA2007` errors (missing `ConfigureAwait`) * Finish addressing `CA2007` errors (missing `ConfigureAwait`) * Increase wait time in test for CI. * Ensure toxiproxy proxy is deleted before trying to create it. * Add debugging of `rabbitmqctl list_connections` * Use correct file for `hashFiles` * Misc other changes from #1519 * Add `.ConfigureAwait(false)` * Add `TestCloseConnection` * Fix `Makefile` * Add `ToxiproxyManager` to allow multiple proxies to be set up in `TestToxiproxy` * Ensure correct port ranges are published in Ubuntu tests * Add re-tries to running external processes * Fix very subtle bug when connection is closed before `basic.ack` is received. * Add debugging to see if `list_connections` is hanging on Windows GHA runner * Init some static vars differently, remove debugging * Add an acceptable inner exception case to the test
rabbitmq · Apr 9, 2024 · dbbf038 · dbbf038
1 parent e0367ad
commit dbbf038
Show file tree

Hide file tree

Showing 30 changed files with 548 additions and 316 deletions.
diff --git a/.ci/ubuntu/gha-setup.sh b/.ci/ubuntu/gha-setup.sh
@@ -43,13 +43,13 @@ function start_toxiproxy
         # sudo ss -4nlp
         echo "[INFO] starting Toxiproxy server docker container"
         docker rm --force "$toxiproxy_docker_name" 2>/dev/null || echo "[INFO] $toxiproxy_docker_name was not running"
-        docker run --pull always --detach \
+        docker run --detach \
             --name "$toxiproxy_docker_name" \
             --hostname "$toxiproxy_docker_name" \
             --publish 8474:8474 \
-            --publish 55672:55672 \
+            --publish 55670-55680:55670-55680 \
             --network "$docker_network_name" \
-            'ghcr.io/shopify/toxiproxy:2.7.0'
+            'ghcr.io/shopify/toxiproxy:latest'
     fi
 }
 
@@ -58,7 +58,7 @@ function start_rabbitmq
     echo "[INFO] starting RabbitMQ server docker container"
     chmod 0777 "$GITHUB_WORKSPACE/.ci/ubuntu/log"
     docker rm --force "$rabbitmq_docker_name" 2>/dev/null || echo "[INFO] $rabbitmq_docker_name was not running"
-    docker run --pull always --detach \
+    docker run --detach \
         --name "$rabbitmq_docker_name" \
         --hostname "$rabbitmq_docker_name" \
         --publish 5671:5671 \

diff --git a/.editorconfig b/.editorconfig
@@ -169,6 +169,8 @@ dotnet_diagnostic.RS0036.severity = none
 dotnet_diagnostic.RS0041.severity = none
 dotnet_diagnostic.RS0051.severity = error
 
+dotnet_diagnostic.CA2007.severity = error
+
 # C++ Files
 [*.{cpp,h,in}]
 curly_bracket_next_line = true

diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml
@@ -57,7 +57,7 @@ jobs:
           # Note: the cache path is relative to the workspace directory
           # https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#using-the-cache-action
           path: ~/installers
-          key: ${{ runner.os }}-v0-${{ hashFiles('.ci/versions.json') }}
+          key: ${{ runner.os }}-v0-${{ hashFiles('.ci/windows/versions.json') }}
       - name: Download Build (Debug)
         uses: actions/download-artifact@v4
         with:
@@ -104,7 +104,7 @@ jobs:
           # Note: the cache path is relative to the workspace directory
           # https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#using-the-cache-action
           path: ~/installers
-          key: ${{ runner.os }}-v0-${{ hashFiles('.ci/versions.json') }}
+          key: ${{ runner.os }}-v0-${{ hashFiles('.ci/windows/versions.json') }}
       - name: Download Build (Debug)
         uses: actions/download-artifact@v4
         with:
@@ -190,6 +190,9 @@ jobs:
                 "${{ github.workspace }}/projects/Test/Integration/Integration.csproj" --no-restore --no-build --logger 'console;verbosity=detailed'
       - name: Check for errors in RabbitMQ logs
         run: ${{ github.workspace}}/.ci/ubuntu/gha-log-check.sh
+      - name: Maybe collect toxiproxy logs
+        if: failure()
+        run: docker logs rabbitmq-dotnet-client-toxiproxy > ${{ github.workspace }}/.ci/ubuntu/log/toxiproxy.log
       - name: Maybe upload RabbitMQ logs
         if: failure()
         uses: actions/upload-artifact@v4

diff --git a/Makefile b/Makefile
@@ -6,10 +6,23 @@ RABBITMQ_DOCKER_NAME ?= rabbitmq-dotnet-client-rabbitmq
 build:
 	dotnet build $(CURDIR)/Build.csproj
 
+# Note:
+#
+# --environment 'GITHUB_ACTIONS=true'
+#
+# The above argument is passed to `dotnet test` because it's assumed you
+# use this command to set up your local environment on Linux:
+#
+# ./.ci/ubuntu/gha-setup.sh toxiproxy
+#
+# The gha-setup.sh command has been tested on Ubuntu 22 and Arch Linux
+# and should work on any Linux system with a recent docker.
+#
 test:
 	dotnet test $(CURDIR)/projects/Test/Unit/Unit.csproj --logger 'console;verbosity=detailed'
-	dotnet test --environment "RABBITMQ_RABBITMQCTL_PATH=DOCKER:$$(docker inspect --format='{{.Id}}' $(RABBITMQ_DOCKER_NAME))" \
-		--environment 'RABBITMQ_LONG_RUNNING_TESTS=true' $(CURDIR)/projects/Test/Integration/Integration.csproj --logger 'console;verbosity=detailed'
+	dotnet test --environment 'GITHUB_ACTIONS=true' \
+		--environment "RABBITMQ_RABBITMQCTL_PATH=DOCKER:$$(docker inspect --format='{{.Id}}' $(RABBITMQ_DOCKER_NAME))" \
+		--environment 'RABBITMQ_LONG_RUNNING_TESTS=true' \
 		--environment 'RABBITMQ_TOXIPROXY_TESTS=true' \
 		--environment 'PASSWORD=grapefruit' \
 		--environment SSL_CERTS_DIR="$(CURDIR)/.ci/certs" \

diff --git a/projects/Benchmarks/Benchmarks.csproj b/projects/Benchmarks/Benchmarks.csproj
@@ -2,10 +2,14 @@
 
   <PropertyGroup Condition="$([MSBuild]::IsOSPlatform('Windows'))">
     <TargetFrameworks>net6.0;net472</TargetFrameworks>
+    <NoWarn>$(NoWarn);CA2007</NoWarn>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
   </PropertyGroup>
 
   <PropertyGroup Condition="!$([MSBuild]::IsOSPlatform('Windows'))">
     <TargetFramework>net6.0</TargetFramework>
+    <NoWarn>$(NoWarn);CA2007</NoWarn>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
   </PropertyGroup>
 
   <PropertyGroup>

diff --git a/projects/RabbitMQ.Client/client/api/InternalConstants.cs b/projects/RabbitMQ.Client/client/api/InternalConstants.cs
@@ -37,7 +37,5 @@ internal static class InternalConstants
     {
         internal static readonly TimeSpan DefaultConnectionAbortTimeout = TimeSpan.FromSeconds(5);
         internal static readonly TimeSpan DefaultConnectionCloseTimeout = TimeSpan.FromSeconds(30);
-
-        internal static string Now => DateTime.UtcNow.ToString("s", System.Globalization.CultureInfo.InvariantCulture);
     }
 }
diff --git a/projects/RabbitMQ.Client/client/impl/AutorecoveringChannel.cs b/projects/RabbitMQ.Client/client/impl/AutorecoveringChannel.cs
@@ -396,7 +396,8 @@ public Task<uint> ConsumerCountAsync(string queue)
 
         public async Task<uint> QueueDeleteAsync(string queue, bool ifUnused, bool ifEmpty, bool noWait)
         {
-            uint result = await InnerChannel.QueueDeleteAsync(queue, ifUnused, ifEmpty, noWait);
+            uint result = await InnerChannel.QueueDeleteAsync(queue, ifUnused, ifEmpty, noWait)
+                .ConfigureAwait(false);
             await _connection.DeleteRecordedQueueAsync(queue, recordedEntitiesSemaphoreHeld: false)
                 .ConfigureAwait(false);
             return result;

diff --git a/projects/RabbitMQ.Client/client/impl/AutorecoveringConnection.Recovery.cs b/projects/RabbitMQ.Client/client/impl/AutorecoveringConnection.Recovery.cs
@@ -214,7 +214,8 @@ await RecoverChannelsAndItsConsumersAsync(recordedEntitiesSemaphoreHeld: true, c
                      */
                     if (_innerConnection?.IsOpen == true)
                     {
-                        await _innerConnection.AbortAsync(Constants.InternalError, "FailedAutoRecovery", _config.RequestedConnectionTimeout);
+                        await _innerConnection.AbortAsync(Constants.InternalError, "FailedAutoRecovery", _config.RequestedConnectionTimeout)
+                            .ConfigureAwait(false);
                     }
                 }
                 catch (Exception e2)
@@ -286,7 +287,8 @@ await ch.CloseAsync()
                         try
                         {
                             _recordedEntitiesSemaphore.Release();
-                            await _config.TopologyRecoveryExceptionHandler.ExchangeRecoveryExceptionHandlerAsync(recordedExchange, ex, this);
+                            await _config.TopologyRecoveryExceptionHandler.ExchangeRecoveryExceptionHandlerAsync(recordedExchange, ex, this)
+                                .ConfigureAwait(false);
                         }
                         finally
                         {
@@ -373,7 +375,8 @@ await _recordedEntitiesSemaphore.WaitAsync()
                         try
                         {
                             _recordedEntitiesSemaphore.Release();
-                            await _config.TopologyRecoveryExceptionHandler.QueueRecoveryExceptionHandlerAsync(recordedQueue, ex, this);
+                            await _config.TopologyRecoveryExceptionHandler.QueueRecoveryExceptionHandlerAsync(recordedQueue, ex, this)
+                                .ConfigureAwait(false);
                         }
                         finally
                         {
@@ -445,7 +448,8 @@ await ch.CloseAsync()
                         try
                         {
                             _recordedEntitiesSemaphore.Release();
-                            await _config.TopologyRecoveryExceptionHandler.BindingRecoveryExceptionHandlerAsync(binding, ex, this);
+                            await _config.TopologyRecoveryExceptionHandler.BindingRecoveryExceptionHandlerAsync(binding, ex, this)
+                                .ConfigureAwait(false);
                         }
                         finally
                         {
@@ -521,7 +525,8 @@ await _recordedEntitiesSemaphore.WaitAsync()
                         try
                         {
                             _recordedEntitiesSemaphore.Release();
-                            await _config.TopologyRecoveryExceptionHandler.ConsumerRecoveryExceptionHandlerAsync(consumer, ex, this);
+                            await _config.TopologyRecoveryExceptionHandler.ConsumerRecoveryExceptionHandlerAsync(consumer, ex, this)
+                                .ConfigureAwait(false);
                         }
                         finally
                         {

diff --git a/projects/RabbitMQ.Client/client/impl/ChannelBase.cs b/projects/RabbitMQ.Client/client/impl/ChannelBase.cs
@@ -846,7 +846,8 @@ protected async Task<bool> HandleConnectionStartAsync(IncomingCommand cmd, Cance
                     var reason = new ShutdownEventArgs(ShutdownInitiator.Library, Constants.CommandInvalid, "Unexpected Connection.Start");
                     await Session.Connection.CloseAsync(reason, false,
                         InternalConstants.DefaultConnectionCloseTimeout,
-                        cancellationToken);
+                        cancellationToken)
+                        .ConfigureAwait(false);
                 }
                 else
                 {
@@ -1045,12 +1046,14 @@ public async ValueTask BasicPublishAsync<TProperties>(string exchange, string ro
                 {
                     BasicProperties props = PopulateActivityAndPropagateTraceId(basicProperties, sendActivity);
                     // TODO cancellation token
-                    await ModelSendAsync(in cmd, in props, body, CancellationToken.None);
+                    await ModelSendAsync(in cmd, in props, body, CancellationToken.None)
+                        .ConfigureAwait(false);
                 }
                 else
                 {
                     // TODO cancellation token
-                    await ModelSendAsync(in cmd, in basicProperties, body, CancellationToken.None);
+                    await ModelSendAsync(in cmd, in basicProperties, body, CancellationToken.None)
+                        .ConfigureAwait(false);
                 }
             }
             catch
@@ -1107,12 +1110,14 @@ public async void BasicPublish<TProperties>(CachedString exchange, CachedString
                 {
                     BasicProperties props = PopulateActivityAndPropagateTraceId(basicProperties, sendActivity);
                     // TODO cancellation token
-                    await ModelSendAsync(in cmd, in basicProperties, body, CancellationToken.None);
+                    await ModelSendAsync(in cmd, in basicProperties, body, CancellationToken.None)
+                        .ConfigureAwait(false);
                 }
                 else
                 {
                     // TODO cancellation token
-                    await ModelSendAsync(in cmd, in basicProperties, body, CancellationToken.None);
+                    await ModelSendAsync(in cmd, in basicProperties, body, CancellationToken.None)
+                        .ConfigureAwait(false);
                 }
             }
             catch
@@ -1155,12 +1160,14 @@ public async ValueTask BasicPublishAsync<TProperties>(CachedString exchange, Cac
                 {
                     BasicProperties props = PopulateActivityAndPropagateTraceId(basicProperties, sendActivity);
                     // TODO cancellation token
-                    await ModelSendAsync(in cmd, in props, body, CancellationToken.None);
+                    await ModelSendAsync(in cmd, in props, body, CancellationToken.None)
+                        .ConfigureAwait(false);
                 }
                 else
                 {
                     // TODO cancellation token
-                    await ModelSendAsync(in cmd, in basicProperties, body, CancellationToken.None);
+                    await ModelSendAsync(in cmd, in basicProperties, body, CancellationToken.None)
+                        .ConfigureAwait(false);
                 }
             }
             catch
@@ -1564,13 +1571,15 @@ await ModelSendAsync(method, k.CancellationToken)
 
         public async Task<uint> MessageCountAsync(string queue)
         {
-            QueueDeclareOk ok = await QueueDeclarePassiveAsync(queue);
+            QueueDeclareOk ok = await QueueDeclarePassiveAsync(queue)
+                .ConfigureAwait(false);
             return ok.MessageCount;
         }
 
         public async Task<uint> ConsumerCountAsync(string queue)
         {
-            QueueDeclareOk ok = await QueueDeclarePassiveAsync(queue);
+            QueueDeclareOk ok = await QueueDeclarePassiveAsync(queue)
+                .ConfigureAwait(false);
             return ok.ConsumerCount;
         }
 

diff --git a/projects/RabbitMQ.Client/client/impl/Connection.Commands.cs b/projects/RabbitMQ.Client/client/impl/Connection.Commands.cs
@@ -117,7 +117,8 @@ await _frameHandler.SendProtocolHeaderAsync(cancellationToken)
                  * FinishCloseAsync will cancel the main loop
                  */
                 MaybeTerminateMainloopAndStopHeartbeatTimers();
-                await FinishCloseAsync(cancellationToken);
+                await FinishCloseAsync(cancellationToken)
+                    .ConfigureAwait(false);
                 throw new ProtocolVersionMismatchException(Protocol.MajorVersion, Protocol.MinorVersion, serverVersion.Major, serverVersion.Minor);
             }
 
@@ -183,7 +184,8 @@ await _frameHandler.SendProtocolHeaderAsync(cancellationToken)
             uint heartbeatInSeconds = NegotiatedMaxValue((uint)_config.HeartbeatInterval.TotalSeconds, (uint)connectionTune.m_heartbeatInSeconds);
             Heartbeat = TimeSpan.FromSeconds(heartbeatInSeconds);
 
-            await _channel0.ConnectionTuneOkAsync(channelMax, frameMax, (ushort)Heartbeat.TotalSeconds, cancellationToken);
+            await _channel0.ConnectionTuneOkAsync(channelMax, frameMax, (ushort)Heartbeat.TotalSeconds, cancellationToken)
+                .ConfigureAwait(false);
 
             // TODO check for cancellation
             MaybeStartCredentialRefresher();

diff --git a/projects/RabbitMQ.Client/client/impl/Connection.Receive.cs b/projects/RabbitMQ.Client/client/impl/Connection.Receive.cs
@@ -87,7 +87,8 @@ await HardProtocolExceptionHandlerAsync(hpe, mainLoopToken)
             }
 
             using var cts = new CancellationTokenSource(InternalConstants.DefaultConnectionCloseTimeout);
-            await FinishCloseAsync(cts.Token);
+            await FinishCloseAsync(cts.Token)
+                .ConfigureAwait(false);
         }
 
         private async Task ReceiveLoopAsync(CancellationToken mainLoopCancelllationToken)
@@ -194,7 +195,8 @@ private async Task HardProtocolExceptionHandlerAsync(HardProtocolException hpe,
             if (SetCloseReason(hpe.ShutdownReason))
             {
                 OnShutdown(hpe.ShutdownReason);
-                await _session0.SetSessionClosingAsync(false);
+                await _session0.SetSessionClosingAsync(false)
+                    .ConfigureAwait(false);
                 try
                 {
                     var cmd = new ConnectionClose(hpe.ShutdownReason.ReplyCode, hpe.ShutdownReason.ReplyText, 0, 0);

diff --git a/projects/RabbitMQ.Client/client/impl/Connection.cs b/projects/RabbitMQ.Client/client/impl/Connection.cs
@@ -322,7 +322,8 @@ internal async Task CloseAsync(ShutdownEventArgs reason, bool abort, TimeSpan ti
                 cancellationToken.ThrowIfCancellationRequested();
 
                 OnShutdown(reason);
-                await _session0.SetSessionClosingAsync(false);
+                await _session0.SetSessionClosingAsync(false)
+                    .ConfigureAwait(false);
 
                 try
                 {

diff --git a/projects/RabbitMQ.Client/client/impl/ConsumerDispatching/ConsumerDispatcher.cs b/projects/RabbitMQ.Client/client/impl/ConsumerDispatching/ConsumerDispatcher.cs
@@ -31,7 +31,8 @@ protected override async Task ProcessChannelAsync(CancellationToken token)
                                 case WorkType.Deliver:
                                     await consumer.HandleBasicDeliverAsync(
                                         consumerTag, work.DeliveryTag, work.Redelivered,
-                                        work.Exchange, work.RoutingKey, work.BasicProperties, work.Body.Memory);
+                                        work.Exchange, work.RoutingKey, work.BasicProperties, work.Body.Memory)
+                                        .ConfigureAwait(false);
                                     break;
                                 case WorkType.Cancel:
                                     consumer.HandleBasicCancel(consumerTag);

diff --git a/projects/Test/Applications/CreateChannel/CreateChannel.csproj b/projects/Test/Applications/CreateChannel/CreateChannel.csproj
@@ -2,10 +2,14 @@
 
   <PropertyGroup Condition="$([MSBuild]::IsOSPlatform('Windows'))">
     <TargetFrameworks>net6.0;net472</TargetFrameworks>
+    <NoWarn>$(NoWarn);CA2007</NoWarn>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
   </PropertyGroup>
 
   <PropertyGroup Condition="!$([MSBuild]::IsOSPlatform('Windows'))">
     <TargetFramework>net6.0</TargetFramework>
+    <NoWarn>$(NoWarn);CA2007</NoWarn>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
   </PropertyGroup>
 
   <PropertyGroup>

diff --git a/projects/Test/Applications/MassPublish/MassPublish.csproj b/projects/Test/Applications/MassPublish/MassPublish.csproj
@@ -2,10 +2,14 @@
 
   <PropertyGroup Condition="$([MSBuild]::IsOSPlatform('Windows'))">
     <TargetFrameworks>net6.0;net472</TargetFrameworks>
+    <NoWarn>$(NoWarn);CA2007</NoWarn>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
   </PropertyGroup>
 
   <PropertyGroup Condition="!$([MSBuild]::IsOSPlatform('Windows'))">
     <TargetFramework>net6.0</TargetFramework>
+    <NoWarn>$(NoWarn);CA2007</NoWarn>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
   </PropertyGroup>
 
   <PropertyGroup>

diff --git a/projects/Test/Common/Common.csproj b/projects/Test/Common/Common.csproj
@@ -2,10 +2,14 @@
 
   <PropertyGroup Condition="$([MSBuild]::IsOSPlatform('Windows'))">
     <TargetFrameworks>net6.0;net472</TargetFrameworks>
+    <NoWarn>$(NoWarn);CA2007</NoWarn>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
   </PropertyGroup>
 
   <PropertyGroup Condition="!$([MSBuild]::IsOSPlatform('Windows'))">
     <TargetFramework>net6.0</TargetFramework>
+    <NoWarn>$(NoWarn);CA2007</NoWarn>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
   </PropertyGroup>
 
   <PropertyGroup>