Skip to content

Conversation

Gsantomaggio
Copy link
Member

@Gsantomaggio Gsantomaggio commented Apr 1, 2022

When the tcp connection is forced the client sets the status as closed.

Fixes: #97

See the comment on #97 (comment) or the test CloseProducerConsumerAfterForceCloseShouldNotRaiseError to reproduce the error.

@Gsantomaggio
Copy link
Member Author

@simone-fariselli you may be interested in this PR.

@Gsantomaggio Gsantomaggio requested a review from lukebakken April 1, 2022 15:02
@Gsantomaggio Gsantomaggio marked this pull request as draft April 1, 2022 15:15
@Gsantomaggio Gsantomaggio marked this pull request as ready for review April 1, 2022 15:22
@Gsantomaggio Gsantomaggio added this to the v1.0.0-beta.5 milestone Apr 4, 2022
@codecov-commenter
Copy link

codecov-commenter commented Apr 4, 2022

Codecov Report

Merging #98 (b501738) into main (5d44375) will increase coverage by 0.04%.
The diff coverage is 94.80%.

❗ Current head b501738 differs from pull request most recent head f63d30d. Consider uploading reports for the commit f63d30d to get more accurate results

@@            Coverage Diff             @@
##             main      #98      +/-   ##
==========================================
+ Coverage   91.39%   91.44%   +0.04%     
==========================================
  Files          69       69              
  Lines        4696     4743      +47     
  Branches      272      276       +4     
==========================================
+ Hits         4292     4337      +45     
- Misses        343      345       +2     
  Partials       61       61              
Impacted Files Coverage Δ
Tests/Amqp10Tests.cs 100.00% <ø> (ø)
Tests/ClientTests.cs 97.29% <ø> (ø)
Tests/ConsumerSystemTests.cs 100.00% <ø> (ø)
Tests/PermissionTests.cs 100.00% <ø> (ø)
Tests/ProducerSystemTests.cs 99.42% <ø> (ø)
Tests/StreamSpecTests.cs 100.00% <ø> (ø)
Tests/UnitTests.cs 97.64% <ø> (ø)
Tests/Utils.cs 90.90% <85.00%> (-2.12%) ⬇️
RabbitMQ.Stream.Client/Client.cs 89.91% <96.29%> (+0.58%) ⬆️
RabbitMQ.Stream.Client/Connection.cs 93.51% <100.00%> (+0.31%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d44375...f63d30d. Read the comment docs.

@lukebakken
Copy link
Contributor

I'm investigating this failure:

[xUnit.net 00:01:02.85]         C:\Users\bakkenl\development\rabbitmq\rabbitmq-stream-dotnet-client\Tests\SystemTests.cs(213,0): at Tests.SystemTests.CloseProducerConsumerAfterForceCloseShouldNotRaiseError()
[xUnit.net 00:01:02.85]            at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__127_0(Object state)
  Failed Tests.SystemTests.CloseProducerConsumerAfterForceCloseShouldNotRaiseError [15 s]
  Error Message:
   System.TimeoutException : The operation has timed out.
  Stack Trace:
     at RabbitMQ.Stream.Client.ManualResetValueTaskSource`1.System.Threading.Tasks.Sources.IValueTaskSource<T>.GetResult(Int16 token) in C:\Users\bakkenl\development\rabbitmq\rabbitmq-stream-dotnet-client\RabbitMQ.Stream.Client\Client.cs:line 559
   at RabbitMQ.Stream.Client.Client.Request[TIn,TOut](Func`2 request, Nullable`1 timeout) in C:\Users\bakkenl\development\rabbitmq\rabbitmq-stream-dotnet-client\RabbitMQ.Stream.Client\Client.cs:line 291
   at RabbitMQ.Stream.Client.Client.Request[TIn,TOut](Func`2 request, Nullable`1 timeout) in C:\Users\bakkenl\development\rabbitmq\rabbitmq-stream-dotnet-client\RabbitMQ.Stream.Client\Client.cs:line 293
   at RabbitMQ.Stream.Client.Client.Unsubscribe(Byte subscriptionId) in C:\Users\bakkenl\development\rabbitmq\rabbitmq-stream-dotnet-client\RabbitMQ.Stream.Client\Client.cs:line 269
   at RabbitMQ.Stream.Client.Consumer.Close() in C:\Users\bakkenl\development\rabbitmq\rabbitmq-stream-dotnet-client\RabbitMQ.Stream.Client\Consumer.cs:line 127
   at Tests.SystemTests.CloseProducerConsumerAfterForceCloseShouldNotRaiseError() in C:\Users\bakkenl\development\rabbitmq\rabbitmq-stream-dotnet-client\Tests\SystemTests.cs:line 213
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__127_0(Object state)

@lukebakken lukebakken force-pushed the handle_forced_close branch 3 times, most recently from 128c334 to 818552a Compare April 4, 2022 15:44
When the tcp connection is forced the client sets the status as closed.

Make IsClosed setter private and only set in the Client ctor

Remove limitations on test parallelism, add IsClosed to Connection
@lukebakken lukebakken force-pushed the handle_forced_close branch from 818552a to f63d30d Compare April 4, 2022 16:24
@lukebakken lukebakken merged commit a6f8240 into main Apr 4, 2022
@lukebakken lukebakken deleted the handle_forced_close branch April 4, 2022 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Producer Close Exception when the producer is closed
3 participants