Skip to content

Fix handling when rate limit lease can't be acquired. #1794

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

Merged
merged 1 commit into from
Feb 25, 2025

Conversation

lukebakken
Copy link
Collaborator

@lukebakken lukebakken commented Feb 24, 2025

Fixes #1793

Also refactor the throttling delay code to be a bit easier to read/understand.

@lukebakken lukebakken self-assigned this Feb 24, 2025
@lukebakken lukebakken added this to the 7.1.1 milestone Feb 24, 2025
@lukebakken lukebakken changed the title Add test for discussion #1793 Fix handling when rate limit lease can't be acquired. Feb 24, 2025
@lukebakken lukebakken marked this pull request as ready for review February 24, 2025 19:47
@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-1793 branch 2 times, most recently from c7d40c3 to 4271e64 Compare February 25, 2025 17:02
* Expand throttling delay code, add comment.
* Add `ThrowsExceptionRateLimiter` and use it in `MaybeSomethingUpWithRateLimiter_GH1793`, which reproduces the issue.
* Reproduce issue by artifically never acquiring rate limiter leases.
* Fix issue by catching `SemaphoreFullException` and checking that the exception was thrown in a valid failure case.
@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-1793 branch from 4271e64 to 068bc00 Compare February 25, 2025 17:04
@lukebakken
Copy link
Collaborator Author

Reviewed with @Gsantomaggio over Zoom 💪

@lukebakken lukebakken merged commit 3862722 into main Feb 25, 2025
11 checks passed
@lukebakken lukebakken deleted the rabbitmq-dotnet-client-1793 branch February 25, 2025 17:06
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.

1 participant