RetryAfter metadata in FixedWindowRateLimiter returns time to next window by asbjornvad · Pull Request #124478 · dotnet/runtime

asbjornvad · 2026-02-16T20:26:20Z

This solves the issue of wrongly returned RetryAfter value, however only in the FixedWindowRateLimiter.
The queue is not taken into consideration in this implementation.

#92557

asbjornvad · 2026-02-16T20:27:17Z

@dotnet-policy-service agree

dotnet-policy-service · 2026-02-16T20:27:20Z

Tagging subscribers to this area: @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR attempts to fix issue #92557 where the FixedWindowRateLimiter incorrectly returns the full window duration as the Retry-After value instead of the remaining time until the next window. The implementation changes the CreateFailedWindowLease method to calculate the remaining time by subtracting the elapsed time from the window duration.

Changes:

Modified CreateFailedWindowLease to calculate remaining time instead of returning static window multiples
Added a test-injectable function field to control elapsed time behavior for testing
Updated test assertions to reflect the new expected behavior
Added a test helper method to manipulate elapsed time in tests

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 12 comments.

File	Description
src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs	Adds injectable `getElapsedTime` function field; reimplements `CreateFailedWindowLease` to calculate remaining window time; comments out original queue-aware implementation
src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs	Adds `SetElapsedTime` helper for testing; updates test expectations; adds calls to set elapsed time to 0 in multiple tests

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

JulieLeeMSFT · 2026-04-13T12:06:21Z

@VSadov, please review this community PR.

VSadov · 2026-04-15T19:32:55Z

I am looking. Not very familiar with the area, so may need some time to understand the change. It does not look too big though.

VSadov · 2026-04-15T22:10:00Z

The change overall looks good to me, I just had a question about use of Interlocked.Read.

VSadov · 2026-04-15T22:18:03Z

Regarding the queue: In my changes the thinking is that you are always told when the window resets. If there is a queue attached then you might end in the queue after waiting the RetryAfter value. This is different from the old way, where the returned value represented when the window + queue window(s) would be over.

I agree with this.
Considering permits count and queue length would just result in overestimate of minimum wait time without any additional guarantees.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

…xt window. This commit solves the issue of wrongly returned RetryAfter value in the FixedWindowRateLimiter. It now returns the time to window refresh. It includes tests to verify that the RetryAfter value is correct when the window has not yet elapsed, and when it has elapsed but has not been refreshed. Changed Interlocked.Read to Volatile.Read, since the objective of the read was to avoid torn longs on 32 bit. Added Volatile.Write when updating _lastReplenishmentTick. The queue is not taken into consideration in this implementation and does not cover other RateLimiters that may have RetryAfter metadata.

VSadov

LGTM. Thanks!

asbjornvad · 2026-04-17T11:46:51Z

Seems like the Build Analysis is stalled due to failed builds.

VSadov · 2026-04-17T15:59:04Z

/ba-g build analysis stalled, all failures are known

Copilot AI review requested due to automatic review settings February 16, 2026 20:26

github-actions Bot added the area-System.Threading label Feb 16, 2026

dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Feb 16, 2026

Copilot started reviewing on behalf of asbjornvad February 16, 2026 20:27 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

build-analysis Bot mentioned this pull request Feb 16, 2026

LibraryImportGenerator.Unit.Tests crashing on linux-x64 mono interpreter #100800

Open

Copilot AI review requested due to automatic review settings February 21, 2026 12:41

asbjornvad force-pushed the #92557-RetryAfter_Ratelimiting branch from a64de24 to 4a1af36 Compare February 21, 2026 12:41

Copilot started reviewing on behalf of asbjornvad February 21, 2026 12:41 View session

Copilot AI reviewed Feb 21, 2026

View reviewed changes

Comment thread ...es/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs Outdated

asbjornvad force-pushed the #92557-RetryAfter_Ratelimiting branch from af257cb to eced7f5 Compare March 1, 2026 15:14

asbjornvad changed the title ~~Initial overview of how the RetryAfter metadata should be set and tested~~ RetryAfter metadata in FixedWindowRateLimiter returns time to next window Mar 1, 2026

asbjornvad marked this pull request as ready for review March 1, 2026 16:57

Copilot AI review requested due to automatic review settings March 1, 2026 16:57

Copilot started reviewing on behalf of asbjornvad March 1, 2026 16:59 View session

Copilot AI reviewed Mar 1, 2026

View reviewed changes

Comment thread ...es/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs

Comment thread src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs

asbjornvad force-pushed the #92557-RetryAfter_Ratelimiting branch from eced7f5 to 361f6a6 Compare March 1, 2026 17:19

This was referenced Mar 1, 2026

Android WebSocket failure #121518

Closed

CharClassSubtraction_DeepNesting_DoesNotStackOverflow fails with PlatformNotSupportedException on browser-wasm (NonBacktracking) #125020

Closed

JulieLeeMSFT assigned VSadov Apr 13, 2026

JulieLeeMSFT requested a review from VSadov April 13, 2026 12:05

VSadov reviewed Apr 15, 2026

View reviewed changes

Comment thread ...es/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs Outdated

Copilot AI review requested due to automatic review settings April 16, 2026 20:05

asbjornvad force-pushed the #92557-RetryAfter_Ratelimiting branch from 361f6a6 to da83766 Compare April 16, 2026 20:05

Copilot started reviewing on behalf of asbjornvad April 16, 2026 20:06 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Comment thread ...es/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs

Comment thread ...es/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs

asbjornvad force-pushed the #92557-RetryAfter_Ratelimiting branch from da83766 to 4d76805 Compare April 16, 2026 20:22

VSadov approved these changes Apr 17, 2026

View reviewed changes

VSadov enabled auto-merge (squash) April 17, 2026 07:20

VSadov merged commit 23e090b into dotnet:main Apr 17, 2026
91 of 96 checks passed

dotnet-maestro Bot mentioned this pull request Apr 18, 2026

[main] Source code updates from dotnet/runtime dotnet/dotnet#6098

Merged

VSadov mentioned this pull request Apr 27, 2026

FixedWindowRateLimiter miscalculates Retry-After value #92557

Closed

asbjornvad deleted the #92557-RetryAfter_Ratelimiting branch April 28, 2026 16:35

coderabbitai Bot mentioned this pull request May 11, 2026

Added rate limiting for auth, and otp endpoints shesha-io/shesha-framework#4928

Merged

github-actions Bot locked and limited conversation to collaborators May 29, 2026

Conversation

asbjornvad commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asbjornvad commented Feb 16, 2026

Uh oh!

dotnet-policy-service Bot commented Feb 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

JulieLeeMSFT commented Apr 13, 2026

Uh oh!

VSadov commented Apr 15, 2026

Uh oh!

Uh oh!

VSadov commented Apr 15, 2026

Uh oh!

VSadov commented Apr 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

VSadov left a comment

Choose a reason for hiding this comment

Uh oh!

asbjornvad commented Apr 17, 2026

Uh oh!

VSadov commented Apr 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

asbjornvad commented Feb 16, 2026 •

edited

Loading