-
Notifications
You must be signed in to change notification settings - Fork 595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add cancellation to initial socket connection #1428
Conversation
9c37140
to
ef1c97a
Compare
8c68cf8
to
7e14222
Compare
9e9b59e
to
4d261b6
Compare
Thanks @danielmarbach! cc @paulomorgado @bording (in case y'all have an idea) I'm busy trying to figure out the following exception (from some of the recent workflow runs). The exception normally would indicate that RabbitMQ closed the connection abruptly, but the RabbitMQ logs state the following:
This error never happens on my local workstation, and only happens with .NET Framework 4.7.2. This error (socket error Exception Stacktrace
|
Sorry. Networking is not really one of my expertises. This might be related to some networking equipment/software between the client and the server. I don't know if there are some ETW events you can collect to investigate. You may find interesting tools here: https://www.youtube.com/watch?v=wf2M0GNQpWI |
@paulomorgado thanks for taking a look.
Yep, normally I would tell a RabbitMQ user "your firewall or load balancer is acting up" but in the case of GitHub Actions and Windows, RabbitMQ and the tests are running on the same machine, communicating via the loopback adapter. |
I'm pretty sure I have observed the same problem occasionally, but I also don't have any insights to offer as to the cause. |
Is this something that can be forced to happen? |
I sure wish it were! |
81b4128
to
f62dcda
Compare
Part of the fix to #1420 Use a linked token source to combine external cancellation and timeout cancelltion for initial RMQ connection Fix API since we have to use the same for all target frameworks. Fix the order of operations when establishing a connection. Add missing ConfigureAwait Add some more missing ConfigureAwait(false) statements. A timeout of 1 second can result in OperationCanceledException Validate hostname in SocketFrameHandler to restore expectation that an exception will be thrown. Ugh. Increase connection timeout for CI Add helpful message to connection failure. Fix tests in a very ugly way. Much TODO Make endpoint resolver SelectOne async. Pass on CancellationToken Pass cancellation token into more connection establishment methods. Fix expected exception for net472 Use Async method otherwise GetAwaiter/GetResult locks up Change expected exception type Increase connection timeout for CI due to slow runners Try out idea to fix odd CI error in GHA Bump version Upgrade dependencies. Modify Windows GHA setup to explicitly create firewall rules for Erlang programs. Tweak setting up firewalls on Windows Use Get-Random to help ensure unique fw rule names
714be7a
to
2aed524
Compare
@stebet @danielmarbach thanks again for taking a look. I realize this isn't a complete use of cancellation tokens, but is a good start. I'd like to merge this soon and produce another alpha release. Community users are using and testing out version 7 alpha releases already. |
Oh, and magically it seems like that socket error is less likely to happen 🤷♂️ |
No comments in a couple days, merging and producing a new 7.0 alpha! |
I'm happy to give feedback but sometimes I just can't do that in a matter of days and especially not before holiday season |
No worries @danielmarbach. All your feedback is appreciated. I just want to keep the ball rolling on version 7 with new alpha releases. |
Part of the fix to #1420