Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add a test that closes a connection with Toxiproxy #1522

Merged
merged 1 commit into from
Apr 9, 2024
Merged

Add a test that closes a connection with Toxiproxy #1522

merged 1 commit into from
Apr 9, 2024

Conversation

lukebakken
Copy link
Contributor

Follow-up / replacement to #1519

@lukebakken lukebakken self-assigned this Mar 26, 2024
@lukebakken lukebakken added this to the 7.0.0 milestone Mar 26, 2024
@michaelklishin
Copy link
Member

Yay! Bunny has been using a Toxiproxy suite for years (optionally, closing connections over the HTTP API is enough for many recovery tests). It's very useful.

@lukebakken
Copy link
Contributor Author

@michaelklishin yes, in #1519 I tried to get to the bottom of a strange issue (.NET not getting an end-of-stream when toxiproxy closes the TCP connection) and decided to re-start here.

@lukebakken lukebakken force-pushed the lukebakken/toxiproxy-close-connection-2 branch from 4283dca to 03f2392 Compare March 27, 2024 23:13
@lukebakken
Copy link
Contributor Author

@michaelklishin it turns out toxiproxy uncovered a specific scenario - what happens when you publish a message and the connection is closed before your library receives the basic.ack?

This library does the correct thing when WaitForConfirmsAsync() is used - it raises an exception when the channel is closed. But, my test code wasn't handling the exception, which made it appear as though the connection wasn't being recovered, and the test would timeout. By handling this case correctly in the test, everything is working fine. Whew!

@lukebakken lukebakken force-pushed the lukebakken/toxiproxy-close-connection-2 branch from 25ca268 to ab88285 Compare March 28, 2024 20:52
@lukebakken
Add a test that closes a connection with Toxiproxy
f5235ba 
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
@lukebakken lukebakken force-pushed the lukebakken/toxiproxy-close-connection-2 branch from dbd70ae to f5235ba Compare April 9, 2024 22:18
@lukebakken lukebakken marked this pull request as ready for review April 9, 2024 22:19
@lukebakken lukebakken merged commit dbbf038 into main Apr 9, 2024
11 checks passed
@lukebakken lukebakken deleted the lukebakken/toxiproxy-close-connection-2 branch April 9, 2024 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@lukebakken lukebakken

Labels
None yet
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@lukebakken @michaelklishin